aihuaxu commented on code in PR #13137:
URL: https://github.com/apache/iceberg/pull/13137#discussion_r2118091704
##########
api/src/main/java/org/apache/iceberg/expressions/ExpressionUtil.java:
##########
@@ -646,6 +654,86 @@ private static String sanitizeSimpleString(CharSequence
value) {
return String.format(Locale.ROOT, "(hash-%08x)", HASH_FUNC.apply(value));
}
+ private static String sanitizeVariant(VariantData value, long now, int
today) {
+ return sanitizeVariant(value.value(), now, today);
+ }
+
+ private static String sanitizeVariant(VariantValue value, long now, int
today) {
+ if (value instanceof VariantObject) {
+ return sanitizeVariantObject(value.asObject(), now, today);
+ } else {
+ return sanitizeVariantArray(value.asArray(), now, today);
+ }
Review Comment:
How about primitive type? VariantValue can be VariantPrimitive.
##########
api/src/main/java/org/apache/iceberg/expressions/Literal.java:
##########
@@ -71,6 +72,10 @@ static Literal<BigDecimal> of(BigDecimal value) {
return new Literals.DecimalLiteral(value);
}
+ static Literal<VariantData> of(VariantData value) {
Review Comment:
You should use Variant interface instead of VariantData. So you don't need
to expose VariantData as public class.
##########
api/src/test/java/org/apache/iceberg/expressions/TestExpressionUtil.java:
##########
@@ -1050,6 +1057,53 @@ public void testSelectsPartitions() {
.isFalse();
}
+ @Test
+ public void testSanitizeVariant() {
Review Comment:
Can we test out variant primitive, variant array in addition to variant
object?
##########
api/src/main/java/org/apache/iceberg/variants/VariantData.java:
##########
@@ -20,7 +20,7 @@
import org.apache.iceberg.relocated.com.google.common.base.Preconditions;
-class VariantData implements Variant {
+public class VariantData implements Variant {
Review Comment:
We don't want to expose this as public.
##########
api/src/test/java/org/apache/iceberg/variants/VariantTestUtil.java:
##########
@@ -107,6 +130,124 @@ static SerializedPrimitive createString(String string) {
return SerializedPrimitive.from(buffer, buffer.get(0));
}
+ public static SerializedArray createMixedArray() {
+ ByteBuffer nestedBuffer = VariantTestUtil.createArray(A, C, D);
Review Comment:
I think VariantTestUtil is just a util class with the input provided
externally. We don't want to provide a hard coded createINt16() e.g., here
since in the test, how can you check and compare the expected value.
Better to move to the test itself.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscr...@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscr...@iceberg.apache.org
For additional commands, e-mail: issues-h...@iceberg.apache.org
