Skip to content

Commit ede085d

Browse files
authored
API: Restore the type of the identity transform (#6242)
* API: Restore the type of the identity transform This caused some regression for the Iceberg 1.1.0 release: ``` 2022-11-21T12:05:46.6549795Z [ERROR] io.trino.plugin.iceberg.TestIcebergSystemTables.testManifestsTable Time elapsed: 0.701 s <<< FAILURE! 2022-11-21T12:05:46.6550853Z java.lang.AssertionError: 2022-11-21T12:05:46.6551986Z [Rows for query [SELECT added_data_files_count, existing_rows_count, added_rows_count, deleted_data_files_count, deleted_rows_count, partitions FROM test_schema."test_table$manifests"]] 2022-11-21T12:05:46.6553075Z Expecting: 2022-11-21T12:05:46.6553593Z <(2, 0, 3, 0, 0, [[false, false, 18148, 18149]]), (2, 0, 3, 0, 0, [[false, false, 18147, 18148]])> 2022-11-21T12:05:46.6553980Z to contain exactly in any order: 2022-11-21T12:05:46.6554557Z <[(2, 0, 3, 0, 0, [[false, false, 2019-09-08, 2019-09-09]]), 2022-11-21T12:05:46.6554992Z (2, 0, 3, 0, 0, [[false, false, 2019-09-09, 2019-09-10]])]> 2022-11-21T12:05:46.6555273Z elements not found: 2022-11-21T12:05:46.6555804Z <(2, 0, 3, 0, 0, [[false, false, 2019-09-08, 2019-09-09]]), (2, 0, 3, 0, 0, [[false, false, 2019-09-09, 2019-09-10]])> 2022-11-21T12:05:46.6556132Z and elements not expected: 2022-11-21T12:05:46.6556488Z <(2, 0, 3, 0, 0, [[false, false, 18148, 18149]]), (2, 0, 3, 0, 0, [[false, false, 18147, 18148]])> ``` The system tables (manifests in this example above), would return the days since epoch instead of a date. * Restore equals * Bind SortOrder as well * Add todo
1 parent a59599d commit ede085d

File tree

5 files changed

+79
-5
lines changed

5 files changed

+79
-5
lines changed

api/src/main/java/org/apache/iceberg/SortOrder.java

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -284,7 +284,7 @@ SortOrder buildUnchecked() {
284284

285285
private Transform<?, ?> toTransform(BoundTerm<?> term) {
286286
if (term instanceof BoundReference) {
287-
return Transforms.identity();
287+
return Transforms.identity(term.type());
288288
} else if (term instanceof BoundTransform) {
289289
return ((BoundTransform<?, ?>) term).transform();
290290
} else {

api/src/main/java/org/apache/iceberg/UnboundSortOrder.java

Lines changed: 9 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -23,6 +23,7 @@
2323
import org.apache.iceberg.relocated.com.google.common.collect.Lists;
2424
import org.apache.iceberg.transforms.Transform;
2525
import org.apache.iceberg.transforms.Transforms;
26+
import org.apache.iceberg.types.Type;
2627

2728
public class UnboundSortOrder {
2829
private static final UnboundSortOrder UNSORTED_ORDER =
@@ -40,7 +41,14 @@ public SortOrder bind(Schema schema) {
4041
SortOrder.Builder builder = SortOrder.builderFor(schema).withOrderId(orderId);
4142

4243
for (UnboundSortField field : fields) {
43-
builder.addSortField(field.transform, field.sourceId, field.direction, field.nullOrder);
44+
Type sourceType = schema.findType(field.sourceId);
45+
Transform<?, ?> transform;
46+
if (sourceType != null) {
47+
transform = Transforms.fromString(sourceType, field.transform.toString());
48+
} else {
49+
transform = field.transform;
50+
}
51+
builder.addSortField(transform, field.sourceId, field.direction, field.nullOrder);
4452
}
4553

4654
return builder.build();

api/src/main/java/org/apache/iceberg/transforms/Identity.java

Lines changed: 55 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -29,6 +29,18 @@
2929
class Identity<T> implements Transform<T, T> {
3030
private static final Identity<?> INSTANCE = new Identity<>();
3131

32+
private final Type type;
33+
34+
/**
35+
* Instantiates a new Identity Transform
36+
*
37+
* @deprecated use {@link #get()} instead; will be removed in 2.0.0
38+
*/
39+
@Deprecated
40+
public static <I> Identity<I> get(Type type) {
41+
return new Identity<>(type);
42+
}
43+
3244
@SuppressWarnings("unchecked")
3345
public static <I> Identity<I> get() {
3446
return (Identity<I>) INSTANCE;
@@ -48,14 +60,21 @@ public T apply(T t) {
4860
}
4961
}
5062

51-
private Identity() {}
63+
private Identity() {
64+
this(null);
65+
}
66+
67+
private Identity(Type type) {
68+
this.type = type;
69+
}
5270

5371
@Override
5472
public T apply(T value) {
5573
return value;
5674
}
5775

5876
@Override
77+
@SuppressWarnings("checkstyle:HiddenField")
5978
public SerializableFunction<T, T> bind(Type type) {
6079
Preconditions.checkArgument(canTransform(type), "Cannot bind to unsupported type: %s", type);
6180
return Apply.get();
@@ -66,6 +85,24 @@ public boolean canTransform(Type maybePrimitive) {
6685
return maybePrimitive.isPrimitiveType();
6786
}
6887

88+
/**
89+
* Returns a human-readable String representation of a transformed value.
90+
*
91+
* <p>null values will return "null"
92+
*
93+
* @param value a transformed value
94+
* @return a human-readable String representation of the value
95+
* @deprecated use {@link #toHumanString(Type, Object)} instead; will be removed in 2.0.0
96+
*/
97+
@Deprecated
98+
@Override
99+
public String toHumanString(T value) {
100+
if (this.type != null) {
101+
return toHumanString(this.type, value);
102+
}
103+
return Transform.super.toHumanString(value);
104+
}
105+
69106
@Override
70107
public Type getResultType(Type sourceType) {
71108
return sourceType;
@@ -105,6 +142,23 @@ public boolean isIdentity() {
105142
return true;
106143
}
107144

145+
@Override
146+
public boolean equals(Object o) {
147+
// Can be removed with 2.0.0 deprecation of get(Type)
148+
if (this == o) {
149+
return true;
150+
} else if (o instanceof Identity) {
151+
return true;
152+
}
153+
return false;
154+
}
155+
156+
@Override
157+
public int hashCode() {
158+
// Can be removed with 2.0.0 deprecation of get(Type)
159+
return 0;
160+
}
161+
108162
@Override
109163
public String toString() {
110164
return "identity";

api/src/main/java/org/apache/iceberg/transforms/Transforms.java

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -81,7 +81,7 @@ private Transforms() {}
8181
}
8282

8383
if (transform.equalsIgnoreCase("identity")) {
84-
return Identity.get();
84+
return Identity.get(type);
8585
}
8686

8787
try {
@@ -111,7 +111,7 @@ private Transforms() {}
111111
*/
112112
@Deprecated
113113
public static <T> Transform<T, T> identity(Type type) {
114-
return Identity.get();
114+
return Identity.get(type);
115115
}
116116

117117
/**

api/src/test/java/org/apache/iceberg/transforms/TestIdentity.java

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -69,6 +69,18 @@ public void testDateHumanString() {
6969
"Should produce identical date", dateString, identity.toHumanString(date, dateLit.value()));
7070
}
7171

72+
@Test
73+
public void testDateHumanStringDeprecated() {
74+
Types.DateType date = Types.DateType.get();
75+
Transform<Integer, Integer> identity = Transforms.identity(date);
76+
77+
String dateString = "2017-12-01";
78+
Literal<Integer> dateLit = Literal.of(dateString).to(date);
79+
80+
Assert.assertEquals(
81+
"Should produce identical date", dateString, identity.toHumanString(dateLit.value()));
82+
}
83+
7284
@Test
7385
public void testTimeHumanString() {
7486
Types.TimeType time = Types.TimeType.get();

0 commit comments

Comments
 (0)