You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@iceberg.apache.org by si...@apache.org on 2025/10/23 00:33:48 UTC
(iceberg) branch feature/serialize-bound-expression created (now 67f0039c4a)
This is an automated email from the ASF dual-hosted git repository.
singhpk234 pushed a change to branch feature/serialize-bound-expression
in repository https://gitbox.apache.org/repos/asf/iceberg.git
at 67f0039c4a Address review feedback
This branch includes the following new commits:
new 91a7baeb52 POC: Try Resolved Reference
new ba63fcf387 more changes
new 11269fceac Add ref impl
new bb325ebd2b Address review comment
new 870d601ecc adjust test
new 93252f5bf7 Address review feedback
new 4a2017d8fe Address review per commnuity sync 10/22
new a249936822 more cleanup
new 67f0039c4a Address review feedback
The 9 revisions listed above as "new" are entirely new to this
repository and will be described in separate emails. The revisions
listed as "add" were already present in the repository and have only
been added to this reference.
(iceberg) 02/09: more changes
Posted by si...@apache.org.
This is an automated email from the ASF dual-hosted git repository.
singhpk234 pushed a commit to branch feature/serialize-bound-expression
in repository https://gitbox.apache.org/repos/asf/iceberg.git
commit ba63fcf387e314afc84a99c87dc1d7608c284364
Author: Prashant Kumar Singh <pr...@snowflake.com>
AuthorDate: Fri Sep 26 02:43:32 2025 +0000
more changes
---
.../apache/iceberg/expressions/ExpressionUtil.java | 2 +
.../apache/iceberg/expressions/Expressions.java | 32 +
.../iceberg/expressions/ResolvedReference.java | 17 +
.../iceberg/expressions/ResolvedTransform.java | 88 +++
.../expressions/TestExpressionSerialization.java | 21 +-
.../iceberg/expressions/TestResolvedReference.java | 132 ++++
.../iceberg/expressions/ExpressionParser.java | 55 +-
.../TestEnhancedExpressionParserWithFieldIds.java | 433 +++++++++++
.../TestExpressionParserWithResolvedReference.java | 863 +++++++++++++++++++++
9 files changed, 1639 insertions(+), 4 deletions(-)
diff --git a/api/src/main/java/org/apache/iceberg/expressions/ExpressionUtil.java b/api/src/main/java/org/apache/iceberg/expressions/ExpressionUtil.java
index d3dc00d914..6450321eda 100644
--- a/api/src/main/java/org/apache/iceberg/expressions/ExpressionUtil.java
+++ b/api/src/main/java/org/apache/iceberg/expressions/ExpressionUtil.java
@@ -232,6 +232,8 @@ public class ExpressionUtil {
+ ")";
} else if (term instanceof NamedReference) {
return ((NamedReference<?>) term).name();
+ } else if (term instanceof ResolvedReference) {
+ return ((ResolvedReference<?>) term).name();
} else if (term instanceof BoundReference) {
return ((BoundReference<?>) term).name();
} else {
diff --git a/api/src/main/java/org/apache/iceberg/expressions/Expressions.java b/api/src/main/java/org/apache/iceberg/expressions/Expressions.java
index 1e93d8e75e..658b296f63 100644
--- a/api/src/main/java/org/apache/iceberg/expressions/Expressions.java
+++ b/api/src/main/java/org/apache/iceberg/expressions/Expressions.java
@@ -102,6 +102,38 @@ public class Expressions {
return new UnboundTransform<>(ref(name), Transforms.truncate(width));
}
+ @SuppressWarnings("unchecked")
+ public static <S, T> UnboundTerm<T> bucket(ResolvedReference<S> resolvedRef, int numBuckets) {
+ Transform<S, T> transform = (Transform<S, T>) Transforms.bucket(numBuckets);
+ return new ResolvedTransform<>(resolvedRef, transform);
+ }
+
+ @SuppressWarnings("unchecked")
+ public static <S, T> UnboundTerm<T> year(ResolvedReference<S> resolvedRef) {
+ return new ResolvedTransform<>(resolvedRef, (Transform<S, T>) Transforms.year());
+ }
+
+ @SuppressWarnings("unchecked")
+ public static <S, T> UnboundTerm<T> month(ResolvedReference<S> resolvedRef) {
+ return new ResolvedTransform<>(resolvedRef, (Transform<S, T>) Transforms.month());
+ }
+
+ @SuppressWarnings("unchecked")
+ public static <S, T> UnboundTerm<T> day(ResolvedReference<S> resolvedRef) {
+ return new ResolvedTransform<>(resolvedRef, (Transform<S, T>) Transforms.day());
+ }
+
+ @SuppressWarnings("unchecked")
+ public static <S, T> UnboundTerm<T> hour(ResolvedReference<S> resolvedRef) {
+ return new ResolvedTransform<>(resolvedRef, (Transform<S, T>) Transforms.hour());
+ }
+
+ @SuppressWarnings("unchecked")
+ public static <S, T> UnboundTerm<T> truncate(ResolvedReference<S> resolvedRef, int width) {
+ Transform<S, T> transform = (Transform<S, T>) Transforms.truncate(width);
+ return new ResolvedTransform<>(resolvedRef, transform);
+ }
+
public static <T> UnboundTerm<T> extract(String name, String path, String type) {
return new UnboundExtract<>(ref(name), path, type);
}
diff --git a/api/src/main/java/org/apache/iceberg/expressions/ResolvedReference.java b/api/src/main/java/org/apache/iceberg/expressions/ResolvedReference.java
index b12bff5832..ab272d352b 100644
--- a/api/src/main/java/org/apache/iceberg/expressions/ResolvedReference.java
+++ b/api/src/main/java/org/apache/iceberg/expressions/ResolvedReference.java
@@ -57,6 +57,23 @@ public class ResolvedReference<T> implements UnboundTerm<T>, Reference<T> {
return new NamedReference<>(name);
}
+ @Override
+ public boolean equals(Object o) {
+ if (this == o) {
+ return true;
+ }
+ if (o == null || getClass() != o.getClass()) {
+ return false;
+ }
+ ResolvedReference<?> that = (ResolvedReference<?>) o;
+ return fieldId == that.fieldId && name.equals(that.name);
+ }
+
+ @Override
+ public int hashCode() {
+ return 31 * fieldId + name.hashCode();
+ }
+
@Override
public String toString() {
return String.format("ref(name=\"%s\", fieldId=\"%s\")", name, fieldId);
diff --git a/api/src/main/java/org/apache/iceberg/expressions/ResolvedTransform.java b/api/src/main/java/org/apache/iceberg/expressions/ResolvedTransform.java
new file mode 100644
index 0000000000..287fc6d994
--- /dev/null
+++ b/api/src/main/java/org/apache/iceberg/expressions/ResolvedTransform.java
@@ -0,0 +1,88 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.apache.iceberg.expressions;
+
+import org.apache.iceberg.exceptions.ValidationException;
+import org.apache.iceberg.transforms.Transform;
+import org.apache.iceberg.types.Types;
+
+public class ResolvedTransform<S, T> implements UnboundTerm<T>, Term {
+ private final ResolvedReference<S> ref;
+ private final Transform<S, T> transform;
+
+ ResolvedTransform(ResolvedReference<S> ref, Transform<S, T> transform) {
+ this.ref = ref;
+ this.transform = transform;
+ }
+
+ @Override
+ public NamedReference<S> ref() {
+ return (NamedReference<S>) ref.ref();
+ }
+
+ public ResolvedReference<S> resolvedRef() {
+ return ref;
+ }
+
+ public Transform<S, T> transform() {
+ return transform;
+ }
+
+ @Override
+ public BoundTransform<S, T> bind(Types.StructType struct, boolean caseSensitive) {
+ BoundReference<S> boundRef = (BoundReference<S>) ref.bind(struct, caseSensitive);
+
+ try {
+ ValidationException.check(
+ transform.canTransform(boundRef.type()),
+ "Cannot bind: %s cannot transform %s values from '%s'",
+ transform,
+ boundRef.type(),
+ ref.name());
+ } catch (IllegalArgumentException e) {
+ throw new ValidationException(
+ "Cannot bind: %s cannot transform %s values from '%s'",
+ transform, boundRef.type(), ref.name());
+ }
+
+ return new BoundTransform<>(boundRef, transform);
+ }
+
+ @Override
+ public String toString() {
+ return transform + "(" + ref + ")";
+ }
+
+ @Override
+ public boolean equals(Object o) {
+ if (this == o) {
+ return true;
+ }
+ if (o == null || getClass() != o.getClass()) {
+ return false;
+ }
+ ResolvedTransform<?, ?> that = (ResolvedTransform<?, ?>) o;
+ return ref.equals(that.ref) && transform.equals(that.transform);
+ }
+
+ @Override
+ public int hashCode() {
+ return 31 * ref.hashCode() + transform.hashCode();
+ }
+}
\ No newline at end of file
diff --git a/api/src/test/java/org/apache/iceberg/expressions/TestExpressionSerialization.java b/api/src/test/java/org/apache/iceberg/expressions/TestExpressionSerialization.java
index fc7ddd035b..6968f8c1fe 100644
--- a/api/src/test/java/org/apache/iceberg/expressions/TestExpressionSerialization.java
+++ b/api/src/test/java/org/apache/iceberg/expressions/TestExpressionSerialization.java
@@ -61,7 +61,17 @@ public class TestExpressionSerialization {
Expressions.notIn("s", "abc", "xyz").bind(schema.asStruct()),
Expressions.isNull("a").bind(schema.asStruct()),
Expressions.startsWith("s", "abc").bind(schema.asStruct()),
- Expressions.notStartsWith("s", "xyz").bind(schema.asStruct())
+ Expressions.notStartsWith("s", "xyz").bind(schema.asStruct()),
+ // ResolvedReference tests
+ Expressions.equal(Expressions.ref("a", 34), 5),
+ Expressions.in(Expressions.ref("s", 35), "abc", "xyz"),
+ Expressions.notNull(Expressions.ref("a", 34)),
+ Expressions.isNull(Expressions.ref("a", 34)),
+ Expressions.startsWith(Expressions.ref("s", 35), "test"),
+ // ResolvedReference bound tests
+ Expressions.equal(Expressions.ref("a", 34), 5).bind(schema.asStruct()),
+ Expressions.in(Expressions.ref("s", 35), "abc", "xyz").bind(schema.asStruct()),
+ Expressions.notNull(Expressions.ref("a", 34)).bind(schema.asStruct())
};
for (Expression expression : expressions) {
@@ -196,6 +206,15 @@ public class TestExpressionSerialization {
NamedReference rref = (NamedReference) right;
return lref.name().equals(rref.name());
+ } else if (left instanceof ResolvedReference) {
+ if (!(right instanceof ResolvedReference)) {
+ return false;
+ }
+
+ ResolvedReference lref = (ResolvedReference) left;
+ ResolvedReference rref = (ResolvedReference) right;
+
+ return lref.fieldId() == rref.fieldId() && lref.name().equals(rref.name());
} else if (left instanceof BoundReference) {
if (!(right instanceof BoundReference)) {
return false;
diff --git a/api/src/test/java/org/apache/iceberg/expressions/TestResolvedReference.java b/api/src/test/java/org/apache/iceberg/expressions/TestResolvedReference.java
new file mode 100644
index 0000000000..54c5694138
--- /dev/null
+++ b/api/src/test/java/org/apache/iceberg/expressions/TestResolvedReference.java
@@ -0,0 +1,132 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.apache.iceberg.expressions;
+
+import static org.assertj.core.api.Assertions.assertThat;
+import static org.assertj.core.api.Assertions.assertThatThrownBy;
+
+import org.apache.iceberg.Schema;
+import org.apache.iceberg.exceptions.ValidationException;
+import org.apache.iceberg.types.Types;
+import org.junit.jupiter.api.Test;
+
+public class TestResolvedReference {
+ private static final Schema SCHEMA =
+ new Schema(
+ Types.NestedField.optional(34, "a", Types.IntegerType.get()),
+ Types.NestedField.required(35, "s", Types.StringType.get()));
+
+ @Test
+ public void testResolvedReferenceEquality() {
+ ResolvedReference<Integer> ref1 = new ResolvedReference<>("a", 34);
+ ResolvedReference<Integer> ref2 = new ResolvedReference<>("a", 34);
+ ResolvedReference<Integer> ref3 = new ResolvedReference<>("b", 34);
+ ResolvedReference<Integer> ref4 = new ResolvedReference<>("a", 35);
+
+ // Equal references
+ assertThat(ref1).isEqualTo(ref2);
+ assertThat(ref1.hashCode()).isEqualTo(ref2.hashCode());
+
+ // Different names, same fieldId
+ assertThat(ref1).isNotEqualTo(ref3);
+
+ // Same name, different fieldId
+ assertThat(ref1).isNotEqualTo(ref4);
+ }
+
+ @Test
+ public void testResolvedReferenceBind() {
+ ResolvedReference<Integer> ref = new ResolvedReference<>("a", 34);
+ BoundTerm<Integer> bound = ref.bind(SCHEMA.asStruct(), true);
+
+ assertThat(bound).isInstanceOf(BoundReference.class);
+ BoundReference<Integer> boundRef = (BoundReference<Integer>) bound;
+ assertThat(boundRef.fieldId()).isEqualTo(34);
+ assertThat(boundRef.name()).isEqualTo("a");
+ assertThat(boundRef.type()).isEqualTo(Types.IntegerType.get());
+ }
+
+ @Test
+ public void testResolvedReferenceBindIgnoresCaseSensitivity() {
+ ResolvedReference<Integer> ref = new ResolvedReference<>("A", 34);
+
+ // Should work regardless of case sensitivity since we use fieldId
+ BoundTerm<Integer> bound1 = ref.bind(SCHEMA.asStruct(), true);
+ BoundTerm<Integer> bound2 = ref.bind(SCHEMA.asStruct(), false);
+
+ assertThat(bound1).isInstanceOf(BoundReference.class);
+ assertThat(bound2).isInstanceOf(BoundReference.class);
+ assertThat(((BoundReference<Integer>) bound1).fieldId()).isEqualTo(34);
+ assertThat(((BoundReference<Integer>) bound2).fieldId()).isEqualTo(34);
+ }
+
+ @Test
+ public void testResolvedReferenceBindWithInvalidFieldId() {
+ ResolvedReference<Integer> ref = new ResolvedReference<>("invalid", 999);
+
+ assertThatThrownBy(() -> ref.bind(SCHEMA.asStruct(), true))
+ .isInstanceOf(ValidationException.class)
+ .hasMessageContaining("Cannot find field 'invalid' in struct");
+ }
+
+ @Test
+ public void testResolvedReferenceRef() {
+ ResolvedReference<Integer> ref = new ResolvedReference<>("a", 34);
+ NamedReference<?> namedRef = ref.ref();
+
+ assertThat(namedRef.name()).isEqualTo("a");
+ }
+
+ @Test
+ public void testResolvedReferenceToString() {
+ ResolvedReference<Integer> ref = new ResolvedReference<>("a", 34);
+
+ assertThat(ref.toString()).isEqualTo("ref(name=\"a\", fieldId=\"34\")");
+ }
+
+ @Test
+ public void testResolvedReferenceExpressionIntegration() {
+ // Test that ResolvedReference works in expression predicates
+ Expression expr = Expressions.equal(Expressions.ref("a", 34), 5);
+ assertThat(expr).isInstanceOf(UnboundPredicate.class);
+
+ UnboundPredicate<?> predicate = (UnboundPredicate<?>) expr;
+ assertThat(predicate.term()).isInstanceOf(ResolvedReference.class);
+
+ ResolvedReference<?> resolvedRef = (ResolvedReference<?>) predicate.term();
+ assertThat(resolvedRef.name()).isEqualTo("a");
+ assertThat(resolvedRef.fieldId()).isEqualTo(34);
+ }
+
+ @Test
+ public void testResolvedReferenceUnbind() {
+ // Test that unbinding a bound reference returns a NamedReference for compatibility
+ Expression expr = Expressions.equal(Expressions.ref("a", 34), 5);
+ Expression boundExpr = Binder.bind(SCHEMA.asStruct(), expr, true);
+
+ assertThat(boundExpr).isInstanceOf(BoundPredicate.class);
+ BoundPredicate<?> boundPred = (BoundPredicate<?>) boundExpr;
+
+ UnboundTerm<?> unbound = ExpressionUtil.unbind(boundPred.term());
+ assertThat(unbound).isInstanceOf(NamedReference.class);
+
+ NamedReference<?> namedRef = (NamedReference<?>) unbound;
+ assertThat(namedRef.name()).isEqualTo("a");
+ }
+}
\ No newline at end of file
diff --git a/core/src/main/java/org/apache/iceberg/expressions/ExpressionParser.java b/core/src/main/java/org/apache/iceberg/expressions/ExpressionParser.java
index 9bb5b7d05f..396520f997 100644
--- a/core/src/main/java/org/apache/iceberg/expressions/ExpressionParser.java
+++ b/core/src/main/java/org/apache/iceberg/expressions/ExpressionParser.java
@@ -62,16 +62,27 @@ public class ExpressionParser {
return JsonUtil.generate(gen -> toJson(expression, gen), pretty);
}
+ public static String toJson(Expression expression, boolean pretty, boolean includeFieldIds) {
+ Preconditions.checkArgument(expression != null, "Invalid expression: null");
+ return JsonUtil.generate(gen -> toJson(expression, gen, includeFieldIds), pretty);
+ }
+
public static void toJson(Expression expression, JsonGenerator gen) {
- ExpressionVisitors.visit(expression, new JsonGeneratorVisitor(gen));
+ ExpressionVisitors.visit(expression, new JsonGeneratorVisitor(gen, false));
+ }
+
+ public static void toJson(Expression expression, JsonGenerator gen, boolean includeFieldIds) {
+ ExpressionVisitors.visit(expression, new JsonGeneratorVisitor(gen, includeFieldIds));
}
private static class JsonGeneratorVisitor
extends ExpressionVisitors.CustomOrderExpressionVisitor<Void> {
private final JsonGenerator gen;
+ private final boolean includeFieldIds;
- private JsonGeneratorVisitor(JsonGenerator gen) {
+ private JsonGeneratorVisitor(JsonGenerator gen, boolean includeFieldIds) {
this.gen = gen;
+ this.includeFieldIds = includeFieldIds;
}
/**
@@ -238,9 +249,29 @@ public class ExpressionParser {
UnboundTransform<?, ?> transform = (UnboundTransform<?, ?>) term;
transform(transform.transform().toString(), transform.ref().name());
return;
+ } else if (term instanceof ResolvedTransform) {
+ ResolvedTransform<?, ?> transform = (ResolvedTransform<?, ?>) term;
+ if (includeFieldIds) {
+ transformWithFieldId(transform.transform().toString(), transform.resolvedRef().name(), transform.resolvedRef().fieldId());
+ } else {
+ transform(transform.transform().toString(), transform.ref().name());
+ }
+ return;
} else if (term instanceof BoundTransform) {
BoundTransform<?, ?> transform = (BoundTransform<?, ?>) term;
- transform(transform.transform().toString(), transform.ref().name());
+ if (includeFieldIds) {
+ transformWithFieldId(transform.transform().toString(), transform.ref().name(), transform.ref().fieldId());
+ } else {
+ transform(transform.transform().toString(), transform.ref().name());
+ }
+ return;
+ } else if (term instanceof BoundReference) {
+ BoundReference<?> ref = (BoundReference<?>) term;
+ if (includeFieldIds) {
+ referenceWithFieldId(ref.name(), ref.fieldId());
+ } else {
+ gen.writeString(ref.name());
+ }
return;
} else if (term instanceof Reference) {
gen.writeString(((Reference<?>) term).name());
@@ -257,6 +288,24 @@ public class ExpressionParser {
gen.writeStringField(TERM, name);
gen.writeEndObject();
}
+
+ private void transformWithFieldId(String transform, String name, int fieldId) throws IOException {
+ gen.writeStartObject();
+ gen.writeStringField(TYPE, TRANSFORM);
+ gen.writeStringField(TRANSFORM, transform);
+ // Write term as a ResolvedReference object
+ gen.writeFieldName(TERM);
+ referenceWithFieldId(name, fieldId);
+ gen.writeEndObject();
+ }
+
+ private void referenceWithFieldId(String name, int fieldId) throws IOException {
+ gen.writeStartObject();
+ gen.writeStringField("type", "ref");
+ gen.writeStringField("name", name);
+ gen.writeNumberField("fieldId", fieldId);
+ gen.writeEndObject();
+ }
}
public static Expression fromJson(String json) {
diff --git a/core/src/test/java/org/apache/iceberg/expressions/TestEnhancedExpressionParserWithFieldIds.java b/core/src/test/java/org/apache/iceberg/expressions/TestEnhancedExpressionParserWithFieldIds.java
new file mode 100644
index 0000000000..9d5817e82d
--- /dev/null
+++ b/core/src/test/java/org/apache/iceberg/expressions/TestEnhancedExpressionParserWithFieldIds.java
@@ -0,0 +1,433 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.apache.iceberg.expressions;
+
+import static org.apache.iceberg.types.Types.NestedField.optional;
+import static org.apache.iceberg.types.Types.NestedField.required;
+import static org.assertj.core.api.Assertions.assertThat;
+
+import com.fasterxml.jackson.core.JsonGenerator;
+import com.fasterxml.jackson.databind.JsonNode;
+import java.io.IOException;
+import java.io.UncheckedIOException;
+import java.util.function.Supplier;
+import org.apache.iceberg.Schema;
+import org.apache.iceberg.relocated.com.google.common.base.Preconditions;
+import org.apache.iceberg.types.Types;
+import org.apache.iceberg.util.JsonUtil;
+import org.junit.jupiter.api.Test;
+
+/**
+ * Test demonstrating how ExpressionParser could be enhanced to support field IDs
+ * for ResolvedReference serialization/deserialization.
+ *
+ * This is a proof of concept showing the enhanced JSON format that would preserve
+ * field ID information during serialization round-trips.
+ */
+public class TestEnhancedExpressionParserWithFieldIds {
+
+ private static final Types.StructType STRUCT_TYPE =
+ Types.StructType.of(
+ required(100, "id", Types.LongType.get()),
+ optional(101, "data", Types.StringType.get()),
+ required(102, "active", Types.BooleanType.get()));
+
+ private static final Schema SCHEMA = new Schema(STRUCT_TYPE.fields());
+
+ // Enhanced JSON format constants for field ID support
+ private static final String FIELD_ID = "field-id";
+ private static final String REFERENCE_WITH_ID = "resolved-reference";
+
+ @Test
+ public void testEnhancedJsonFormatWithFieldIds() {
+ // Test the enhanced JSON format that would support field IDs
+ Expression resolvedExpr = Expressions.equal(Expressions.ref("data", 101), "test");
+
+ // Generate enhanced JSON manually to show the concept
+ String enhancedJson = generateEnhancedJson(resolvedExpr);
+
+ // Expected enhanced JSON structure with field ID
+ String expectedJson = "{\n" +
+ " \"type\" : \"eq\",\n" +
+ " \"term\" : {\n" +
+ " \"type\" : \"resolved-reference\",\n" +
+ " \"name\" : \"data\",\n" +
+ " \"field-id\" : 101\n" +
+ " },\n" +
+ " \"value\" : \"test\"\n" +
+ "}";
+
+ assertThat(enhancedJson).isEqualTo(expectedJson);
+ }
+
+ @Test
+ public void testEnhancedParsingWithFieldIds() {
+ // Test parsing enhanced JSON that includes field IDs
+ String enhancedJson = "{\n" +
+ " \"type\" : \"eq\",\n" +
+ " \"term\" : {\n" +
+ " \"type\" : \"resolved-reference\",\n" +
+ " \"name\" : \"data\",\n" +
+ " \"field-id\" : 101\n" +
+ " },\n" +
+ " \"value\" : \"test\"\n" +
+ "}";
+
+ // Parse using enhanced parser (concept)
+ Expression parsed = parseEnhancedJson(enhancedJson);
+
+ assertThat(parsed).isInstanceOf(UnboundPredicate.class);
+ UnboundPredicate<?> predicate = (UnboundPredicate<?>) parsed;
+ assertThat(predicate.term()).isInstanceOf(ResolvedReference.class);
+
+ ResolvedReference<?> resolvedRef = (ResolvedReference<?>) predicate.term();
+ assertThat(resolvedRef.name()).isEqualTo("data");
+ assertThat(resolvedRef.fieldId()).isEqualTo(101);
+ }
+
+ @Test
+ public void testBackwardCompatibilityWithExistingFormat() {
+ // Test that enhanced parser can handle existing JSON format without field IDs
+ String standardJson = "{\n" +
+ " \"type\" : \"eq\",\n" +
+ " \"term\" : \"data\",\n" +
+ " \"value\" : \"test\"\n" +
+ "}";
+
+ Expression parsed = parseEnhancedJson(standardJson);
+
+ assertThat(parsed).isInstanceOf(UnboundPredicate.class);
+ UnboundPredicate<?> predicate = (UnboundPredicate<?>) parsed;
+ assertThat(predicate.term()).isInstanceOf(NamedReference.class);
+
+ NamedReference<?> namedRef = (NamedReference<?>) predicate.term();
+ assertThat(namedRef.name()).isEqualTo("data");
+ }
+
+ @Test
+ public void testComplexExpressionWithMixedReferences() {
+ // Test complex expression with both ResolvedReference and NamedReference
+ Expression mixedExpr = Expressions.and(
+ Expressions.equal(Expressions.ref("data", 101), "test"), // ResolvedReference
+ Expressions.isNull("active")); // NamedReference
+
+ String enhancedJson = generateEnhancedJson(mixedExpr);
+
+ // Should contain both reference types in JSON
+ assertThat(enhancedJson).contains("\"resolved-reference\"");
+ assertThat(enhancedJson).contains("\"field-id\" : 101");
+ assertThat(enhancedJson).contains("\"active\"");
+
+ // Parse back and verify
+ Expression parsed = parseEnhancedJson(enhancedJson);
+ assertThat(parsed).isInstanceOf(And.class);
+
+ And andExpr = (And) parsed;
+
+ // Left side should be ResolvedReference
+ UnboundPredicate<?> leftPred = (UnboundPredicate<?>) andExpr.left();
+ assertThat(leftPred.term()).isInstanceOf(ResolvedReference.class);
+
+ // Right side should be NamedReference
+ UnboundPredicate<?> rightPred = (UnboundPredicate<?>) andExpr.right();
+ assertThat(rightPred.term()).isInstanceOf(NamedReference.class);
+ }
+
+ @Test
+ public void testFieldIdPreservationThroughRoundTrip() {
+ // Test that field IDs are preserved through complete round-trip
+ Expression original = Expressions.and(
+ Expressions.greaterThan(Expressions.ref("id", 100), 50L),
+ Expressions.equal(Expressions.ref("data", 101), "test"));
+
+ // Generate enhanced JSON
+ String json = generateEnhancedJson(original);
+
+ // Parse back
+ Expression parsed = parseEnhancedJson(json);
+
+ // Verify structure is preserved
+ assertThat(parsed).isInstanceOf(And.class);
+ And andExpr = (And) parsed;
+
+ // Check left predicate (id > 50)
+ UnboundPredicate<?> leftPred = (UnboundPredicate<?>) andExpr.left();
+ ResolvedReference<?> leftRef = (ResolvedReference<?>) leftPred.term();
+ assertThat(leftRef.name()).isEqualTo("id");
+ assertThat(leftRef.fieldId()).isEqualTo(100);
+
+ // Check right predicate (data = "test")
+ UnboundPredicate<?> rightPred = (UnboundPredicate<?>) andExpr.right();
+ ResolvedReference<?> rightRef = (ResolvedReference<?>) rightPred.term();
+ assertThat(rightRef.name()).isEqualTo("data");
+ assertThat(rightRef.fieldId()).isEqualTo(101);
+ }
+
+ // Helper methods to demonstrate enhanced JSON generation and parsing
+
+ private String generateEnhancedJson(Expression expr) {
+ return JsonUtil.generate(gen -> {
+ try {
+ generateEnhanced(expr, gen);
+ } catch (IOException e) {
+ throw new UncheckedIOException(e);
+ }
+ }, true);
+ }
+
+ private void generateEnhanced(Expression expr, JsonGenerator gen) throws IOException {
+ ExpressionVisitors.visit(expr, new EnhancedJsonVisitor(gen));
+ }
+
+ private Expression parseEnhancedJson(String json) {
+ try {
+ JsonNode node = JsonUtil.mapper().readTree(json);
+ return parseEnhancedExpression(node);
+ } catch (Exception e) {
+ throw new RuntimeException("Failed to parse JSON", e);
+ }
+ }
+
+ // Enhanced JSON visitor that supports field IDs
+ private static class EnhancedJsonVisitor extends ExpressionVisitors.CustomOrderExpressionVisitor<Void> {
+ private final JsonGenerator gen;
+
+ EnhancedJsonVisitor(JsonGenerator gen) {
+ this.gen = gen;
+ }
+
+ private void toJson(Supplier<Void> child) {
+ child.get();
+ }
+
+ private Void generate(Runnable task) {
+ try {
+ task.run();
+ return null;
+ } catch (Exception e) {
+ throw new RuntimeException(e);
+ }
+ }
+
+ @Override
+ public Void alwaysTrue() {
+ return generate(() -> {
+ try {
+ gen.writeBoolean(true);
+ } catch (IOException e) {
+ throw new UncheckedIOException(e);
+ }
+ });
+ }
+
+ @Override
+ public Void alwaysFalse() {
+ return generate(() -> {
+ try {
+ gen.writeBoolean(false);
+ } catch (IOException e) {
+ throw new UncheckedIOException(e);
+ }
+ });
+ }
+
+ @Override
+ public Void not(Supplier<Void> result) {
+ return generate(() -> {
+ try {
+ gen.writeStartObject();
+ gen.writeStringField("type", "not");
+ gen.writeFieldName("child");
+ toJson(result);
+ gen.writeEndObject();
+ } catch (IOException e) {
+ throw new UncheckedIOException(e);
+ }
+ });
+ }
+
+ @Override
+ public Void and(Supplier<Void> leftResult, Supplier<Void> rightResult) {
+ return generate(() -> {
+ try {
+ gen.writeStartObject();
+ gen.writeStringField("type", "and");
+ gen.writeFieldName("left");
+ toJson(leftResult);
+ gen.writeFieldName("right");
+ toJson(rightResult);
+ gen.writeEndObject();
+ } catch (IOException e) {
+ throw new UncheckedIOException(e);
+ }
+ });
+ }
+
+ @Override
+ public Void or(Supplier<Void> leftResult, Supplier<Void> rightResult) {
+ return generate(() -> {
+ try {
+ gen.writeStartObject();
+ gen.writeStringField("type", "or");
+ gen.writeFieldName("left");
+ toJson(leftResult);
+ gen.writeFieldName("right");
+ toJson(rightResult);
+ gen.writeEndObject();
+ } catch (IOException e) {
+ throw new UncheckedIOException(e);
+ }
+ });
+ }
+
+ @Override
+ public <T> Void predicate(UnboundPredicate<T> pred) {
+ return generate(() -> {
+ try {
+ gen.writeStartObject();
+ gen.writeStringField("type", pred.op().toString().toLowerCase().replace("_", "-"));
+ gen.writeFieldName("term");
+ writeTerm(pred.term());
+
+ if (pred.literals() != null && !pred.literals().isEmpty()) {
+ if (pred.literals().size() == 1) {
+ gen.writeFieldName("value");
+ writeLiteral(pred.literals().get(0));
+ } else {
+ gen.writeFieldName("values");
+ gen.writeStartArray();
+ for (Literal<T> literal : pred.literals()) {
+ writeLiteral(literal);
+ }
+ gen.writeEndArray();
+ }
+ }
+
+ gen.writeEndObject();
+ } catch (IOException e) {
+ throw new UncheckedIOException(e);
+ }
+ });
+ }
+
+ private void writeTerm(UnboundTerm<?> term) throws IOException {
+ if (term instanceof ResolvedReference) {
+ ResolvedReference<?> resolvedRef = (ResolvedReference<?>) term;
+ gen.writeStartObject();
+ gen.writeStringField("type", REFERENCE_WITH_ID);
+ gen.writeStringField("name", resolvedRef.name());
+ gen.writeNumberField(FIELD_ID, resolvedRef.fieldId());
+ gen.writeEndObject();
+ } else if (term instanceof NamedReference) {
+ gen.writeString(((NamedReference<?>) term).name());
+ } else {
+ throw new UnsupportedOperationException("Unsupported term type: " + term.getClass());
+ }
+ }
+
+ private void writeLiteral(Literal<?> literal) throws IOException {
+ Object value = literal.value();
+ if (value instanceof String) {
+ gen.writeString((String) value);
+ } else if (value instanceof Long) {
+ gen.writeNumber((Long) value);
+ } else if (value instanceof Integer) {
+ gen.writeNumber((Integer) value);
+ } else if (value instanceof Boolean) {
+ gen.writeBoolean((Boolean) value);
+ } else if (value instanceof Double) {
+ gen.writeNumber((Double) value);
+ } else if (value instanceof Float) {
+ gen.writeNumber((Float) value);
+ } else {
+ gen.writeString(value.toString());
+ }
+ }
+ }
+
+ // Enhanced expression parser that supports field IDs
+ private Expression parseEnhancedExpression(JsonNode node) {
+ if (node.isBoolean()) {
+ return node.asBoolean() ? Expressions.alwaysTrue() : Expressions.alwaysFalse();
+ }
+
+ if (!node.isObject()) {
+ throw new IllegalArgumentException("Cannot parse expression from: " + node);
+ }
+
+ String type = JsonUtil.getString("type", node);
+ switch (type) {
+ case "and":
+ return Expressions.and(
+ parseEnhancedExpression(JsonUtil.get("left", node)),
+ parseEnhancedExpression(JsonUtil.get("right", node)));
+ case "or":
+ return Expressions.or(
+ parseEnhancedExpression(JsonUtil.get("left", node)),
+ parseEnhancedExpression(JsonUtil.get("right", node)));
+ case "not":
+ return Expressions.not(parseEnhancedExpression(JsonUtil.get("child", node)));
+ case "eq":
+ return Expressions.equal(
+ parseEnhancedTerm(JsonUtil.get("term", node)),
+ parseValue(JsonUtil.get("value", node)));
+ case "gt":
+ return Expressions.greaterThan(
+ parseEnhancedTerm(JsonUtil.get("term", node)),
+ parseValue(JsonUtil.get("value", node)));
+ case "is-null":
+ return Expressions.isNull(parseEnhancedTerm(JsonUtil.get("term", node)));
+ default:
+ throw new IllegalArgumentException("Unknown expression type: " + type);
+ }
+ }
+
+ @SuppressWarnings("unchecked")
+ private <T> UnboundTerm<T> parseEnhancedTerm(JsonNode node) {
+ if (node.isTextual()) {
+ return Expressions.ref(node.asText());
+ }
+
+ if (node.isObject()) {
+ String type = JsonUtil.getString("type", node);
+ if (REFERENCE_WITH_ID.equals(type)) {
+ String name = JsonUtil.getString("name", node);
+ int fieldId = JsonUtil.getInt(FIELD_ID, node);
+ return Expressions.ref(name, fieldId);
+ }
+ }
+
+ throw new IllegalArgumentException("Cannot parse term from: " + node);
+ }
+
+ private Object parseValue(JsonNode node) {
+ if (node.isTextual()) {
+ return node.asText();
+ } else if (node.isLong()) {
+ return node.asLong();
+ } else if (node.isInt()) {
+ return node.asInt();
+ } else if (node.isBoolean()) {
+ return node.asBoolean();
+ } else if (node.isDouble()) {
+ return node.asDouble();
+ }
+ return node.asText();
+ }
+}
\ No newline at end of file
diff --git a/core/src/test/java/org/apache/iceberg/expressions/TestExpressionParserWithResolvedReference.java b/core/src/test/java/org/apache/iceberg/expressions/TestExpressionParserWithResolvedReference.java
new file mode 100644
index 0000000000..97981a6471
--- /dev/null
+++ b/core/src/test/java/org/apache/iceberg/expressions/TestExpressionParserWithResolvedReference.java
@@ -0,0 +1,863 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.apache.iceberg.expressions;
+
+import static org.apache.iceberg.types.Types.NestedField.optional;
+import static org.apache.iceberg.types.Types.NestedField.required;
+import static org.assertj.core.api.Assertions.assertThat;
+
+import java.math.BigDecimal;
+import java.util.UUID;
+import org.apache.iceberg.Schema;
+import org.apache.iceberg.types.Types;
+import org.junit.jupiter.api.Test;
+
+public class TestExpressionParserWithResolvedReference {
+
+ private static final Types.StructType STRUCT_TYPE =
+ Types.StructType.of(
+ required(100, "id", Types.LongType.get()),
+ optional(101, "data", Types.StringType.get()),
+ required(102, "b", Types.BooleanType.get()),
+ optional(103, "i", Types.IntegerType.get()),
+ required(104, "l", Types.LongType.get()),
+ optional(105, "f", Types.FloatType.get()),
+ required(106, "d", Types.DoubleType.get()),
+ optional(107, "date", Types.DateType.get()),
+ required(108, "ts", Types.TimestampType.withoutZone()),
+ required(110, "s", Types.StringType.get()),
+ required(111, "uuid", Types.UUIDType.get()),
+ required(112, "fixed", Types.FixedType.ofLength(7)),
+ optional(113, "bytes", Types.BinaryType.get()),
+ required(114, "dec_9_0", Types.DecimalType.of(9, 0)),
+ required(115, "dec_11_2", Types.DecimalType.of(11, 2)));
+
+ private static final Schema SCHEMA = new Schema(STRUCT_TYPE.fields());
+
+ @Test
+ public void testResolvedReferenceExpressionSerialization() {
+ // Create expressions using ResolvedReference
+ Expression[] resolvedExpressions = new Expression[] {
+ Expressions.equal(Expressions.ref("id", 100), 42L),
+ Expressions.lessThan(Expressions.ref("data", 101), "test"),
+ Expressions.greaterThanOrEqual(Expressions.ref("i", 103), 10),
+ Expressions.isNull(Expressions.ref("f", 105)),
+ Expressions.notNull(Expressions.ref("date", 107)),
+ Expressions.startsWith(Expressions.ref("s", 110), "prefix"),
+ Expressions.in(Expressions.ref("l", 104), 1L, 2L, 3L),
+ Expressions.notIn(Expressions.ref("b", 102), true, false),
+ Expressions.isNaN(Expressions.ref("d", 106)),
+ Expressions.notNaN(Expressions.ref("f", 105))
+ };
+
+ for (Expression expr : resolvedExpressions) {
+ // Verify the expression uses ResolvedReference
+ assertThat(expr).isInstanceOf(UnboundPredicate.class);
+ UnboundPredicate<?> predicate = (UnboundPredicate<?>) expr;
+ assertThat(predicate.term()).isInstanceOf(ResolvedReference.class);
+
+ // Test JSON serialization
+ String json = ExpressionParser.toJson(expr, true);
+ assertThat(json).isNotNull();
+ assertThat(json).contains("\"type\"");
+
+ // Test that JSON contains the field name (not field ID since parser doesn't support it yet)
+ ResolvedReference<?> resolvedRef = (ResolvedReference<?>) predicate.term();
+ assertThat(json).contains(resolvedRef.name());
+ }
+ }
+
+ @Test
+ public void testResolvedReferenceRoundTripCompatibility() {
+ // Test that ResolvedReference expressions can be serialized and parsed back
+ Expression resolvedExpr = Expressions.equal(Expressions.ref("id", 100), 42L);
+ Expression namedExpr = Expressions.equal(Expressions.ref("id"), 42L);
+
+ // Both should produce the same JSON since parser only uses names
+ String resolvedJson = ExpressionParser.toJson(resolvedExpr, true);
+ String namedJson = ExpressionParser.toJson(namedExpr, true);
+ assertThat(resolvedJson).isEqualTo(namedJson);
+
+ // Parse back and verify equivalence
+ Expression parsedFromResolved = ExpressionParser.fromJson(resolvedJson, SCHEMA);
+ Expression parsedFromNamed = ExpressionParser.fromJson(namedJson, SCHEMA);
+
+ // Both parsed expressions should be equivalent
+ assertThat(ExpressionUtil.equivalent(parsedFromResolved, parsedFromNamed, STRUCT_TYPE, true))
+ .isTrue();
+
+ // The parsed expression should be equivalent to the original named reference expression
+ assertThat(ExpressionUtil.equivalent(namedExpr, parsedFromResolved, STRUCT_TYPE, true))
+ .isTrue();
+ }
+
+ @Test
+ public void testResolvedReferenceComplexExpressions() {
+ // Test complex expressions with ResolvedReference
+ Expression complexExpr = Expressions.and(
+ Expressions.or(
+ Expressions.equal(Expressions.ref("data", 101), "test"),
+ Expressions.isNull(Expressions.ref("data", 101))),
+ Expressions.greaterThanOrEqual(Expressions.ref("id", 100), 100L));
+
+ // Serialize to JSON
+ String json = ExpressionParser.toJson(complexExpr, true);
+ assertThat(json).contains("\"type\" : \"and\"");
+ assertThat(json).contains("\"type\" : \"or\"");
+ assertThat(json).contains("\"data\"");
+ assertThat(json).contains("\"id\"");
+
+ // Parse back
+ Expression parsed = ExpressionParser.fromJson(json, SCHEMA);
+
+ // Create equivalent expression with NamedReference for comparison
+ Expression namedEquivalent = Expressions.and(
+ Expressions.or(
+ Expressions.equal("data", "test"),
+ Expressions.isNull("data")),
+ Expressions.greaterThanOrEqual("id", 100L));
+
+ // Should be equivalent
+ assertThat(ExpressionUtil.equivalent(namedEquivalent, parsed, STRUCT_TYPE, true))
+ .isTrue();
+ }
+
+ @Test
+ public void testResolvedReferenceTransformExpressions() {
+ // Test transform expressions - using NamedReference for transforms since
+ // ResolvedReference needs proper type parameters for transform methods
+ Expression dayTransform = Expressions.equal(
+ Expressions.day("date"), "2023-01-15");
+ Expression bucketTransform = Expressions.equal(
+ Expressions.bucket("id", 10), 5);
+
+ // Test serialization
+ String dayJson = ExpressionParser.toJson(dayTransform, true);
+ String bucketJson = ExpressionParser.toJson(bucketTransform, true);
+
+ assertThat(dayJson).contains("\"transform\" : \"day\"");
+ assertThat(dayJson).contains("\"term\" : \"date\"");
+ assertThat(bucketJson).contains("\"transform\" : \"bucket[10]\"");
+ assertThat(bucketJson).contains("\"term\" : \"id\"");
+
+ // Test round-trip
+ Expression parsedDay = ExpressionParser.fromJson(dayJson, SCHEMA);
+ Expression parsedBucket = ExpressionParser.fromJson(bucketJson, SCHEMA);
+
+ // Should maintain equivalence after round-trip
+ assertThat(ExpressionUtil.equivalent(dayTransform, parsedDay, STRUCT_TYPE, true))
+ .isTrue();
+ assertThat(ExpressionUtil.equivalent(bucketTransform, parsedBucket, STRUCT_TYPE, true))
+ .isTrue();
+ }
+
+ @Test
+ public void testResolvedReferenceBindingAfterParsing() {
+ // Test that expressions with ResolvedReference bind correctly after parsing
+ Expression original = Expressions.equal(Expressions.ref("id", 100), 42L);
+
+ // Serialize and parse
+ String json = ExpressionParser.toJson(original, true);
+ Expression parsed = ExpressionParser.fromJson(json, SCHEMA);
+
+ // Both should bind successfully
+ Expression originalBound = Binder.bind(STRUCT_TYPE, original, true);
+ Expression parsedBound = Binder.bind(STRUCT_TYPE, parsed, true);
+
+ // Both bound expressions should be identical
+ assertThat(originalBound).isInstanceOf(BoundPredicate.class);
+ assertThat(parsedBound).isInstanceOf(BoundPredicate.class);
+
+ BoundPredicate<?> originalBoundPred = (BoundPredicate<?>) originalBound;
+ BoundPredicate<?> parsedBoundPred = (BoundPredicate<?>) parsedBound;
+
+ // Should reference the same field
+ assertThat(originalBoundPred.ref().fieldId()).isEqualTo(100);
+ assertThat(parsedBoundPred.ref().fieldId()).isEqualTo(100);
+ assertThat(originalBoundPred.ref().name()).isEqualTo("id");
+ assertThat(parsedBoundPred.ref().name()).isEqualTo("id");
+ }
+
+ @Test
+ public void testResolvedReferenceWithDifferentTypes() {
+ // Test ResolvedReference with various data types
+ Expression[] typedExpressions = new Expression[] {
+ Expressions.equal(Expressions.ref("b", 102), true),
+ Expressions.equal(Expressions.ref("i", 103), 42),
+ Expressions.equal(Expressions.ref("l", 104), 42L),
+ Expressions.equal(Expressions.ref("f", 105), 3.14f),
+ Expressions.equal(Expressions.ref("d", 106), 3.14159),
+ Expressions.equal(Expressions.ref("s", 110), "test string"),
+ Expressions.equal(Expressions.ref("uuid", 111), UUID.randomUUID()),
+ Expressions.equal(Expressions.ref("dec_11_2", 115), new BigDecimal("123.45"))
+ };
+
+ for (Expression expr : typedExpressions) {
+ // Test serialization doesn't break with different types
+ String json = ExpressionParser.toJson(expr, true);
+ assertThat(json).isNotNull();
+
+ // Test parsing back
+ Expression parsed = ExpressionParser.fromJson(json, SCHEMA);
+ assertThat(parsed).isNotNull();
+
+ // Test binding
+ Expression bound = Binder.bind(STRUCT_TYPE, parsed, true);
+ assertThat(bound).isInstanceOf(BoundPredicate.class);
+ }
+ }
+
+ @Test
+ public void testResolvedReferenceJsonStructure() {
+ // Test the exact JSON structure produced by ResolvedReference
+ Expression expr = Expressions.equal(Expressions.ref("data", 101), "test");
+ String json = ExpressionParser.toJson(expr, true);
+
+ // The JSON should look like a regular reference since parser doesn't support field IDs yet
+ String expectedStructure = "{\n" +
+ " \"type\" : \"eq\",\n" +
+ " \"term\" : \"data\",\n" +
+ " \"value\" : \"test\"\n" +
+ "}";
+
+ assertThat(json).isEqualTo(expectedStructure);
+
+ // Verify it parses back correctly
+ Expression parsed = ExpressionParser.fromJson(json);
+ assertThat(parsed).isInstanceOf(UnboundPredicate.class);
+
+ UnboundPredicate<?> predicate = (UnboundPredicate<?>) parsed;
+ assertThat(predicate.term()).isInstanceOf(NamedReference.class);
+ assertThat(predicate.term().ref().name()).isEqualTo("data");
+ }
+
+ @Test
+ public void testResolvedReferenceEquivalenceAfterSerialization() {
+ // Test that ResolvedReference expressions maintain equivalence after serialization
+ Expression resolvedExpr = Expressions.and(
+ Expressions.greaterThan(Expressions.ref("id", 100), 50L),
+ Expressions.lessThan(Expressions.ref("id", 100), 200L));
+
+ Expression namedExpr = Expressions.and(
+ Expressions.greaterThan("id", 50L),
+ Expressions.lessThan("id", 200L));
+
+ // Serialize both
+ String resolvedJson = ExpressionParser.toJson(resolvedExpr, true);
+ String namedJson = ExpressionParser.toJson(namedExpr, true);
+
+ // Should produce identical JSON
+ assertThat(resolvedJson).isEqualTo(namedJson);
+
+ // Parse both back
+ Expression parsedResolved = ExpressionParser.fromJson(resolvedJson, SCHEMA);
+ Expression parsedNamed = ExpressionParser.fromJson(namedJson, SCHEMA);
+
+ // All should be equivalent
+ assertThat(ExpressionUtil.equivalent(resolvedExpr, namedExpr, STRUCT_TYPE, true))
+ .isTrue();
+ assertThat(ExpressionUtil.equivalent(parsedResolved, parsedNamed, STRUCT_TYPE, true))
+ .isTrue();
+ assertThat(ExpressionUtil.equivalent(resolvedExpr, parsedResolved, STRUCT_TYPE, true))
+ .isTrue();
+ }
+
+ @Test
+ public void testResolvedReferenceTransformExpressionEquivalence() {
+ // Test expressions that reference transforms where the transform terms are equivalent to resolved references
+ // Since UnboundTransform only accepts NamedReference, we test equivalence through binding/unbinding
+
+ // Create transform expressions using NamedReference (current approach)
+ Expression dayTransformNamed = Expressions.equal(Expressions.day("date"), "2023-01-15");
+ Expression bucketTransformNamed = Expressions.equal(Expressions.bucket("id", 10), 5);
+ Expression truncateTransformNamed = Expressions.equal(Expressions.truncate("data", 4), "test");
+
+ // Create equivalent expressions using ResolvedReference for the predicate terms
+ Expression dayWithResolvedRef = Expressions.equal(Expressions.ref("date", 107), "2023-01-15");
+ Expression bucketWithResolvedRef = Expressions.equal(Expressions.ref("id", 100), 5L);
+ Expression truncateWithResolvedRef = Expressions.equal(Expressions.ref("data", 101), "test");
+
+ // Bind all expressions
+ Expression boundDayTransform = Binder.bind(STRUCT_TYPE, dayTransformNamed, true);
+ Expression boundBucketTransform = Binder.bind(STRUCT_TYPE, bucketTransformNamed, true);
+ Expression boundTruncateTransform = Binder.bind(STRUCT_TYPE, truncateTransformNamed, true);
+
+ Expression boundDayResolved = Binder.bind(STRUCT_TYPE, dayWithResolvedRef, true);
+ Expression boundBucketResolved = Binder.bind(STRUCT_TYPE, bucketWithResolvedRef, true);
+ Expression boundTruncateResolved = Binder.bind(STRUCT_TYPE, truncateWithResolvedRef, true);
+
+ // Verify all expressions bound successfully
+ assertThat(boundDayTransform).isInstanceOf(BoundPredicate.class);
+ assertThat(boundBucketTransform).isInstanceOf(BoundPredicate.class);
+ assertThat(boundTruncateTransform).isInstanceOf(BoundPredicate.class);
+ assertThat(boundDayResolved).isInstanceOf(BoundPredicate.class);
+ assertThat(boundBucketResolved).isInstanceOf(BoundPredicate.class);
+ assertThat(boundTruncateResolved).isInstanceOf(BoundPredicate.class);
+
+ // Test transform expressions in complex expressions with resolved references
+ Expression complexTransformExpr = Expressions.and(
+ Expressions.equal(Expressions.day("date"), "2023-01-15"),
+ Expressions.equal(Expressions.ref("id", 100), 42L));
+
+ Expression boundComplexExpr = Binder.bind(STRUCT_TYPE, complexTransformExpr, true);
+ assertThat(boundComplexExpr).isInstanceOf(And.class);
+
+ // Verify serialization works for transform expressions that coexist with resolved references
+ String complexJson = ExpressionParser.toJson(complexTransformExpr, true);
+ assertThat(complexJson).contains("\"transform\" : \"day\"");
+ assertThat(complexJson).contains("\"term\" : \"date\"");
+ assertThat(complexJson).contains("\"term\" : \"id\"");
+
+ // Verify round-trip maintains correctness
+ Expression parsedComplex = ExpressionParser.fromJson(complexJson, SCHEMA);
+ Expression boundParsedComplex = Binder.bind(STRUCT_TYPE, parsedComplex, true);
+
+ // Both bound expressions should reference the same fields
+ assertThat(boundComplexExpr.toString()).isEqualTo(boundParsedComplex.toString());
+ }
+
+ @Test
+ public void testResolvedReferenceInComplexTransformExpressions() {
+ // Test complex expressions that combine transforms with resolved references
+ Expression complexExpr = Expressions.or(
+ Expressions.and(
+ Expressions.equal(Expressions.bucket("id", 8), 3),
+ Expressions.equal(Expressions.ref("data", 101), "test")),
+ Expressions.and(
+ Expressions.equal(Expressions.day("date"), "2023-01-15"),
+ Expressions.isNull(Expressions.ref("f", 105))));
+
+ // Test serialization
+ String json = ExpressionParser.toJson(complexExpr, true);
+ assertThat(json).contains("\"transform\" : \"bucket[8]\"");
+ assertThat(json).contains("\"transform\" : \"day\"");
+ assertThat(json).contains("\"term\" : \"id\"");
+ assertThat(json).contains("\"term\" : \"date\"");
+ assertThat(json).contains("\"term\" : \"data\"");
+ assertThat(json).contains("\"term\" : \"f\"");
+
+ // Test that parsing back maintains structure
+ Expression parsed = ExpressionParser.fromJson(json, SCHEMA);
+ assertThat(parsed).isInstanceOf(Or.class);
+
+ // Test binding works correctly
+ Expression bound = Binder.bind(STRUCT_TYPE, parsed, true);
+ assertThat(bound).isInstanceOf(Or.class);
+
+ // Verify equivalence with original
+ assertThat(ExpressionUtil.equivalent(complexExpr, parsed, STRUCT_TYPE, true)).isTrue();
+
+ // Test that mixed transform and resolved reference expressions bind to same fields
+ Expression originalBound = Binder.bind(STRUCT_TYPE, complexExpr, true);
+
+ // Both bound expressions should be structurally equivalent
+ assertThat(originalBound.toString()).isEqualTo(bound.toString());
+ }
+
+ @Test
+ public void testTransformExpressionsWithResolvedReference() {
+ // Test expressions that reference transforms which in turn reference resolved references
+ // Create transforms using the new ResolvedReference-based factory methods
+ Expression bucketExpr = Expressions.equal(
+ Expressions.bucket(Expressions.ref("id", 100), 8), 3);
+ Expression dayExpr = Expressions.equal(
+ Expressions.day(Expressions.ref("date", 107)), "2023-01-15");
+ Expression hourExpr = Expressions.equal(
+ Expressions.hour(Expressions.ref("ts", 108)), 10);
+ Expression truncateExpr = Expressions.equal(
+ Expressions.truncate(Expressions.ref("data", 101), 4), "test");
+
+ // Verify the expressions are created correctly
+ assertThat(bucketExpr).isInstanceOf(UnboundPredicate.class);
+ assertThat(dayExpr).isInstanceOf(UnboundPredicate.class);
+ assertThat(hourExpr).isInstanceOf(UnboundPredicate.class);
+ assertThat(truncateExpr).isInstanceOf(UnboundPredicate.class);
+
+ // Verify the terms are ResolvedTransform instances
+ UnboundPredicate<?> bucketPred = (UnboundPredicate<?>) bucketExpr;
+ UnboundPredicate<?> dayPred = (UnboundPredicate<?>) dayExpr;
+ UnboundPredicate<?> hourPred = (UnboundPredicate<?>) hourExpr;
+ UnboundPredicate<?> truncatePred = (UnboundPredicate<?>) truncateExpr;
+
+ assertThat(bucketPred.term()).isInstanceOf(ResolvedTransform.class);
+ assertThat(dayPred.term()).isInstanceOf(ResolvedTransform.class);
+ assertThat(hourPred.term()).isInstanceOf(ResolvedTransform.class);
+ assertThat(truncatePred.term()).isInstanceOf(ResolvedTransform.class);
+
+ // Verify that ResolvedTransform preserves the ResolvedReference with field IDs
+ ResolvedTransform<?, ?> bucketTransform = (ResolvedTransform<?, ?>) bucketPred.term();
+ ResolvedTransform<?, ?> dayTransform = (ResolvedTransform<?, ?>) dayPred.term();
+ ResolvedTransform<?, ?> hourTransform = (ResolvedTransform<?, ?>) hourPred.term();
+ ResolvedTransform<?, ?> truncateTransform = (ResolvedTransform<?, ?>) truncatePred.term();
+
+ assertThat(bucketTransform.resolvedRef().fieldId()).isEqualTo(100);
+ assertThat(bucketTransform.resolvedRef().name()).isEqualTo("id");
+ assertThat(dayTransform.resolvedRef().fieldId()).isEqualTo(107);
+ assertThat(dayTransform.resolvedRef().name()).isEqualTo("date");
+ assertThat(hourTransform.resolvedRef().fieldId()).isEqualTo(108);
+ assertThat(hourTransform.resolvedRef().name()).isEqualTo("ts");
+ assertThat(truncateTransform.resolvedRef().fieldId()).isEqualTo(101);
+ assertThat(truncateTransform.resolvedRef().name()).isEqualTo("data");
+
+ // Test serialization
+ String bucketJson = ExpressionParser.toJson(bucketExpr, true);
+ String dayJson = ExpressionParser.toJson(dayExpr, true);
+ String hourJson = ExpressionParser.toJson(hourExpr, true);
+ String truncateJson = ExpressionParser.toJson(truncateExpr, true);
+
+ // Verify JSON contains the expected transform and term information
+ assertThat(bucketJson).contains("\"transform\" : \"bucket[8]\"");
+ assertThat(bucketJson).contains("\"term\" : \"id\"");
+ assertThat(dayJson).contains("\"transform\" : \"day\"");
+ assertThat(dayJson).contains("\"term\" : \"date\"");
+ assertThat(hourJson).contains("\"transform\" : \"hour\"");
+ assertThat(hourJson).contains("\"term\" : \"ts\"");
+ assertThat(truncateJson).contains("\"transform\" : \"truncate[4]\"");
+ assertThat(truncateJson).contains("\"term\" : \"data\"");
+
+ // Test parsing back
+ Expression parsedBucket = ExpressionParser.fromJson(bucketJson, SCHEMA);
+ Expression parsedDay = ExpressionParser.fromJson(dayJson, SCHEMA);
+ Expression parsedHour = ExpressionParser.fromJson(hourJson, SCHEMA);
+ Expression parsedTruncate = ExpressionParser.fromJson(truncateJson, SCHEMA);
+
+ // Verify equivalence after round-trip
+ assertThat(ExpressionUtil.equivalent(bucketExpr, parsedBucket, STRUCT_TYPE, true)).isTrue();
+ assertThat(ExpressionUtil.equivalent(dayExpr, parsedDay, STRUCT_TYPE, true)).isTrue();
+ assertThat(ExpressionUtil.equivalent(hourExpr, parsedHour, STRUCT_TYPE, true)).isTrue();
+ assertThat(ExpressionUtil.equivalent(truncateExpr, parsedTruncate, STRUCT_TYPE, true)).isTrue();
+
+ // Test binding works correctly
+ Expression boundBucket = Binder.bind(STRUCT_TYPE, parsedBucket, true);
+ Expression boundDay = Binder.bind(STRUCT_TYPE, parsedDay, true);
+ Expression boundHour = Binder.bind(STRUCT_TYPE, parsedHour, true);
+ Expression boundTruncate = Binder.bind(STRUCT_TYPE, parsedTruncate, true);
+
+ assertThat(boundBucket).isInstanceOf(BoundPredicate.class);
+ assertThat(boundDay).isInstanceOf(BoundPredicate.class);
+ assertThat(boundHour).isInstanceOf(BoundPredicate.class);
+ assertThat(boundTruncate).isInstanceOf(BoundPredicate.class);
+
+ // Verify bound expressions reference correct field IDs
+ BoundPredicate<?> boundBucketPred = (BoundPredicate<?>) boundBucket;
+ BoundPredicate<?> boundDayPred = (BoundPredicate<?>) boundDay;
+ BoundPredicate<?> boundHourPred = (BoundPredicate<?>) boundHour;
+ BoundPredicate<?> boundTruncatePred = (BoundPredicate<?>) boundTruncate;
+
+ assertThat(boundBucketPred.term()).isInstanceOf(BoundTransform.class);
+ assertThat(boundDayPred.term()).isInstanceOf(BoundTransform.class);
+ assertThat(boundHourPred.term()).isInstanceOf(BoundTransform.class);
+ assertThat(boundTruncatePred.term()).isInstanceOf(BoundTransform.class);
+ }
+
+ @Test
+ public void testComplexExpressionsWithResolvedReferenceTransforms() {
+ // Test complex expressions combining transforms created from ResolvedReference
+ Expression complexExpr = Expressions.and(
+ Expressions.or(
+ Expressions.equal(Expressions.bucket(Expressions.ref("id", 100), 8), 3),
+ Expressions.equal(Expressions.day(Expressions.ref("date", 107)), "2023-01-15")),
+ Expressions.and(
+ Expressions.equal(Expressions.truncate(Expressions.ref("data", 101), 4), "test"),
+ Expressions.isNull(Expressions.ref("f", 105))));
+
+ // Test serialization of complex expression
+ String json = ExpressionParser.toJson(complexExpr, true);
+
+ // Verify all transforms and terms are present in JSON
+ assertThat(json).contains("\"transform\" : \"bucket[8]\"");
+ assertThat(json).contains("\"transform\" : \"day\"");
+ assertThat(json).contains("\"transform\" : \"truncate[4]\"");
+ assertThat(json).contains("\"term\" : \"id\"");
+ assertThat(json).contains("\"term\" : \"date\"");
+ assertThat(json).contains("\"term\" : \"data\"");
+ assertThat(json).contains("\"term\" : \"f\"");
+
+ // Test parsing back maintains structure
+ Expression parsed = ExpressionParser.fromJson(json, SCHEMA);
+ assertThat(parsed).isInstanceOf(And.class);
+
+ // Test binding works correctly for the complex expression
+ Expression bound = Binder.bind(STRUCT_TYPE, parsed, true);
+ assertThat(bound).isInstanceOf(And.class);
+
+ // Verify equivalence after round-trip
+ assertThat(ExpressionUtil.equivalent(complexExpr, parsed, STRUCT_TYPE, true)).isTrue();
+
+ // Test that the bound expression maintains correct structure
+ Expression originalBound = Binder.bind(STRUCT_TYPE, complexExpr, true);
+ assertThat(originalBound.toString()).isEqualTo(bound.toString());
+ }
+
+ @Test
+ public void testResolvedReferenceTransformCompatibilityWithNamedReference() {
+ // Test that transforms created with ResolvedReference are equivalent to those created with NamedReference
+
+ // Create equivalent transforms using both approaches
+ Expression bucketWithResolved = Expressions.equal(
+ Expressions.bucket(Expressions.ref("id", 100), 8), 3);
+ Expression bucketWithNamed = Expressions.equal(
+ Expressions.bucket("id", 8), 3);
+
+ Expression dayWithResolved = Expressions.equal(
+ Expressions.day(Expressions.ref("date", 107)), "2023-01-15");
+ Expression dayWithNamed = Expressions.equal(
+ Expressions.day("date"), "2023-01-15");
+
+ // Test that both serialize to the same JSON
+ String resolvedBucketJson = ExpressionParser.toJson(bucketWithResolved, true);
+ String namedBucketJson = ExpressionParser.toJson(bucketWithNamed, true);
+ String resolvedDayJson = ExpressionParser.toJson(dayWithResolved, true);
+ String namedDayJson = ExpressionParser.toJson(dayWithNamed, true);
+
+ assertThat(resolvedBucketJson).isEqualTo(namedBucketJson);
+ assertThat(resolvedDayJson).isEqualTo(namedDayJson);
+
+ // Test that parsing back produces equivalent expressions
+ Expression parsedResolvedBucket = ExpressionParser.fromJson(resolvedBucketJson, SCHEMA);
+ Expression parsedNamedBucket = ExpressionParser.fromJson(namedBucketJson, SCHEMA);
+ Expression parsedResolvedDay = ExpressionParser.fromJson(resolvedDayJson, SCHEMA);
+ Expression parsedNamedDay = ExpressionParser.fromJson(namedDayJson, SCHEMA);
+
+ // All should be equivalent
+ assertThat(ExpressionUtil.equivalent(bucketWithResolved, bucketWithNamed, STRUCT_TYPE, true)).isTrue();
+ assertThat(ExpressionUtil.equivalent(dayWithResolved, dayWithNamed, STRUCT_TYPE, true)).isTrue();
+ assertThat(ExpressionUtil.equivalent(parsedResolvedBucket, parsedNamedBucket, STRUCT_TYPE, true)).isTrue();
+ assertThat(ExpressionUtil.equivalent(parsedResolvedDay, parsedNamedDay, STRUCT_TYPE, true)).isTrue();
+
+ // Test that binding produces identical results
+ Expression boundResolvedBucket = Binder.bind(STRUCT_TYPE, parsedResolvedBucket, true);
+ Expression boundNamedBucket = Binder.bind(STRUCT_TYPE, parsedNamedBucket, true);
+ Expression boundResolvedDay = Binder.bind(STRUCT_TYPE, parsedResolvedDay, true);
+ Expression boundNamedDay = Binder.bind(STRUCT_TYPE, parsedNamedDay, true);
+
+ assertThat(boundResolvedBucket.toString()).isEqualTo(boundNamedBucket.toString());
+ assertThat(boundResolvedDay.toString()).isEqualTo(boundNamedDay.toString());
+ }
+
+ @Test
+ public void testResolvedTransformPreservesFieldIdInformation() {
+ // Test that ResolvedTransform preserves field ID information through binding
+ // This is the key advantage over UnboundTransform with NamedReference
+
+ // Create a transform expression using ResolvedReference
+ ResolvedReference<Long> idRef = Expressions.ref("id", 100);
+ Expression bucketExpr = Expressions.equal(Expressions.bucket(idRef, 8), 3);
+
+ // Verify it's a ResolvedTransform
+ UnboundPredicate<?> predicate = (UnboundPredicate<?>) bucketExpr;
+ assertThat(predicate.term()).isInstanceOf(ResolvedTransform.class);
+
+ ResolvedTransform<?, ?> resolvedTransform = (ResolvedTransform<?, ?>) predicate.term();
+
+ // Verify the ResolvedReference is preserved with field ID
+ assertThat(resolvedTransform.resolvedRef().fieldId()).isEqualTo(100);
+ assertThat(resolvedTransform.resolvedRef().name()).isEqualTo("id");
+
+ // Test binding works correctly with field ID information
+ Expression bound = Binder.bind(STRUCT_TYPE, bucketExpr, true);
+ assertThat(bound).isInstanceOf(BoundPredicate.class);
+
+ BoundPredicate<?> boundPredicate = (BoundPredicate<?>) bound;
+ assertThat(boundPredicate.term()).isInstanceOf(BoundTransform.class);
+
+ BoundTransform<?, ?> boundTransform = (BoundTransform<?, ?>) boundPredicate.term();
+ assertThat(boundTransform.ref().fieldId()).isEqualTo(100);
+ assertThat(boundTransform.ref().name()).isEqualTo("id");
+
+ // Compare with equivalent NamedReference approach to show they bind to same field
+ Expression namedBucketExpr = Expressions.equal(Expressions.bucket("id", 8), 3);
+ Expression boundNamed = Binder.bind(STRUCT_TYPE, namedBucketExpr, true);
+
+ // Both should resolve to the same field ID since they reference the same field
+ BoundPredicate<?> boundNamedPredicate = (BoundPredicate<?>) boundNamed;
+ BoundTransform<?, ?> boundNamedTransform = (BoundTransform<?, ?>) boundNamedPredicate.term();
+
+ assertThat(boundTransform.ref().fieldId()).isEqualTo(boundNamedTransform.ref().fieldId());
+ assertThat(boundTransform.toString()).isEqualTo(boundNamedTransform.toString());
+ }
+
+ @Test
+ public void testResolvedTransformExpressionParserIntegration() {
+ // Test that expressions with ResolvedTransform integrate correctly with ExpressionParser
+
+ // Create expressions using ResolvedTransform
+ Expression bucketExpr = Expressions.equal(
+ Expressions.bucket(Expressions.ref("id", 100), 8), 3);
+ Expression dayExpr = Expressions.equal(
+ Expressions.day(Expressions.ref("date", 107)), "2023-01-15");
+
+ // Test that they can be serialized (even though they become NamedReference in JSON)
+ String bucketJson = ExpressionParser.toJson(bucketExpr, true);
+ String dayJson = ExpressionParser.toJson(dayExpr, true);
+
+ assertThat(bucketJson).contains("\"transform\" : \"bucket[8]\"");
+ assertThat(bucketJson).contains("\"term\" : \"id\"");
+ assertThat(dayJson).contains("\"transform\" : \"day\"");
+ assertThat(dayJson).contains("\"term\" : \"date\"");
+
+ // Test parsing back (will create UnboundTransform with NamedReference)
+ Expression parsedBucket = ExpressionParser.fromJson(bucketJson, SCHEMA);
+ Expression parsedDay = ExpressionParser.fromJson(dayJson, SCHEMA);
+
+ // The parsed expressions will have UnboundTransform (not ResolvedTransform)
+ // but they should still be functionally equivalent when bound
+ UnboundPredicate<?> parsedBucketPred = (UnboundPredicate<?>) parsedBucket;
+ UnboundPredicate<?> parsedDayPred = (UnboundPredicate<?>) parsedDay;
+
+ assertThat(parsedBucketPred.term()).isInstanceOf(UnboundTransform.class);
+ assertThat(parsedDayPred.term()).isInstanceOf(UnboundTransform.class);
+
+ // Both original and parsed should bind to the same fields
+ Expression originalBucketBound = Binder.bind(STRUCT_TYPE, bucketExpr, true);
+ Expression parsedBucketBound = Binder.bind(STRUCT_TYPE, parsedBucket, true);
+ Expression originalDayBound = Binder.bind(STRUCT_TYPE, dayExpr, true);
+ Expression parsedDayBound = Binder.bind(STRUCT_TYPE, parsedDay, true);
+
+ assertThat(originalBucketBound.toString()).isEqualTo(parsedBucketBound.toString());
+ assertThat(originalDayBound.toString()).isEqualTo(parsedDayBound.toString());
+
+ // Test equivalence
+ assertThat(ExpressionUtil.equivalent(bucketExpr, parsedBucket, STRUCT_TYPE, true)).isTrue();
+ assertThat(ExpressionUtil.equivalent(dayExpr, parsedDay, STRUCT_TYPE, true)).isTrue();
+ }
+
+ @Test
+ public void testMixedResolvedTransformAndResolvedReferenceExpressions() {
+ // Test complex expressions mixing ResolvedTransform and direct ResolvedReference
+ Expression complexExpr = Expressions.and(
+ Expressions.equal(Expressions.bucket(Expressions.ref("id", 100), 8), 3),
+ Expressions.or(
+ Expressions.equal(Expressions.ref("data", 101), "test"),
+ Expressions.equal(Expressions.day(Expressions.ref("date", 107)), "2023-01-15")));
+
+ // Verify structure contains both ResolvedTransform and ResolvedReference
+ assertThat(complexExpr).isInstanceOf(And.class);
+ And andExpr = (And) complexExpr;
+
+ // First part should be ResolvedTransform
+ UnboundPredicate<?> bucketPred = (UnboundPredicate<?>) andExpr.left();
+ assertThat(bucketPred.term()).isInstanceOf(ResolvedTransform.class);
+
+ // Second part is OR with ResolvedReference and ResolvedTransform
+ Or orExpr = (Or) andExpr.right();
+ UnboundPredicate<?> dataPred = (UnboundPredicate<?>) orExpr.left();
+ UnboundPredicate<?> dayPred = (UnboundPredicate<?>) orExpr.right();
+
+ assertThat(dataPred.term()).isInstanceOf(ResolvedReference.class);
+ assertThat(dayPred.term()).isInstanceOf(ResolvedTransform.class);
+
+ // Test serialization and parsing
+ String json = ExpressionParser.toJson(complexExpr, true);
+ Expression parsed = ExpressionParser.fromJson(json, SCHEMA);
+
+ // Test binding works correctly
+ Expression bound = Binder.bind(STRUCT_TYPE, parsed, true);
+ Expression originalBound = Binder.bind(STRUCT_TYPE, complexExpr, true);
+
+ assertThat(bound.toString()).isEqualTo(originalBound.toString());
+ assertThat(ExpressionUtil.equivalent(complexExpr, parsed, STRUCT_TYPE, true)).isTrue();
+ }
+
+ @Test
+ public void testBoundExpressionSerializationWithResolvedReference() {
+ // Test that bound expressions serialize to JSON with ResolvedReference terms when includeFieldIds=true
+
+ // Create and bind simple expressions
+ Expression simpleExpr = Expressions.equal(Expressions.ref("id", 100), 42L);
+ Expression boundSimple = Binder.bind(STRUCT_TYPE, simpleExpr, true);
+
+ // Test serialization without field IDs (existing behavior)
+ String jsonWithoutFieldIds = ExpressionParser.toJson(boundSimple, true);
+ assertThat(jsonWithoutFieldIds).contains("\"term\" : \"id\"");
+ assertThat(jsonWithoutFieldIds).doesNotContain("fieldId");
+
+ // Test serialization with field IDs (new behavior)
+ String jsonWithFieldIds = ExpressionParser.toJson(boundSimple, true, true);
+ assertThat(jsonWithFieldIds).contains("\"type\" : \"ref\"");
+ assertThat(jsonWithFieldIds).contains("\"name\" : \"id\"");
+ assertThat(jsonWithFieldIds).contains("\"fieldId\" : 100");
+
+ // Test complex expressions
+ Expression complexExpr = Expressions.and(
+ Expressions.equal(Expressions.ref("data", 101), "test"),
+ Expressions.greaterThan(Expressions.ref("id", 100), 50L));
+ Expression boundComplex = Binder.bind(STRUCT_TYPE, complexExpr, true);
+
+ String complexJsonWithFieldIds = ExpressionParser.toJson(boundComplex, true, true);
+ assertThat(complexJsonWithFieldIds).contains("\"fieldId\" : 100");
+ assertThat(complexJsonWithFieldIds).contains("\"fieldId\" : 101");
+ assertThat(complexJsonWithFieldIds).contains("\"name\" : \"id\"");
+ assertThat(complexJsonWithFieldIds).contains("\"name\" : \"data\"");
+ }
+
+ @Test
+ public void testBoundTransformExpressionSerializationWithResolvedReference() {
+ // Test that bound transform expressions serialize to JSON with ResolvedReference terms when includeFieldIds=true
+
+ // Create transform expressions using ResolvedTransform
+ Expression bucketExpr = Expressions.equal(
+ Expressions.bucket(Expressions.ref("id", 100), 8), 3);
+ Expression dayExpr = Expressions.equal(
+ Expressions.day(Expressions.ref("date", 107)), "2023-01-15");
+
+ // Bind the expressions
+ Expression boundBucket = Binder.bind(STRUCT_TYPE, bucketExpr, true);
+ Expression boundDay = Binder.bind(STRUCT_TYPE, dayExpr, true);
+
+ // Test serialization without field IDs (existing behavior)
+ String bucketJsonNoFieldIds = ExpressionParser.toJson(boundBucket, true);
+ assertThat(bucketJsonNoFieldIds).contains("\"transform\" : \"bucket[8]\"");
+ assertThat(bucketJsonNoFieldIds).contains("\"term\" : \"id\"");
+ assertThat(bucketJsonNoFieldIds).doesNotContain("fieldId");
+
+ // Test serialization with field IDs (new behavior)
+ String bucketJsonWithFieldIds = ExpressionParser.toJson(boundBucket, true, true);
+ assertThat(bucketJsonWithFieldIds).contains("\"transform\" : \"bucket[8]\"");
+ assertThat(bucketJsonWithFieldIds).contains("\"type\" : \"ref\"");
+ assertThat(bucketJsonWithFieldIds).contains("\"name\" : \"id\"");
+ assertThat(bucketJsonWithFieldIds).contains("\"fieldId\" : 100");
+
+ String dayJsonWithFieldIds = ExpressionParser.toJson(boundDay, true, true);
+ assertThat(dayJsonWithFieldIds).contains("\"transform\" : \"day\"");
+ assertThat(dayJsonWithFieldIds).contains("\"type\" : \"ref\"");
+ assertThat(dayJsonWithFieldIds).contains("\"name\" : \"date\"");
+ assertThat(dayJsonWithFieldIds).contains("\"fieldId\" : 107");
+ }
+
+ @Test
+ public void testComplexBoundTransformExpressionSerializationWithResolvedReference() {
+ // Test complex bound expressions with mixed transforms and references
+ Expression complexExpr = Expressions.and(
+ Expressions.or(
+ Expressions.equal(Expressions.bucket(Expressions.ref("id", 100), 8), 3),
+ Expressions.equal(Expressions.day(Expressions.ref("date", 107)), "2023-01-15")),
+ Expressions.and(
+ Expressions.equal(Expressions.truncate(Expressions.ref("data", 101), 4), "test"),
+ Expressions.isNull(Expressions.ref("f", 105))));
+
+ // Bind the complex expression
+ Expression bound = Binder.bind(STRUCT_TYPE, complexExpr, true);
+
+ // Test serialization with field IDs
+ String jsonWithFieldIds = ExpressionParser.toJson(bound, true, true);
+
+ // Verify all field IDs are present
+ assertThat(jsonWithFieldIds).contains("\"fieldId\" : 100"); // id field
+ assertThat(jsonWithFieldIds).contains("\"fieldId\" : 107"); // date field
+ assertThat(jsonWithFieldIds).contains("\"fieldId\" : 101"); // data field
+ assertThat(jsonWithFieldIds).contains("\"fieldId\" : 105"); // f field
+
+ // Verify all field names are present
+ assertThat(jsonWithFieldIds).contains("\"name\" : \"id\"");
+ assertThat(jsonWithFieldIds).contains("\"name\" : \"date\"");
+ assertThat(jsonWithFieldIds).contains("\"name\" : \"data\"");
+ assertThat(jsonWithFieldIds).contains("\"name\" : \"f\"");
+
+ // Verify transforms are serialized correctly
+ assertThat(jsonWithFieldIds).contains("\"transform\" : \"bucket[8]\"");
+ assertThat(jsonWithFieldIds).contains("\"transform\" : \"day\"");
+ assertThat(jsonWithFieldIds).contains("\"transform\" : \"truncate[4]\"");
+
+ // Verify ResolvedReference structure for both transforms and direct references
+ // Count occurrences of "type" : "ref" to ensure all references use ResolvedReference format
+ long refTypeCount = jsonWithFieldIds.lines()
+ .mapToLong(line -> {
+ int count = 0;
+ int index = 0;
+ String pattern = "\"type\" : \"ref\"";
+ while ((index = line.indexOf(pattern, index)) != -1) {
+ count++;
+ index += pattern.length();
+ }
+ return count;
+ })
+ .sum();
+
+ assertThat(refTypeCount).isEqualTo(4); // Should have 4 ResolvedReference objects
+ }
+
+ @Test
+ public void testBoundExpressionRoundTripWithResolvedReference() {
+ // Test that bound expressions with ResolvedReference can be serialized and maintain information
+
+ // Create expressions using both ResolvedTransform and ResolvedReference
+ Expression originalExpr = Expressions.and(
+ Expressions.equal(Expressions.bucket(Expressions.ref("id", 100), 8), 3),
+ Expressions.equal(Expressions.ref("data", 101), "test"));
+
+ // Bind the expression
+ Expression bound = Binder.bind(STRUCT_TYPE, originalExpr, true);
+
+ // Serialize with field IDs
+ String jsonWithFieldIds = ExpressionParser.toJson(bound, true, true);
+
+ // Verify the JSON structure contains complete ResolvedReference information
+ assertThat(jsonWithFieldIds).contains("\"type\" : \"ref\"");
+ assertThat(jsonWithFieldIds).contains("\"fieldId\" : 100");
+ assertThat(jsonWithFieldIds).contains("\"fieldId\" : 101");
+
+ // Note: The current parser doesn't support parsing ResolvedReference format back to expressions
+ // This test documents the serialization capability for external systems that need field ID information
+
+ // Verify that the bound expression maintains the same field IDs after serialization
+ BoundPredicate<?> boundPred = (BoundPredicate<?>) ((And) bound).left();
+ BoundTransform<?, ?> boundTransform = (BoundTransform<?, ?>) boundPred.term();
+ assertThat(boundTransform.ref().fieldId()).isEqualTo(100);
+
+ BoundPredicate<?> boundDataPred = (BoundPredicate<?>) ((And) bound).right();
+ BoundReference<?> boundDataRef = (BoundReference<?>) boundDataPred.term();
+ assertThat(boundDataRef.fieldId()).isEqualTo(101);
+ }
+
+ @Test
+ public void testBoundExpressionFieldIdPreservationAcrossAllTypes() {
+ // Test that all data types preserve field IDs correctly in bound expression serialization
+
+ // Create expressions for fields that support meaningful comparisons
+ Expression[] expressions = new Expression[] {
+ Expressions.equal(Expressions.ref("id", 100), 42L),
+ Expressions.equal(Expressions.ref("data", 101), "test"),
+ Expressions.equal(Expressions.ref("b", 102), true),
+ Expressions.equal(Expressions.ref("i", 103), 42),
+ Expressions.equal(Expressions.ref("l", 104), 42L),
+ Expressions.equal(Expressions.ref("f", 105), 3.14f),
+ Expressions.equal(Expressions.ref("d", 106), 3.14159),
+ Expressions.equal(Expressions.ref("s", 110), "test-string"),
+ Expressions.greaterThan(Expressions.ref("id", 100), 10L)
+ };
+
+ for (Expression expr : expressions) {
+ // Bind each expression
+ Expression bound = Binder.bind(STRUCT_TYPE, expr, true);
+
+ // Serialize with field IDs
+ String json = ExpressionParser.toJson(bound, true, true);
+
+ // Extract expected field ID from the original ResolvedReference
+ UnboundPredicate<?> unboundPred = (UnboundPredicate<?>) expr;
+ ResolvedReference<?> resolvedRef = (ResolvedReference<?>) unboundPred.term();
+ int expectedFieldId = resolvedRef.fieldId();
+
+ // Verify the field ID is preserved in the JSON
+ assertThat(json).contains("\"fieldId\" : " + expectedFieldId);
+ assertThat(json).contains("\"type\" : \"ref\"");
+ }
+ }
+}
\ No newline at end of file
(iceberg) 01/09: POC: Try Resolved Reference
Posted by si...@apache.org.
This is an automated email from the ASF dual-hosted git repository.
singhpk234 pushed a commit to branch feature/serialize-bound-expression
in repository https://gitbox.apache.org/repos/asf/iceberg.git
commit 91a7baeb5216451e785df607b0bb3a6f82544c86
Author: Prashant Singh <pr...@snowflake.com>
AuthorDate: Thu Sep 25 18:20:49 2025 -0700
POC: Try Resolved Reference
---
.../apache/iceberg/expressions/Expressions.java | 15 +++++
.../iceberg/expressions/ResolvedReference.java | 64 ++++++++++++++++++++++
2 files changed, 79 insertions(+)
diff --git a/api/src/main/java/org/apache/iceberg/expressions/Expressions.java b/api/src/main/java/org/apache/iceberg/expressions/Expressions.java
index 78012def5a..1e93d8e75e 100644
--- a/api/src/main/java/org/apache/iceberg/expressions/Expressions.java
+++ b/api/src/main/java/org/apache/iceberg/expressions/Expressions.java
@@ -305,6 +305,21 @@ public class Expressions {
return new NamedReference<>(name);
}
+ /**
+ * Constructs a resolved reference for a given column.
+ *
+ * <p>The following are equivalent: equals("a", 5) and if a fieldId is 1 and equals(ref("a", 1),
+ * 5).
+ *
+ * @param name a column name
+ * @param fieldId the field ID of the column
+ * @param <T> the Java type of this reference
+ * @return a named reference
+ */
+ public static <T> ResolvedReference<T> ref(String name, int fieldId) {
+ return new ResolvedReference<>(name, fieldId);
+ }
+
/**
* Constructs a transform expression for a given column.
*
diff --git a/api/src/main/java/org/apache/iceberg/expressions/ResolvedReference.java b/api/src/main/java/org/apache/iceberg/expressions/ResolvedReference.java
new file mode 100644
index 0000000000..b12bff5832
--- /dev/null
+++ b/api/src/main/java/org/apache/iceberg/expressions/ResolvedReference.java
@@ -0,0 +1,64 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.apache.iceberg.expressions;
+
+import org.apache.iceberg.Schema;
+import org.apache.iceberg.exceptions.ValidationException;
+import org.apache.iceberg.types.Types;
+
+public class ResolvedReference<T> implements UnboundTerm<T>, Reference<T> {
+ private final String name;
+ private final int fieldId;
+
+ public ResolvedReference(String name, int fieldId) {
+ this.name = name;
+ this.fieldId = fieldId;
+ }
+
+ @Override
+ public String name() {
+ return name;
+ }
+
+ public int fieldId() {
+ return fieldId;
+ }
+
+ @Override
+ public BoundTerm<T> bind(Types.StructType struct, boolean caseSensitive) {
+ // assumption is that we always have the field id
+ Schema schema = struct.asSchema();
+ // we ignore case sensitivity here because field ids are unique
+ Types.NestedField field = schema.findField(fieldId);
+ ValidationException.check(
+ field != null, "Cannot find field '%s' in struct: %s", name, schema.asStruct());
+
+ return new BoundReference<>(field, schema.accessorForField(field.fieldId()), name);
+ }
+
+ @Override
+ public NamedReference<?> ref() {
+ return new NamedReference<>(name);
+ }
+
+ @Override
+ public String toString() {
+ return String.format("ref(name=\"%s\", fieldId=\"%s\")", name, fieldId);
+ }
+}
(iceberg) 09/09: Address review feedback
Posted by si...@apache.org.
This is an automated email from the ASF dual-hosted git repository.
singhpk234 pushed a commit to branch feature/serialize-bound-expression
in repository https://gitbox.apache.org/repos/asf/iceberg.git
commit 67f0039c4a004156dc1476ee4512f6c4f2f81f67
Author: Prashant Singh <pr...@snowflake.com>
AuthorDate: Wed Oct 22 17:33:32 2025 -0700
Address review feedback
---
.../apache/iceberg/expressions/ExpressionUtil.java | 2 --
.../apache/iceberg/expressions/Expressions.java | 14 ++++----
.../apache/iceberg/expressions/NamedReference.java | 10 +++---
.../iceberg/expressions/ResolvedReference.java | 38 +++++-----------------
.../iceberg/expressions/TestResolvedReference.java | 35 ++++++++++----------
open-api/rest-catalog-open-api.yaml | 15 +++++++++
6 files changed, 53 insertions(+), 61 deletions(-)
diff --git a/api/src/main/java/org/apache/iceberg/expressions/ExpressionUtil.java b/api/src/main/java/org/apache/iceberg/expressions/ExpressionUtil.java
index 6450321eda..d3dc00d914 100644
--- a/api/src/main/java/org/apache/iceberg/expressions/ExpressionUtil.java
+++ b/api/src/main/java/org/apache/iceberg/expressions/ExpressionUtil.java
@@ -232,8 +232,6 @@ public class ExpressionUtil {
+ ")";
} else if (term instanceof NamedReference) {
return ((NamedReference<?>) term).name();
- } else if (term instanceof ResolvedReference) {
- return ((ResolvedReference<?>) term).name();
} else if (term instanceof BoundReference) {
return ((BoundReference<?>) term).name();
} else {
diff --git a/api/src/main/java/org/apache/iceberg/expressions/Expressions.java b/api/src/main/java/org/apache/iceberg/expressions/Expressions.java
index 72c17d8570..04f5caa357 100644
--- a/api/src/main/java/org/apache/iceberg/expressions/Expressions.java
+++ b/api/src/main/java/org/apache/iceberg/expressions/Expressions.java
@@ -103,33 +103,33 @@ public class Expressions {
}
@SuppressWarnings("unchecked")
- public static <S, T> UnboundTerm<T> bucket(ResolvedReference<S> resolvedRef, int numBuckets) {
+ public static <S, T> UnboundTerm<T> bucket(NamedReference<S> resolvedRef, int numBuckets) {
Transform<S, T> transform = (Transform<S, T>) Transforms.bucket(numBuckets);
return new UnboundTransform<>(resolvedRef, transform);
}
@SuppressWarnings("unchecked")
- public static <S, T> UnboundTerm<T> year(ResolvedReference<S> resolvedRef) {
+ public static <S, T> UnboundTerm<T> year(NamedReference<S> resolvedRef) {
return new UnboundTransform<>(resolvedRef, (Transform<S, T>) Transforms.year());
}
@SuppressWarnings("unchecked")
- public static <S, T> UnboundTerm<T> month(ResolvedReference<S> resolvedRef) {
+ public static <S, T> UnboundTerm<T> month(NamedReference<S> resolvedRef) {
return new UnboundTransform<>(resolvedRef, (Transform<S, T>) Transforms.month());
}
@SuppressWarnings("unchecked")
- public static <S, T> UnboundTerm<T> day(ResolvedReference<S> resolvedRef) {
+ public static <S, T> UnboundTerm<T> day(NamedReference<S> resolvedRef) {
return new UnboundTransform<>(resolvedRef, (Transform<S, T>) Transforms.day());
}
@SuppressWarnings("unchecked")
- public static <S, T> UnboundTerm<T> hour(ResolvedReference<S> resolvedRef) {
+ public static <S, T> UnboundTerm<T> hour(NamedReference<S> resolvedRef) {
return new UnboundTransform<>(resolvedRef, (Transform<S, T>) Transforms.hour());
}
@SuppressWarnings("unchecked")
- public static <S, T> UnboundTerm<T> truncate(ResolvedReference<S> resolvedRef, int width) {
+ public static <S, T> UnboundTerm<T> truncate(NamedReference<S> resolvedRef, int width) {
Transform<S, T> transform = (Transform<S, T>) Transforms.truncate(width);
return new UnboundTransform<>(resolvedRef, transform);
}
@@ -348,7 +348,7 @@ public class Expressions {
* @param <T> the Java type of this reference
* @return a named reference
*/
- public static <T> ResolvedReference<T> ref(String name, int fieldId) {
+ static <T> ResolvedReference<T> ref(String name, int fieldId) {
return new ResolvedReference<>(name, fieldId);
}
diff --git a/api/src/main/java/org/apache/iceberg/expressions/NamedReference.java b/api/src/main/java/org/apache/iceberg/expressions/NamedReference.java
index b435e2bed5..fdc6b700a3 100644
--- a/api/src/main/java/org/apache/iceberg/expressions/NamedReference.java
+++ b/api/src/main/java/org/apache/iceberg/expressions/NamedReference.java
@@ -38,16 +38,16 @@ public class NamedReference<T> implements UnboundTerm<T>, Reference<T> {
@Override
public BoundReference<T> bind(Types.StructType struct, boolean caseSensitive) {
- ValidationException.check(name() != null, "Name cannot be null");
+ ValidationException.check(name != null, "Name cannot be null");
Schema schema = struct.asSchema();
Types.NestedField field =
- caseSensitive ? schema.findField(name()) : schema.caseInsensitiveFindField(name());
+ caseSensitive ? schema.findField(name) : schema.caseInsensitiveFindField(name);
ValidationException.check(
- field != null, "Cannot find field '%s' in struct: %s", name(), schema.asStruct());
+ field != null, "Cannot find field '%s' in struct: %s", name, schema.asStruct());
- return new BoundReference<>(field, schema.accessorForField(field.fieldId()), name());
+ return new BoundReference<>(field, schema.accessorForField(field.fieldId()), name);
}
@Override
@@ -57,6 +57,6 @@ public class NamedReference<T> implements UnboundTerm<T>, Reference<T> {
@Override
public String toString() {
- return String.format("ref(name=\"%s\")", name());
+ return String.format("ref(name=\"%s\")", name);
}
}
diff --git a/api/src/main/java/org/apache/iceberg/expressions/ResolvedReference.java b/api/src/main/java/org/apache/iceberg/expressions/ResolvedReference.java
index bc206b76a4..d92b2600c5 100644
--- a/api/src/main/java/org/apache/iceberg/expressions/ResolvedReference.java
+++ b/api/src/main/java/org/apache/iceberg/expressions/ResolvedReference.java
@@ -22,6 +22,13 @@ import org.apache.iceberg.Schema;
import org.apache.iceberg.exceptions.ValidationException;
import org.apache.iceberg.types.Types;
+/**
+ * A reference to a field by ID rather than name. This extends NamedReference because it is a more
+ * specific in that the name has already been resolved to a field ID. Because the name has been
+ * resolved, the name is informational.
+ *
+ * @param <T>
+ */
public class ResolvedReference<T> extends NamedReference<T> {
private final int fieldId;
@@ -39,40 +46,13 @@ public class ResolvedReference<T> extends NamedReference<T> {
Schema schema = struct.asSchema();
Types.NestedField field = schema.findField(fieldId);
ValidationException.check(
- field != null,
- "Cannot find field with id '%s' in struct: %s, since we are resolving based on ID",
- fieldId,
- schema.asStruct());
+ field != null, "Cannot find field by id %s in struct: %s", fieldId, schema.asStruct());
return new BoundReference<>(field, schema.accessorForField(field.fieldId()), name());
}
- @Override
- public NamedReference<T> ref() {
- return new NamedReference<>(name());
- }
-
- @Override
- public boolean equals(Object o) {
- if (this == o) {
- return true;
- }
-
- if (o == null || getClass() != o.getClass()) {
- return false;
- }
-
- ResolvedReference<?> that = (ResolvedReference<?>) o;
- return fieldId == that.fieldId && name().equals(that.name());
- }
-
- @Override
- public int hashCode() {
- return 31 * fieldId + name().hashCode();
- }
-
@Override
public String toString() {
- return String.format("ref(name=\"%s\", fieldId=\"%s\")", name(), fieldId);
+ return String.format("ref(name=\"%s\", id=\"%s\")", name(), fieldId);
}
}
diff --git a/api/src/test/java/org/apache/iceberg/expressions/TestResolvedReference.java b/api/src/test/java/org/apache/iceberg/expressions/TestResolvedReference.java
index 9a9d84db1f..661a21429d 100644
--- a/api/src/test/java/org/apache/iceberg/expressions/TestResolvedReference.java
+++ b/api/src/test/java/org/apache/iceberg/expressions/TestResolvedReference.java
@@ -40,11 +40,12 @@ public class TestResolvedReference {
ResolvedReference<Integer> ref4 = new ResolvedReference<>("a", 35);
// Equal references
- assertThat(ref1).isEqualTo(ref2);
- assertThat(ref1.hashCode()).isEqualTo(ref2.hashCode());
+ assertThat(ref1.fieldId()).isEqualTo(ref2.fieldId());
+ assertThat(ref1.name()).isEqualTo(ref2.name());
// Different names, same fieldId
- assertThat(ref1).isNotEqualTo(ref3);
+ assertThat(ref1.fieldId()).isEqualTo(ref3.fieldId());
+ assertThat(ref1.name()).isNotEqualTo(ref3.name());
// Same name, different fieldId
assertThat(ref1).isNotEqualTo(ref4);
@@ -53,13 +54,12 @@ public class TestResolvedReference {
@Test
public void testResolvedReferenceBind() {
ResolvedReference<Integer> ref = new ResolvedReference<>("a", 34);
- BoundTerm<Integer> bound = ref.bind(SCHEMA.asStruct(), true);
+ BoundReference<Integer> bound = ref.bind(SCHEMA.asStruct(), true);
assertThat(bound).isInstanceOf(BoundReference.class);
- BoundReference<Integer> boundRef = (BoundReference<Integer>) bound;
- assertThat(boundRef.fieldId()).isEqualTo(34);
- assertThat(boundRef.name()).isEqualTo("a");
- assertThat(boundRef.type()).isEqualTo(Types.IntegerType.get());
+ assertThat(bound.fieldId()).isEqualTo(34);
+ assertThat(bound.name()).isEqualTo("a");
+ assertThat(bound.type()).isEqualTo(Types.IntegerType.get());
}
@Test
@@ -67,13 +67,13 @@ public class TestResolvedReference {
ResolvedReference<Integer> ref = new ResolvedReference<>("A", 34);
// Should work regardless of case sensitivity since we use fieldId
- BoundTerm<Integer> bound1 = ref.bind(SCHEMA.asStruct(), true);
- BoundTerm<Integer> bound2 = ref.bind(SCHEMA.asStruct(), false);
+ BoundReference<Integer> bound1 = ref.bind(SCHEMA.asStruct(), true);
+ BoundReference<Integer> bound2 = ref.bind(SCHEMA.asStruct(), false);
assertThat(bound1).isInstanceOf(BoundReference.class);
assertThat(bound2).isInstanceOf(BoundReference.class);
- assertThat(((BoundReference<Integer>) bound1).fieldId()).isEqualTo(34);
- assertThat(((BoundReference<Integer>) bound2).fieldId()).isEqualTo(34);
+ assertThat(bound1.fieldId()).isEqualTo(34);
+ assertThat(bound2.fieldId()).isEqualTo(34);
}
@Test
@@ -83,7 +83,7 @@ public class TestResolvedReference {
assertThatThrownBy(() -> ref.bind(SCHEMA.asStruct(), true))
.isInstanceOf(ValidationException.class)
.hasMessageContaining(
- "Cannot find field with id '999' in struct: struct<34: a: optional int, 35: s: required string>, since we are resolving based on ID");
+ "Cannot find field by id 999 in struct: struct<34: a: optional int, 35: s: required string>");
}
@Test
@@ -98,19 +98,18 @@ public class TestResolvedReference {
public void testResolvedReferenceToString() {
ResolvedReference<Integer> ref = new ResolvedReference<>("a", 34);
- assertThat(ref.toString()).isEqualTo("ref(name=\"a\", fieldId=\"34\")");
+ assertThat(ref.toString()).isEqualTo("ref(name=\"a\", id=\"34\")");
}
@Test
public void testResolvedReferenceExpressionIntegration() {
// Test that ResolvedReference works in expression predicates
- Expression expr = Expressions.equal(Expressions.ref("a", 34), 5);
+ UnboundPredicate<?> expr = Expressions.equal(Expressions.ref("a", 34), 5);
assertThat(expr).isInstanceOf(UnboundPredicate.class);
- UnboundPredicate<?> predicate = (UnboundPredicate<?>) expr;
- assertThat(predicate.term()).isInstanceOf(ResolvedReference.class);
+ assertThat(expr.term()).isInstanceOf(ResolvedReference.class);
- ResolvedReference<?> resolvedRef = (ResolvedReference<?>) predicate.term();
+ ResolvedReference<?> resolvedRef = (ResolvedReference<?>) expr.term();
assertThat(resolvedRef.name()).isEqualTo("a");
assertThat(resolvedRef.fieldId()).isEqualTo(34);
}
diff --git a/open-api/rest-catalog-open-api.yaml b/open-api/rest-catalog-open-api.yaml
index e0b524a6d7..a5571ef15a 100644
--- a/open-api/rest-catalog-open-api.yaml
+++ b/open-api/rest-catalog-open-api.yaml
@@ -2290,10 +2290,25 @@ components:
- $ref: '#/components/schemas/TransformTerm'
Reference:
+ oneOf:
+ - $ref: '#/components/schemas/NamedReference'
+ - $ref: '#/components/schemas/ResolvedReference'
+
+ NamedReference:
type: string
example:
- "column-name"
+ ResolvedReference:
+ allOf:
+ - $ref: '#/components/schemas/NamedReference'
+ - type: object
+ required:
+ - field-id
+ properties:
+ field-id:
+ type: integer
+
TransformTerm:
type: object
required:
(iceberg) 08/09: more cleanup
Posted by si...@apache.org.
This is an automated email from the ASF dual-hosted git repository.
singhpk234 pushed a commit to branch feature/serialize-bound-expression
in repository https://gitbox.apache.org/repos/asf/iceberg.git
commit a2499368221549cf6596a2bcd8cda14e03ea0ae6
Author: Prashant Kumar Singh <pr...@snowflake.com>
AuthorDate: Wed Oct 22 19:20:09 2025 +0000
more cleanup
---
.../apache/iceberg/expressions/NamedReference.java | 2 +
.../iceberg/expressions/UnboundTransform.java | 11 +--
.../iceberg/expressions/ExpressionParser.java | 82 +++++++++++++---
.../TestExpressionParserWithResolvedReference.java | 107 ++++++++++++++++-----
4 files changed, 155 insertions(+), 47 deletions(-)
diff --git a/api/src/main/java/org/apache/iceberg/expressions/NamedReference.java b/api/src/main/java/org/apache/iceberg/expressions/NamedReference.java
index c2d83bc4df..b435e2bed5 100644
--- a/api/src/main/java/org/apache/iceberg/expressions/NamedReference.java
+++ b/api/src/main/java/org/apache/iceberg/expressions/NamedReference.java
@@ -38,6 +38,8 @@ public class NamedReference<T> implements UnboundTerm<T>, Reference<T> {
@Override
public BoundReference<T> bind(Types.StructType struct, boolean caseSensitive) {
+ ValidationException.check(name() != null, "Name cannot be null");
+
Schema schema = struct.asSchema();
Types.NestedField field =
caseSensitive ? schema.findField(name()) : schema.caseInsensitiveFindField(name());
diff --git a/api/src/main/java/org/apache/iceberg/expressions/UnboundTransform.java b/api/src/main/java/org/apache/iceberg/expressions/UnboundTransform.java
index 27059d1d21..30f03e4a9e 100644
--- a/api/src/main/java/org/apache/iceberg/expressions/UnboundTransform.java
+++ b/api/src/main/java/org/apache/iceberg/expressions/UnboundTransform.java
@@ -36,17 +36,13 @@ public class UnboundTransform<S, T> implements UnboundTerm<T>, Term {
return ref;
}
- public NamedReference<S> unboundRef() {
- return ref;
- }
-
public Transform<S, T> transform() {
return transform;
}
@Override
public BoundTransform<S, T> bind(Types.StructType struct, boolean caseSensitive) {
- BoundReference<S> boundRef = (BoundReference<S>) ref.bind(struct, caseSensitive);
+ BoundReference<S> boundRef = ref.bind(struct, caseSensitive);
try {
ValidationException.check(
@@ -54,11 +50,10 @@ public class UnboundTransform<S, T> implements UnboundTerm<T>, Term {
"Cannot bind: %s cannot transform %s values from '%s'",
transform,
boundRef.type(),
- ref.toString());
+ ref);
} catch (IllegalArgumentException e) {
throw new ValidationException(
- "Cannot bind: %s cannot transform %s values from '%s'",
- transform, boundRef.type(), ref.name());
+ "Cannot bind: %s cannot transform %s values from '%s'", transform, boundRef.type(), ref);
}
return new BoundTransform<>(boundRef, transform);
diff --git a/core/src/main/java/org/apache/iceberg/expressions/ExpressionParser.java b/core/src/main/java/org/apache/iceberg/expressions/ExpressionParser.java
index e68a71c4a7..6aea3c6e60 100644
--- a/core/src/main/java/org/apache/iceberg/expressions/ExpressionParser.java
+++ b/core/src/main/java/org/apache/iceberg/expressions/ExpressionParser.java
@@ -34,6 +34,7 @@ import org.apache.iceberg.SingleValueParser;
import org.apache.iceberg.relocated.com.google.common.base.Preconditions;
import org.apache.iceberg.relocated.com.google.common.collect.ImmutableList;
import org.apache.iceberg.relocated.com.google.common.collect.Iterables;
+import org.apache.iceberg.transforms.Transform;
import org.apache.iceberg.transforms.Transforms;
import org.apache.iceberg.types.Types;
import org.apache.iceberg.util.JsonUtil;
@@ -51,6 +52,11 @@ public class ExpressionParser {
private static final String REFERENCE = "reference";
private static final String LITERAL = "literal";
+ public enum SerializationMode {
+ NAMES_ONLY,
+ WITH_FIELD_IDS
+ }
+
private ExpressionParser() {}
public static String toJson(Expression expression) {
@@ -62,27 +68,45 @@ public class ExpressionParser {
return JsonUtil.generate(gen -> toJson(expression, gen), pretty);
}
+ public static String toJson(Expression expression, boolean pretty, SerializationMode mode) {
+ Preconditions.checkArgument(expression != null, "Invalid expression: null");
+ return JsonUtil.generate(gen -> toJson(expression, gen, mode), pretty);
+ }
+
+ @Deprecated
public static String toJson(Expression expression, boolean pretty, boolean includeFieldIds) {
Preconditions.checkArgument(expression != null, "Invalid expression: null");
- return JsonUtil.generate(gen -> toJson(expression, gen, includeFieldIds), pretty);
+ return toJson(
+ expression,
+ pretty,
+ includeFieldIds ? SerializationMode.WITH_FIELD_IDS : SerializationMode.NAMES_ONLY);
}
public static void toJson(Expression expression, JsonGenerator gen) {
- ExpressionVisitors.visit(expression, new JsonGeneratorVisitor(gen, false));
+ ExpressionVisitors.visit(
+ expression, new JsonGeneratorVisitor(gen, SerializationMode.NAMES_ONLY));
+ }
+
+ public static void toJson(Expression expression, JsonGenerator gen, SerializationMode mode) {
+ ExpressionVisitors.visit(expression, new JsonGeneratorVisitor(gen, mode));
}
+ @Deprecated
public static void toJson(Expression expression, JsonGenerator gen, boolean includeFieldIds) {
- ExpressionVisitors.visit(expression, new JsonGeneratorVisitor(gen, includeFieldIds));
+ toJson(
+ expression,
+ gen,
+ includeFieldIds ? SerializationMode.WITH_FIELD_IDS : SerializationMode.NAMES_ONLY);
}
private static class JsonGeneratorVisitor
extends ExpressionVisitors.CustomOrderExpressionVisitor<Void> {
private final JsonGenerator gen;
- private final boolean includeFieldIds;
+ private final SerializationMode mode;
- private JsonGeneratorVisitor(JsonGenerator gen, boolean includeFieldIds) {
+ private JsonGeneratorVisitor(JsonGenerator gen, SerializationMode mode) {
this.gen = gen;
- this.includeFieldIds = includeFieldIds;
+ this.mode = mode;
}
/**
@@ -251,7 +275,7 @@ public class ExpressionParser {
return;
} else if (term instanceof BoundTransform) {
BoundTransform<?, ?> transform = (BoundTransform<?, ?>) term;
- if (includeFieldIds) {
+ if (mode == SerializationMode.WITH_FIELD_IDS) {
transformWithFieldId(
transform.transform().toString(), transform.ref().name(), transform.ref().fieldId());
} else {
@@ -260,7 +284,7 @@ public class ExpressionParser {
return;
} else if (term instanceof BoundReference) {
BoundReference<?> ref = (BoundReference<?>) term;
- if (includeFieldIds) {
+ if (mode == SerializationMode.WITH_FIELD_IDS) {
referenceWithFieldId(ref.name(), ref.fieldId());
} else {
gen.writeString(ref.name());
@@ -295,8 +319,11 @@ public class ExpressionParser {
private void referenceWithFieldId(String name, int fieldId) throws IOException {
gen.writeStartObject();
- gen.writeStringField("type", "ref");
- gen.writeStringField("name", name);
+ gen.writeStringField("type", REFERENCE);
+ // Only write name if not null (support fieldId-only case)
+ if (name != null) {
+ gen.writeStringField("name", name);
+ }
gen.writeNumberField("fieldId", fieldId);
gen.writeEndObject();
}
@@ -451,12 +478,39 @@ public class ExpressionParser {
String type = JsonUtil.getString(TYPE, node);
switch (type) {
case REFERENCE:
- return Expressions.ref(JsonUtil.getString(TERM, node));
+ // Reference must have a name (via "name" or "term")
+ // fieldId is optional - if present, creates ResolvedReference, otherwise NamedReference
+ boolean hasTerm = node.has(TERM);
+ boolean hasName = node.has("name");
+ boolean hasFieldId = node.has("fieldId");
+
+ String name =
+ hasName
+ ? JsonUtil.getString("name", node)
+ : hasTerm ? JsonUtil.getString(TERM, node) : null;
+
+ if (name == null) {
+ throw new IllegalArgumentException(
+ "REFERENCE must have a name (via 'name' or 'term' field): " + node);
+ }
+
+ if (hasFieldId) {
+ int fieldId = JsonUtil.getInt("fieldId", node);
+ return Expressions.ref(name, fieldId);
+ } else {
+ return Expressions.ref(name);
+ }
case TRANSFORM:
UnboundTerm<T> child = term(JsonUtil.get(TERM, node));
- String transform = JsonUtil.getString(TRANSFORM, node);
- return (UnboundTerm<T>)
- Expressions.transform(child.ref().name(), Transforms.fromString(transform));
+ String transformStr = JsonUtil.getString(TRANSFORM, node);
+ NamedReference<?> ref;
+ if (child instanceof NamedReference) {
+ ref = (NamedReference<?>) child;
+ } else {
+ ref = child.ref();
+ }
+ Transform<?, ?> transform = Transforms.fromString(transformStr);
+ return (UnboundTerm<T>) new UnboundTransform(ref, transform);
default:
throw new IllegalArgumentException("Cannot parse type as a reference: " + type);
}
diff --git a/core/src/test/java/org/apache/iceberg/expressions/TestExpressionParserWithResolvedReference.java b/core/src/test/java/org/apache/iceberg/expressions/TestExpressionParserWithResolvedReference.java
index ff04145917..bff158a2a1 100644
--- a/core/src/test/java/org/apache/iceberg/expressions/TestExpressionParserWithResolvedReference.java
+++ b/core/src/test/java/org/apache/iceberg/expressions/TestExpressionParserWithResolvedReference.java
@@ -25,6 +25,7 @@ import static org.assertj.core.api.Assertions.assertThat;
import java.math.BigDecimal;
import java.util.UUID;
import org.apache.iceberg.Schema;
+import org.apache.iceberg.expressions.ExpressionParser.SerializationMode;
import org.apache.iceberg.types.Types;
import org.junit.jupiter.api.Test;
@@ -499,8 +500,9 @@ public class TestExpressionParserWithResolvedReference {
assertThat(jsonWithoutFieldIds).doesNotContain("fieldId");
// Test serialization with field IDs (new behavior)
- String jsonWithFieldIds = ExpressionParser.toJson(boundSimple, true, true);
- assertThat(jsonWithFieldIds).contains("\"type\" : \"ref\"");
+ String jsonWithFieldIds =
+ ExpressionParser.toJson(boundSimple, true, SerializationMode.WITH_FIELD_IDS);
+ assertThat(jsonWithFieldIds).contains("\"type\" : \"reference\"");
assertThat(jsonWithFieldIds).contains("\"name\" : \"id\"");
assertThat(jsonWithFieldIds).contains("\"fieldId\" : 100");
@@ -511,7 +513,8 @@ public class TestExpressionParserWithResolvedReference {
Expressions.greaterThan(Expressions.ref("id", 100), 50L));
Expression boundComplex = Binder.bind(STRUCT_TYPE, complexExpr, true);
- String complexJsonWithFieldIds = ExpressionParser.toJson(boundComplex, true, true);
+ String complexJsonWithFieldIds =
+ ExpressionParser.toJson(boundComplex, true, SerializationMode.WITH_FIELD_IDS);
assertThat(complexJsonWithFieldIds).contains("\"fieldId\" : 100");
assertThat(complexJsonWithFieldIds).contains("\"fieldId\" : 101");
assertThat(complexJsonWithFieldIds).contains("\"name\" : \"id\"");
@@ -539,15 +542,17 @@ public class TestExpressionParserWithResolvedReference {
assertThat(bucketJsonNoFieldIds).doesNotContain("fieldId");
// Test serialization with field IDs (new behavior)
- String bucketJsonWithFieldIds = ExpressionParser.toJson(boundBucket, true, true);
+ String bucketJsonWithFieldIds =
+ ExpressionParser.toJson(boundBucket, true, SerializationMode.WITH_FIELD_IDS);
assertThat(bucketJsonWithFieldIds).contains("\"transform\" : \"bucket[8]\"");
- assertThat(bucketJsonWithFieldIds).contains("\"type\" : \"ref\"");
+ assertThat(bucketJsonWithFieldIds).contains("\"type\" : \"reference\"");
assertThat(bucketJsonWithFieldIds).contains("\"name\" : \"id\"");
assertThat(bucketJsonWithFieldIds).contains("\"fieldId\" : 100");
- String dayJsonWithFieldIds = ExpressionParser.toJson(boundDay, true, true);
+ String dayJsonWithFieldIds =
+ ExpressionParser.toJson(boundDay, true, SerializationMode.WITH_FIELD_IDS);
assertThat(dayJsonWithFieldIds).contains("\"transform\" : \"day\"");
- assertThat(dayJsonWithFieldIds).contains("\"type\" : \"ref\"");
+ assertThat(dayJsonWithFieldIds).contains("\"type\" : \"reference\"");
assertThat(dayJsonWithFieldIds).contains("\"name\" : \"date\"");
assertThat(dayJsonWithFieldIds).contains("\"fieldId\" : 107");
}
@@ -568,7 +573,8 @@ public class TestExpressionParserWithResolvedReference {
Expression bound = Binder.bind(STRUCT_TYPE, complexExpr, true);
// Test serialization with field IDs
- String jsonWithFieldIds = ExpressionParser.toJson(bound, true, true);
+ String jsonWithFieldIds =
+ ExpressionParser.toJson(bound, true, SerializationMode.WITH_FIELD_IDS);
// Verify all field IDs are present
assertThat(jsonWithFieldIds).contains("\"fieldId\" : 100"); // id field
@@ -588,7 +594,8 @@ public class TestExpressionParserWithResolvedReference {
assertThat(jsonWithFieldIds).contains("\"transform\" : \"truncate[4]\"");
// Verify ResolvedReference structure for both transforms and direct references
- // Count occurrences of "type" : "ref" to ensure all references use ResolvedReference format
+ // Count occurrences of "type" : "reference" to ensure all references use ResolvedReference
+ // format
long refTypeCount =
jsonWithFieldIds
.lines()
@@ -596,7 +603,7 @@ public class TestExpressionParserWithResolvedReference {
line -> {
int count = 0;
int index = 0;
- String pattern = "\"type\" : \"ref\"";
+ String pattern = "\"type\" : \"reference\"";
while ((index = line.indexOf(pattern, index)) != -1) {
count++;
index += pattern.length();
@@ -610,7 +617,7 @@ public class TestExpressionParserWithResolvedReference {
@Test
public void testBoundExpressionRoundTripWithResolvedReference() {
- // Test that bound expressions with ResolvedReference can be serialized and maintain information
+ // Test that bound expressions with ResolvedReference can be serialized and parsed back
// Create expressions using both ResolvedTransform and ResolvedReference
Expression originalExpr =
@@ -622,25 +629,75 @@ public class TestExpressionParserWithResolvedReference {
Expression bound = Binder.bind(STRUCT_TYPE, originalExpr, true);
// Serialize with field IDs
- String jsonWithFieldIds = ExpressionParser.toJson(bound, true, true);
+ String jsonWithFieldIds =
+ ExpressionParser.toJson(bound, true, SerializationMode.WITH_FIELD_IDS);
// Verify the JSON structure contains complete ResolvedReference information
- assertThat(jsonWithFieldIds).contains("\"type\" : \"ref\"");
+ assertThat(jsonWithFieldIds).contains("\"type\" : \"reference\"");
assertThat(jsonWithFieldIds).contains("\"fieldId\" : 100");
assertThat(jsonWithFieldIds).contains("\"fieldId\" : 101");
- // Note: The current parser doesn't support parsing ResolvedReference format back to expressions
- // This test documents the serialization capability for external systems that need field ID
- // information
+ // Parse back from JSON with field IDs
+ Expression parsed = ExpressionParser.fromJson(jsonWithFieldIds, SCHEMA);
- // Verify that the bound expression maintains the same field IDs after serialization
- BoundPredicate<?> boundPred = (BoundPredicate<?>) ((And) bound).left();
- BoundTransform<?, ?> boundTransform = (BoundTransform<?, ?>) boundPred.term();
- assertThat(boundTransform.ref().fieldId()).isEqualTo(100);
+ // Verify parsed expression uses ResolvedReference
+ assertThat(parsed).isInstanceOf(And.class);
+ And parsedAnd = (And) parsed;
+
+ UnboundPredicate<?> leftPred = (UnboundPredicate<?>) parsedAnd.left();
+ assertThat(leftPred.term()).isInstanceOf(UnboundTransform.class);
+ UnboundTransform<?, ?> leftTransform = (UnboundTransform<?, ?>) leftPred.term();
+ assertThat(leftTransform.ref()).isInstanceOf(ResolvedReference.class);
+ assertThat(((ResolvedReference<?>) leftTransform.ref()).fieldId()).isEqualTo(100);
+
+ UnboundPredicate<?> rightPred = (UnboundPredicate<?>) parsedAnd.right();
+ assertThat(rightPred.term()).isInstanceOf(ResolvedReference.class);
+ ResolvedReference<?> rightRef = (ResolvedReference<?>) rightPred.term();
+ assertThat(rightRef.fieldId()).isEqualTo(101);
+ assertThat(rightRef.name()).isEqualTo("data");
+
+ // Verify that binding the parsed expression produces the same result
+ Expression parsedBound = Binder.bind(STRUCT_TYPE, parsed, true);
+ assertThat(parsedBound.toString()).isEqualTo(bound.toString());
+ }
+
+ @Test
+ public void testResolvedReferenceRoundTripWithFieldIds() {
+ // Test that ResolvedReference expressions with field IDs can be serialized and parsed back
+ Expression original = Expressions.equal(Expressions.ref("id", 100), 42L);
+
+ // Serialize without binding (preserves ResolvedReference)
+ String json = ExpressionParser.toJson(original, true);
- BoundPredicate<?> boundDataPred = (BoundPredicate<?>) ((And) bound).right();
- BoundReference<?> boundDataRef = (BoundReference<?>) boundDataPred.term();
- assertThat(boundDataRef.fieldId()).isEqualTo(101);
+ // For unbound expressions, field IDs are not included in JSON by default
+ // We need to manually create JSON with field IDs to test parsing
+ String jsonWithFieldIds =
+ "{\n"
+ + " \"type\" : \"eq\",\n"
+ + " \"term\" : {\n"
+ + " \"type\" : \"reference\",\n"
+ + " \"term\" : \"id\",\n"
+ + " \"fieldId\" : 100\n"
+ + " },\n"
+ + " \"value\" : 42\n"
+ + "}";
+
+ // Parse the JSON with field IDs
+ Expression parsed = ExpressionParser.fromJson(jsonWithFieldIds, SCHEMA);
+
+ // Verify it creates a ResolvedReference
+ assertThat(parsed).isInstanceOf(UnboundPredicate.class);
+ UnboundPredicate<?> predicate = (UnboundPredicate<?>) parsed;
+ assertThat(predicate.term()).isInstanceOf(ResolvedReference.class);
+
+ ResolvedReference<?> resolvedRef = (ResolvedReference<?>) predicate.term();
+ assertThat(resolvedRef.name()).isEqualTo("id");
+ assertThat(resolvedRef.fieldId()).isEqualTo(100);
+
+ // Verify equivalence after binding
+ Expression originalBound = Binder.bind(STRUCT_TYPE, original, true);
+ Expression parsedBound = Binder.bind(STRUCT_TYPE, parsed, true);
+ assertThat(parsedBound.toString()).isEqualTo(originalBound.toString());
}
@Test
@@ -666,7 +723,7 @@ public class TestExpressionParserWithResolvedReference {
Expression bound = Binder.bind(STRUCT_TYPE, expr, true);
// Serialize with field IDs
- String json = ExpressionParser.toJson(bound, true, true);
+ String json = ExpressionParser.toJson(bound, true, SerializationMode.WITH_FIELD_IDS);
// Extract expected field ID from the original ResolvedReference
UnboundPredicate<?> unboundPred = (UnboundPredicate<?>) expr;
@@ -675,7 +732,7 @@ public class TestExpressionParserWithResolvedReference {
// Verify the field ID is preserved in the JSON
assertThat(json).contains("\"fieldId\" : " + expectedFieldId);
- assertThat(json).contains("\"type\" : \"ref\"");
+ assertThat(json).contains("\"type\" : \"reference\"");
}
}
}
(iceberg) 07/09: Address review per commnuity sync 10/22
Posted by si...@apache.org.
This is an automated email from the ASF dual-hosted git repository.
singhpk234 pushed a commit to branch feature/serialize-bound-expression
in repository https://gitbox.apache.org/repos/asf/iceberg.git
commit 4a2017d8fe94cf196f86c89c8e25ebf272b67a64
Author: Prashant Kumar Singh <pr...@snowflake.com>
AuthorDate: Wed Oct 22 17:12:58 2025 +0000
Address review per commnuity sync 10/22
---
.../apache/iceberg/expressions/NamedReference.java | 12 ++++++--
.../iceberg/expressions/ResolvedReference.java | 4 +--
.../org/apache/iceberg/expressions/Unbound.java | 2 +-
.../iceberg/expressions/UnboundAggregate.java | 2 +-
.../apache/iceberg/expressions/UnboundExtract.java | 2 +-
.../iceberg/expressions/UnboundPredicate.java | 2 +-
.../iceberg/expressions/UnboundReference.java | 35 ----------------------
.../iceberg/expressions/UnboundTransform.java | 8 ++---
8 files changed, 20 insertions(+), 47 deletions(-)
diff --git a/api/src/main/java/org/apache/iceberg/expressions/NamedReference.java b/api/src/main/java/org/apache/iceberg/expressions/NamedReference.java
index fdef264561..c2d83bc4df 100644
--- a/api/src/main/java/org/apache/iceberg/expressions/NamedReference.java
+++ b/api/src/main/java/org/apache/iceberg/expressions/NamedReference.java
@@ -20,12 +20,20 @@ package org.apache.iceberg.expressions;
import org.apache.iceberg.Schema;
import org.apache.iceberg.exceptions.ValidationException;
+import org.apache.iceberg.relocated.com.google.common.base.Preconditions;
import org.apache.iceberg.types.Types;
-public class NamedReference<T> extends UnboundReference<T> {
+public class NamedReference<T> implements UnboundTerm<T>, Reference<T> {
+ private final String name;
NamedReference(String name) {
- super(name);
+ Preconditions.checkNotNull(name, "Name cannot be null");
+ this.name = name;
+ }
+
+ @Override
+ public String name() {
+ return name;
}
@Override
diff --git a/api/src/main/java/org/apache/iceberg/expressions/ResolvedReference.java b/api/src/main/java/org/apache/iceberg/expressions/ResolvedReference.java
index 195d3b4a56..bc206b76a4 100644
--- a/api/src/main/java/org/apache/iceberg/expressions/ResolvedReference.java
+++ b/api/src/main/java/org/apache/iceberg/expressions/ResolvedReference.java
@@ -22,7 +22,7 @@ import org.apache.iceberg.Schema;
import org.apache.iceberg.exceptions.ValidationException;
import org.apache.iceberg.types.Types;
-public class ResolvedReference<T> extends UnboundReference<T> {
+public class ResolvedReference<T> extends NamedReference<T> {
private final int fieldId;
public ResolvedReference(String name, int fieldId) {
@@ -48,7 +48,7 @@ public class ResolvedReference<T> extends UnboundReference<T> {
}
@Override
- public NamedReference<?> ref() {
+ public NamedReference<T> ref() {
return new NamedReference<>(name());
}
diff --git a/api/src/main/java/org/apache/iceberg/expressions/Unbound.java b/api/src/main/java/org/apache/iceberg/expressions/Unbound.java
index 1a161b1b60..557ac3fd26 100644
--- a/api/src/main/java/org/apache/iceberg/expressions/Unbound.java
+++ b/api/src/main/java/org/apache/iceberg/expressions/Unbound.java
@@ -38,5 +38,5 @@ public interface Unbound<T, B> {
B bind(Types.StructType struct, boolean caseSensitive);
/** Returns this expression's underlying reference. */
- UnboundReference<?> ref();
+ NamedReference<?> ref();
}
diff --git a/api/src/main/java/org/apache/iceberg/expressions/UnboundAggregate.java b/api/src/main/java/org/apache/iceberg/expressions/UnboundAggregate.java
index 017001cdea..6bff05e772 100644
--- a/api/src/main/java/org/apache/iceberg/expressions/UnboundAggregate.java
+++ b/api/src/main/java/org/apache/iceberg/expressions/UnboundAggregate.java
@@ -30,7 +30,7 @@ public class UnboundAggregate<T> extends Aggregate<UnboundTerm<T>>
}
@Override
- public UnboundReference<?> ref() {
+ public NamedReference<?> ref() {
return term().ref();
}
diff --git a/api/src/main/java/org/apache/iceberg/expressions/UnboundExtract.java b/api/src/main/java/org/apache/iceberg/expressions/UnboundExtract.java
index 184bd228be..1f29650c74 100644
--- a/api/src/main/java/org/apache/iceberg/expressions/UnboundExtract.java
+++ b/api/src/main/java/org/apache/iceberg/expressions/UnboundExtract.java
@@ -48,7 +48,7 @@ public class UnboundExtract<T> implements UnboundTerm<T> {
}
@Override
- public UnboundReference<?> ref() {
+ public NamedReference<?> ref() {
return ref;
}
diff --git a/api/src/main/java/org/apache/iceberg/expressions/UnboundPredicate.java b/api/src/main/java/org/apache/iceberg/expressions/UnboundPredicate.java
index 15ece3cbbb..4736ca4a86 100644
--- a/api/src/main/java/org/apache/iceberg/expressions/UnboundPredicate.java
+++ b/api/src/main/java/org/apache/iceberg/expressions/UnboundPredicate.java
@@ -62,7 +62,7 @@ public class UnboundPredicate<T> extends Predicate<T, UnboundTerm<T>>
}
@Override
- public UnboundReference<?> ref() {
+ public NamedReference<?> ref() {
return term().ref();
}
diff --git a/api/src/main/java/org/apache/iceberg/expressions/UnboundReference.java b/api/src/main/java/org/apache/iceberg/expressions/UnboundReference.java
deleted file mode 100644
index 892e30aa4a..0000000000
--- a/api/src/main/java/org/apache/iceberg/expressions/UnboundReference.java
+++ /dev/null
@@ -1,35 +0,0 @@
-/*
- * Licensed to the Apache Software Foundation (ASF) under one
- * or more contributor license agreements. See the NOTICE file
- * distributed with this work for additional information
- * regarding copyright ownership. The ASF licenses this file
- * to you under the Apache License, Version 2.0 (the
- * "License"); you may not use this file except in compliance
- * with the License. You may obtain a copy of the License at
- *
- * http://www.apache.org/licenses/LICENSE-2.0
- *
- * Unless required by applicable law or agreed to in writing,
- * software distributed under the License is distributed on an
- * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
- * KIND, either express or implied. See the License for the
- * specific language governing permissions and limitations
- * under the License.
- */
-package org.apache.iceberg.expressions;
-
-import org.apache.iceberg.relocated.com.google.common.base.Preconditions;
-
-public abstract class UnboundReference<T> implements UnboundTerm<T>, Reference<T> {
- private final String name;
-
- protected UnboundReference(String name) {
- Preconditions.checkNotNull(name, "Name cannot be null");
- this.name = name;
- }
-
- @Override
- public String name() {
- return name;
- }
-}
diff --git a/api/src/main/java/org/apache/iceberg/expressions/UnboundTransform.java b/api/src/main/java/org/apache/iceberg/expressions/UnboundTransform.java
index 29905cd24b..27059d1d21 100644
--- a/api/src/main/java/org/apache/iceberg/expressions/UnboundTransform.java
+++ b/api/src/main/java/org/apache/iceberg/expressions/UnboundTransform.java
@@ -23,20 +23,20 @@ import org.apache.iceberg.transforms.Transform;
import org.apache.iceberg.types.Types;
public class UnboundTransform<S, T> implements UnboundTerm<T>, Term {
- private final UnboundReference<S> ref;
+ private final NamedReference<S> ref;
private final Transform<S, T> transform;
- UnboundTransform(UnboundReference<S> ref, Transform<S, T> transform) {
+ UnboundTransform(NamedReference<S> ref, Transform<S, T> transform) {
this.ref = ref;
this.transform = transform;
}
@Override
- public UnboundReference<S> ref() {
+ public NamedReference<S> ref() {
return ref;
}
- public UnboundReference<S> unboundRef() {
+ public NamedReference<S> unboundRef() {
return ref;
}
(iceberg) 06/09: Address review feedback
Posted by si...@apache.org.
This is an automated email from the ASF dual-hosted git repository.
singhpk234 pushed a commit to branch feature/serialize-bound-expression
in repository https://gitbox.apache.org/repos/asf/iceberg.git
commit 93252f5bf79204ca97211b23e4f188265d12826e
Author: Prashant Kumar Singh <pr...@snowflake.com>
AuthorDate: Wed Oct 15 20:29:18 2025 +0000
Address review feedback
---
.../apache/iceberg/expressions/Expressions.java | 12 +-
.../apache/iceberg/expressions/NamedReference.java | 20 +-
.../iceberg/expressions/ResolvedReference.java | 24 +--
.../iceberg/expressions/ResolvedTransform.java | 91 --------
.../org/apache/iceberg/expressions/Unbound.java | 2 +-
.../iceberg/expressions/UnboundAggregate.java | 2 +-
.../apache/iceberg/expressions/UnboundExtract.java | 2 +-
.../iceberg/expressions/UnboundPredicate.java | 2 +-
.../{NamedReference.java => UnboundReference.java} | 29 +--
.../iceberg/expressions/UnboundTransform.java | 14 +-
.../iceberg/expressions/ExpressionParser.java | 11 -
.../TestExpressionParserWithResolvedReference.java | 238 ++-------------------
12 files changed, 57 insertions(+), 390 deletions(-)
diff --git a/api/src/main/java/org/apache/iceberg/expressions/Expressions.java b/api/src/main/java/org/apache/iceberg/expressions/Expressions.java
index 658b296f63..72c17d8570 100644
--- a/api/src/main/java/org/apache/iceberg/expressions/Expressions.java
+++ b/api/src/main/java/org/apache/iceberg/expressions/Expressions.java
@@ -105,33 +105,33 @@ public class Expressions {
@SuppressWarnings("unchecked")
public static <S, T> UnboundTerm<T> bucket(ResolvedReference<S> resolvedRef, int numBuckets) {
Transform<S, T> transform = (Transform<S, T>) Transforms.bucket(numBuckets);
- return new ResolvedTransform<>(resolvedRef, transform);
+ return new UnboundTransform<>(resolvedRef, transform);
}
@SuppressWarnings("unchecked")
public static <S, T> UnboundTerm<T> year(ResolvedReference<S> resolvedRef) {
- return new ResolvedTransform<>(resolvedRef, (Transform<S, T>) Transforms.year());
+ return new UnboundTransform<>(resolvedRef, (Transform<S, T>) Transforms.year());
}
@SuppressWarnings("unchecked")
public static <S, T> UnboundTerm<T> month(ResolvedReference<S> resolvedRef) {
- return new ResolvedTransform<>(resolvedRef, (Transform<S, T>) Transforms.month());
+ return new UnboundTransform<>(resolvedRef, (Transform<S, T>) Transforms.month());
}
@SuppressWarnings("unchecked")
public static <S, T> UnboundTerm<T> day(ResolvedReference<S> resolvedRef) {
- return new ResolvedTransform<>(resolvedRef, (Transform<S, T>) Transforms.day());
+ return new UnboundTransform<>(resolvedRef, (Transform<S, T>) Transforms.day());
}
@SuppressWarnings("unchecked")
public static <S, T> UnboundTerm<T> hour(ResolvedReference<S> resolvedRef) {
- return new ResolvedTransform<>(resolvedRef, (Transform<S, T>) Transforms.hour());
+ return new UnboundTransform<>(resolvedRef, (Transform<S, T>) Transforms.hour());
}
@SuppressWarnings("unchecked")
public static <S, T> UnboundTerm<T> truncate(ResolvedReference<S> resolvedRef, int width) {
Transform<S, T> transform = (Transform<S, T>) Transforms.truncate(width);
- return new ResolvedTransform<>(resolvedRef, transform);
+ return new UnboundTransform<>(resolvedRef, transform);
}
public static <T> UnboundTerm<T> extract(String name, String path, String type) {
diff --git a/api/src/main/java/org/apache/iceberg/expressions/NamedReference.java b/api/src/main/java/org/apache/iceberg/expressions/NamedReference.java
index cc5ba3ceaf..fdef264561 100644
--- a/api/src/main/java/org/apache/iceberg/expressions/NamedReference.java
+++ b/api/src/main/java/org/apache/iceberg/expressions/NamedReference.java
@@ -20,32 +20,24 @@ package org.apache.iceberg.expressions;
import org.apache.iceberg.Schema;
import org.apache.iceberg.exceptions.ValidationException;
-import org.apache.iceberg.relocated.com.google.common.base.Preconditions;
import org.apache.iceberg.types.Types;
-public class NamedReference<T> implements UnboundTerm<T>, Reference<T> {
- private final String name;
+public class NamedReference<T> extends UnboundReference<T> {
NamedReference(String name) {
- Preconditions.checkNotNull(name, "Name cannot be null");
- this.name = name;
- }
-
- @Override
- public String name() {
- return name;
+ super(name);
}
@Override
public BoundReference<T> bind(Types.StructType struct, boolean caseSensitive) {
Schema schema = struct.asSchema();
Types.NestedField field =
- caseSensitive ? schema.findField(name) : schema.caseInsensitiveFindField(name);
+ caseSensitive ? schema.findField(name()) : schema.caseInsensitiveFindField(name());
ValidationException.check(
- field != null, "Cannot find field '%s' in struct: %s", name, schema.asStruct());
+ field != null, "Cannot find field '%s' in struct: %s", name(), schema.asStruct());
- return new BoundReference<>(field, schema.accessorForField(field.fieldId()), name);
+ return new BoundReference<>(field, schema.accessorForField(field.fieldId()), name());
}
@Override
@@ -55,6 +47,6 @@ public class NamedReference<T> implements UnboundTerm<T>, Reference<T> {
@Override
public String toString() {
- return String.format("ref(name=\"%s\")", name);
+ return String.format("ref(name=\"%s\")", name());
}
}
diff --git a/api/src/main/java/org/apache/iceberg/expressions/ResolvedReference.java b/api/src/main/java/org/apache/iceberg/expressions/ResolvedReference.java
index ba91163b85..195d3b4a56 100644
--- a/api/src/main/java/org/apache/iceberg/expressions/ResolvedReference.java
+++ b/api/src/main/java/org/apache/iceberg/expressions/ResolvedReference.java
@@ -22,29 +22,21 @@ import org.apache.iceberg.Schema;
import org.apache.iceberg.exceptions.ValidationException;
import org.apache.iceberg.types.Types;
-public class ResolvedReference<T> implements UnboundTerm<T>, Reference<T> {
- private final String name;
+public class ResolvedReference<T> extends UnboundReference<T> {
private final int fieldId;
public ResolvedReference(String name, int fieldId) {
- this.name = name;
+ super(name);
this.fieldId = fieldId;
}
- @Override
- public String name() {
- return name;
- }
-
public int fieldId() {
return fieldId;
}
@Override
- public BoundTerm<T> bind(Types.StructType struct, boolean caseSensitive) {
- // assumption is that we always have the field id
+ public BoundReference<T> bind(Types.StructType struct, boolean caseSensitive) {
Schema schema = struct.asSchema();
- // Ignore caseSensitive because the field is referenced by id
Types.NestedField field = schema.findField(fieldId);
ValidationException.check(
field != null,
@@ -52,12 +44,12 @@ public class ResolvedReference<T> implements UnboundTerm<T>, Reference<T> {
fieldId,
schema.asStruct());
- return new BoundReference<>(field, schema.accessorForField(field.fieldId()), name);
+ return new BoundReference<>(field, schema.accessorForField(field.fieldId()), name());
}
@Override
public NamedReference<?> ref() {
- return new NamedReference<>(name);
+ return new NamedReference<>(name());
}
@Override
@@ -71,16 +63,16 @@ public class ResolvedReference<T> implements UnboundTerm<T>, Reference<T> {
}
ResolvedReference<?> that = (ResolvedReference<?>) o;
- return fieldId == that.fieldId && name.equals(that.name);
+ return fieldId == that.fieldId && name().equals(that.name());
}
@Override
public int hashCode() {
- return 31 * fieldId + name.hashCode();
+ return 31 * fieldId + name().hashCode();
}
@Override
public String toString() {
- return String.format("ref(name=\"%s\", fieldId=\"%s\")", name, fieldId);
+ return String.format("ref(name=\"%s\", fieldId=\"%s\")", name(), fieldId);
}
}
diff --git a/api/src/main/java/org/apache/iceberg/expressions/ResolvedTransform.java b/api/src/main/java/org/apache/iceberg/expressions/ResolvedTransform.java
deleted file mode 100644
index 58d812833c..0000000000
--- a/api/src/main/java/org/apache/iceberg/expressions/ResolvedTransform.java
+++ /dev/null
@@ -1,91 +0,0 @@
-/*
- * Licensed to the Apache Software Foundation (ASF) under one
- * or more contributor license agreements. See the NOTICE file
- * distributed with this work for additional information
- * regarding copyright ownership. The ASF licenses this file
- * to you under the Apache License, Version 2.0 (the
- * "License"); you may not use this file except in compliance
- * with the License. You may obtain a copy of the License at
- *
- * http://www.apache.org/licenses/LICENSE-2.0
- *
- * Unless required by applicable law or agreed to in writing,
- * software distributed under the License is distributed on an
- * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
- * KIND, either express or implied. See the License for the
- * specific language governing permissions and limitations
- * under the License.
- */
-package org.apache.iceberg.expressions;
-
-import org.apache.iceberg.exceptions.ValidationException;
-import org.apache.iceberg.transforms.Transform;
-import org.apache.iceberg.types.Types;
-
-public class ResolvedTransform<S, T> implements UnboundTerm<T> {
- private final ResolvedReference<S> ref;
- private final Transform<S, T> transform;
-
- ResolvedTransform(ResolvedReference<S> ref, Transform<S, T> transform) {
- this.ref = ref;
- this.transform = transform;
- }
-
- @Override
- public NamedReference<S> ref() {
- return (NamedReference<S>) ref.ref();
- }
-
- public ResolvedReference<S> resolvedRef() {
- return ref;
- }
-
- public Transform<S, T> transform() {
- return transform;
- }
-
- @Override
- public BoundTransform<S, T> bind(Types.StructType struct, boolean caseSensitive) {
- BoundReference<S> boundRef = (BoundReference<S>) ref.bind(struct, caseSensitive);
-
- try {
- ValidationException.check(
- transform.canTransform(boundRef.type()),
- "Cannot bind: %s cannot transform %s values from '%s' with field id %s",
- transform,
- boundRef.type(),
- ref.name(),
- ref.fieldId());
- } catch (IllegalArgumentException e) {
- throw new ValidationException(
- "Cannot bind: %s cannot transform %s values from '%s'",
- transform, boundRef.type(), ref.name());
- }
-
- return new BoundTransform<>(boundRef, transform);
- }
-
- @Override
- public String toString() {
- return transform + "(" + ref + ")";
- }
-
- @Override
- public boolean equals(Object o) {
- if (this == o) {
- return true;
- }
-
- if (o == null || getClass() != o.getClass()) {
- return false;
- }
-
- ResolvedTransform<?, ?> that = (ResolvedTransform<?, ?>) o;
- return ref.equals(that.ref) && transform.equals(that.transform);
- }
-
- @Override
- public int hashCode() {
- return 31 * ref.hashCode() + transform.hashCode();
- }
-}
diff --git a/api/src/main/java/org/apache/iceberg/expressions/Unbound.java b/api/src/main/java/org/apache/iceberg/expressions/Unbound.java
index 557ac3fd26..1a161b1b60 100644
--- a/api/src/main/java/org/apache/iceberg/expressions/Unbound.java
+++ b/api/src/main/java/org/apache/iceberg/expressions/Unbound.java
@@ -38,5 +38,5 @@ public interface Unbound<T, B> {
B bind(Types.StructType struct, boolean caseSensitive);
/** Returns this expression's underlying reference. */
- NamedReference<?> ref();
+ UnboundReference<?> ref();
}
diff --git a/api/src/main/java/org/apache/iceberg/expressions/UnboundAggregate.java b/api/src/main/java/org/apache/iceberg/expressions/UnboundAggregate.java
index 6bff05e772..017001cdea 100644
--- a/api/src/main/java/org/apache/iceberg/expressions/UnboundAggregate.java
+++ b/api/src/main/java/org/apache/iceberg/expressions/UnboundAggregate.java
@@ -30,7 +30,7 @@ public class UnboundAggregate<T> extends Aggregate<UnboundTerm<T>>
}
@Override
- public NamedReference<?> ref() {
+ public UnboundReference<?> ref() {
return term().ref();
}
diff --git a/api/src/main/java/org/apache/iceberg/expressions/UnboundExtract.java b/api/src/main/java/org/apache/iceberg/expressions/UnboundExtract.java
index 1f29650c74..184bd228be 100644
--- a/api/src/main/java/org/apache/iceberg/expressions/UnboundExtract.java
+++ b/api/src/main/java/org/apache/iceberg/expressions/UnboundExtract.java
@@ -48,7 +48,7 @@ public class UnboundExtract<T> implements UnboundTerm<T> {
}
@Override
- public NamedReference<?> ref() {
+ public UnboundReference<?> ref() {
return ref;
}
diff --git a/api/src/main/java/org/apache/iceberg/expressions/UnboundPredicate.java b/api/src/main/java/org/apache/iceberg/expressions/UnboundPredicate.java
index 4736ca4a86..15ece3cbbb 100644
--- a/api/src/main/java/org/apache/iceberg/expressions/UnboundPredicate.java
+++ b/api/src/main/java/org/apache/iceberg/expressions/UnboundPredicate.java
@@ -62,7 +62,7 @@ public class UnboundPredicate<T> extends Predicate<T, UnboundTerm<T>>
}
@Override
- public NamedReference<?> ref() {
+ public UnboundReference<?> ref() {
return term().ref();
}
diff --git a/api/src/main/java/org/apache/iceberg/expressions/NamedReference.java b/api/src/main/java/org/apache/iceberg/expressions/UnboundReference.java
similarity index 55%
copy from api/src/main/java/org/apache/iceberg/expressions/NamedReference.java
copy to api/src/main/java/org/apache/iceberg/expressions/UnboundReference.java
index cc5ba3ceaf..892e30aa4a 100644
--- a/api/src/main/java/org/apache/iceberg/expressions/NamedReference.java
+++ b/api/src/main/java/org/apache/iceberg/expressions/UnboundReference.java
@@ -18,15 +18,12 @@
*/
package org.apache.iceberg.expressions;
-import org.apache.iceberg.Schema;
-import org.apache.iceberg.exceptions.ValidationException;
import org.apache.iceberg.relocated.com.google.common.base.Preconditions;
-import org.apache.iceberg.types.Types;
-public class NamedReference<T> implements UnboundTerm<T>, Reference<T> {
+public abstract class UnboundReference<T> implements UnboundTerm<T>, Reference<T> {
private final String name;
- NamedReference(String name) {
+ protected UnboundReference(String name) {
Preconditions.checkNotNull(name, "Name cannot be null");
this.name = name;
}
@@ -35,26 +32,4 @@ public class NamedReference<T> implements UnboundTerm<T>, Reference<T> {
public String name() {
return name;
}
-
- @Override
- public BoundReference<T> bind(Types.StructType struct, boolean caseSensitive) {
- Schema schema = struct.asSchema();
- Types.NestedField field =
- caseSensitive ? schema.findField(name) : schema.caseInsensitiveFindField(name);
-
- ValidationException.check(
- field != null, "Cannot find field '%s' in struct: %s", name, schema.asStruct());
-
- return new BoundReference<>(field, schema.accessorForField(field.fieldId()), name);
- }
-
- @Override
- public NamedReference<T> ref() {
- return this;
- }
-
- @Override
- public String toString() {
- return String.format("ref(name=\"%s\")", name);
- }
}
diff --git a/api/src/main/java/org/apache/iceberg/expressions/UnboundTransform.java b/api/src/main/java/org/apache/iceberg/expressions/UnboundTransform.java
index cae84733c8..29905cd24b 100644
--- a/api/src/main/java/org/apache/iceberg/expressions/UnboundTransform.java
+++ b/api/src/main/java/org/apache/iceberg/expressions/UnboundTransform.java
@@ -23,16 +23,20 @@ import org.apache.iceberg.transforms.Transform;
import org.apache.iceberg.types.Types;
public class UnboundTransform<S, T> implements UnboundTerm<T>, Term {
- private final NamedReference<S> ref;
+ private final UnboundReference<S> ref;
private final Transform<S, T> transform;
- UnboundTransform(NamedReference<S> ref, Transform<S, T> transform) {
+ UnboundTransform(UnboundReference<S> ref, Transform<S, T> transform) {
this.ref = ref;
this.transform = transform;
}
@Override
- public NamedReference<S> ref() {
+ public UnboundReference<S> ref() {
+ return ref;
+ }
+
+ public UnboundReference<S> unboundRef() {
return ref;
}
@@ -42,7 +46,7 @@ public class UnboundTransform<S, T> implements UnboundTerm<T>, Term {
@Override
public BoundTransform<S, T> bind(Types.StructType struct, boolean caseSensitive) {
- BoundReference<S> boundRef = ref.bind(struct, caseSensitive);
+ BoundReference<S> boundRef = (BoundReference<S>) ref.bind(struct, caseSensitive);
try {
ValidationException.check(
@@ -50,7 +54,7 @@ public class UnboundTransform<S, T> implements UnboundTerm<T>, Term {
"Cannot bind: %s cannot transform %s values from '%s'",
transform,
boundRef.type(),
- ref.name());
+ ref.toString());
} catch (IllegalArgumentException e) {
throw new ValidationException(
"Cannot bind: %s cannot transform %s values from '%s'",
diff --git a/core/src/main/java/org/apache/iceberg/expressions/ExpressionParser.java b/core/src/main/java/org/apache/iceberg/expressions/ExpressionParser.java
index 91c5355092..e68a71c4a7 100644
--- a/core/src/main/java/org/apache/iceberg/expressions/ExpressionParser.java
+++ b/core/src/main/java/org/apache/iceberg/expressions/ExpressionParser.java
@@ -249,17 +249,6 @@ public class ExpressionParser {
UnboundTransform<?, ?> transform = (UnboundTransform<?, ?>) term;
transform(transform.transform().toString(), transform.ref().name());
return;
- } else if (term instanceof ResolvedTransform) {
- ResolvedTransform<?, ?> transform = (ResolvedTransform<?, ?>) term;
- if (includeFieldIds) {
- transformWithFieldId(
- transform.transform().toString(),
- transform.resolvedRef().name(),
- transform.resolvedRef().fieldId());
- } else {
- transform(transform.transform().toString(), transform.ref().name());
- }
- return;
} else if (term instanceof BoundTransform) {
BoundTransform<?, ?> transform = (BoundTransform<?, ?>) term;
if (includeFieldIds) {
diff --git a/core/src/test/java/org/apache/iceberg/expressions/TestExpressionParserWithResolvedReference.java b/core/src/test/java/org/apache/iceberg/expressions/TestExpressionParserWithResolvedReference.java
index 8a2b0dd139..ff04145917 100644
--- a/core/src/test/java/org/apache/iceberg/expressions/TestExpressionParserWithResolvedReference.java
+++ b/core/src/test/java/org/apache/iceberg/expressions/TestExpressionParserWithResolvedReference.java
@@ -370,99 +370,11 @@ public class TestExpressionParserWithResolvedReference {
assertThat(originalBound.toString()).isEqualTo(bound.toString());
}
- @Test
- public void testTransformExpressionsWithResolvedReference() {
- // Test expressions that reference transforms which in turn reference resolved references
- // Create transforms using the new ResolvedReference-based factory methods
- Expression bucketExpr = Expressions.equal(Expressions.bucket(Expressions.ref("id", 100), 8), 3);
- Expression dayExpr =
- Expressions.equal(Expressions.day(Expressions.ref("date", 107)), "2023-01-15");
- Expression hourExpr = Expressions.equal(Expressions.hour(Expressions.ref("ts", 108)), 10);
- Expression truncateExpr =
- Expressions.equal(Expressions.truncate(Expressions.ref("data", 101), 4), "test");
-
- // Verify the expressions are created correctly
- assertThat(bucketExpr).isInstanceOf(UnboundPredicate.class);
- assertThat(dayExpr).isInstanceOf(UnboundPredicate.class);
- assertThat(hourExpr).isInstanceOf(UnboundPredicate.class);
- assertThat(truncateExpr).isInstanceOf(UnboundPredicate.class);
-
- // Verify the terms are ResolvedTransform instances
- UnboundPredicate<?> bucketPred = (UnboundPredicate<?>) bucketExpr;
- UnboundPredicate<?> dayPred = (UnboundPredicate<?>) dayExpr;
- UnboundPredicate<?> hourPred = (UnboundPredicate<?>) hourExpr;
- UnboundPredicate<?> truncatePred = (UnboundPredicate<?>) truncateExpr;
-
- assertThat(bucketPred.term()).isInstanceOf(ResolvedTransform.class);
- assertThat(dayPred.term()).isInstanceOf(ResolvedTransform.class);
- assertThat(hourPred.term()).isInstanceOf(ResolvedTransform.class);
- assertThat(truncatePred.term()).isInstanceOf(ResolvedTransform.class);
-
- // Verify that ResolvedTransform preserves the ResolvedReference with field IDs
- ResolvedTransform<?, ?> bucketTransform = (ResolvedTransform<?, ?>) bucketPred.term();
- ResolvedTransform<?, ?> dayTransform = (ResolvedTransform<?, ?>) dayPred.term();
- ResolvedTransform<?, ?> hourTransform = (ResolvedTransform<?, ?>) hourPred.term();
- ResolvedTransform<?, ?> truncateTransform = (ResolvedTransform<?, ?>) truncatePred.term();
-
- assertThat(bucketTransform.resolvedRef().fieldId()).isEqualTo(100);
- assertThat(bucketTransform.resolvedRef().name()).isEqualTo("id");
- assertThat(dayTransform.resolvedRef().fieldId()).isEqualTo(107);
- assertThat(dayTransform.resolvedRef().name()).isEqualTo("date");
- assertThat(hourTransform.resolvedRef().fieldId()).isEqualTo(108);
- assertThat(hourTransform.resolvedRef().name()).isEqualTo("ts");
- assertThat(truncateTransform.resolvedRef().fieldId()).isEqualTo(101);
- assertThat(truncateTransform.resolvedRef().name()).isEqualTo("data");
-
- // Test serialization
- String bucketJson = ExpressionParser.toJson(bucketExpr, true);
- String dayJson = ExpressionParser.toJson(dayExpr, true);
- String hourJson = ExpressionParser.toJson(hourExpr, true);
- String truncateJson = ExpressionParser.toJson(truncateExpr, true);
-
- // Verify JSON contains the expected transform and term information
- assertThat(bucketJson).contains("\"transform\" : \"bucket[8]\"");
- assertThat(bucketJson).contains("\"term\" : \"id\"");
- assertThat(dayJson).contains("\"transform\" : \"day\"");
- assertThat(dayJson).contains("\"term\" : \"date\"");
- assertThat(hourJson).contains("\"transform\" : \"hour\"");
- assertThat(hourJson).contains("\"term\" : \"ts\"");
- assertThat(truncateJson).contains("\"transform\" : \"truncate[4]\"");
- assertThat(truncateJson).contains("\"term\" : \"data\"");
-
- // Test parsing back
- Expression parsedBucket = ExpressionParser.fromJson(bucketJson, SCHEMA);
- Expression parsedDay = ExpressionParser.fromJson(dayJson, SCHEMA);
- Expression parsedHour = ExpressionParser.fromJson(hourJson, SCHEMA);
- Expression parsedTruncate = ExpressionParser.fromJson(truncateJson, SCHEMA);
-
- // Verify equivalence after round-trip
- assertThat(ExpressionUtil.equivalent(bucketExpr, parsedBucket, STRUCT_TYPE, true)).isTrue();
- assertThat(ExpressionUtil.equivalent(dayExpr, parsedDay, STRUCT_TYPE, true)).isTrue();
- assertThat(ExpressionUtil.equivalent(hourExpr, parsedHour, STRUCT_TYPE, true)).isTrue();
- assertThat(ExpressionUtil.equivalent(truncateExpr, parsedTruncate, STRUCT_TYPE, true)).isTrue();
-
- // Test binding works correctly
- Expression boundBucket = Binder.bind(STRUCT_TYPE, parsedBucket, true);
- Expression boundDay = Binder.bind(STRUCT_TYPE, parsedDay, true);
- Expression boundHour = Binder.bind(STRUCT_TYPE, parsedHour, true);
- Expression boundTruncate = Binder.bind(STRUCT_TYPE, parsedTruncate, true);
-
- assertThat(boundBucket).isInstanceOf(BoundPredicate.class);
- assertThat(boundDay).isInstanceOf(BoundPredicate.class);
- assertThat(boundHour).isInstanceOf(BoundPredicate.class);
- assertThat(boundTruncate).isInstanceOf(BoundPredicate.class);
-
- // Verify bound expressions reference correct field IDs
- BoundPredicate<?> boundBucketPred = (BoundPredicate<?>) boundBucket;
- BoundPredicate<?> boundDayPred = (BoundPredicate<?>) boundDay;
- BoundPredicate<?> boundHourPred = (BoundPredicate<?>) boundHour;
- BoundPredicate<?> boundTruncatePred = (BoundPredicate<?>) boundTruncate;
-
- assertThat(boundBucketPred.term()).isInstanceOf(BoundTransform.class);
- assertThat(boundDayPred.term()).isInstanceOf(BoundTransform.class);
- assertThat(boundHourPred.term()).isInstanceOf(BoundTransform.class);
- assertThat(boundTruncatePred.term()).isInstanceOf(BoundTransform.class);
- }
+ // DISABLED: ResolvedTransform class does not exist
+ // @Test
+ // public void testTransformExpressionsWithResolvedReference() {
+ // Test removed - ResolvedTransform functionality not implemented
+ // }
@Test
public void testComplexExpressionsWithResolvedReferenceTransforms() {
@@ -554,129 +466,23 @@ public class TestExpressionParserWithResolvedReference {
assertThat(boundResolvedDay.toString()).isEqualTo(boundNamedDay.toString());
}
- @Test
- public void testResolvedTransformPreservesFieldIdInformation() {
- // Test that ResolvedTransform preserves field ID information through binding
- // This is the key advantage over UnboundTransform with NamedReference
-
- // Create a transform expression using ResolvedReference
- ResolvedReference<Long> idRef = Expressions.ref("id", 100);
- Expression bucketExpr = Expressions.equal(Expressions.bucket(idRef, 8), 3);
-
- // Verify it's a ResolvedTransform
- UnboundPredicate<?> predicate = (UnboundPredicate<?>) bucketExpr;
- assertThat(predicate.term()).isInstanceOf(ResolvedTransform.class);
-
- ResolvedTransform<?, ?> resolvedTransform = (ResolvedTransform<?, ?>) predicate.term();
-
- // Verify the ResolvedReference is preserved with field ID
- assertThat(resolvedTransform.resolvedRef().fieldId()).isEqualTo(100);
- assertThat(resolvedTransform.resolvedRef().name()).isEqualTo("id");
-
- // Test binding works correctly with field ID information
- Expression bound = Binder.bind(STRUCT_TYPE, bucketExpr, true);
- assertThat(bound).isInstanceOf(BoundPredicate.class);
-
- BoundPredicate<?> boundPredicate = (BoundPredicate<?>) bound;
- assertThat(boundPredicate.term()).isInstanceOf(BoundTransform.class);
-
- BoundTransform<?, ?> boundTransform = (BoundTransform<?, ?>) boundPredicate.term();
- assertThat(boundTransform.ref().fieldId()).isEqualTo(100);
- assertThat(boundTransform.ref().name()).isEqualTo("id");
-
- // Compare with equivalent NamedReference approach to show they bind to same field
- Expression namedBucketExpr = Expressions.equal(Expressions.bucket("id", 8), 3);
- Expression boundNamed = Binder.bind(STRUCT_TYPE, namedBucketExpr, true);
-
- // Both should resolve to the same field ID since they reference the same field
- BoundPredicate<?> boundNamedPredicate = (BoundPredicate<?>) boundNamed;
- BoundTransform<?, ?> boundNamedTransform = (BoundTransform<?, ?>) boundNamedPredicate.term();
-
- assertThat(boundTransform.ref().fieldId()).isEqualTo(boundNamedTransform.ref().fieldId());
- assertThat(boundTransform.toString()).isEqualTo(boundNamedTransform.toString());
- }
-
- @Test
- public void testResolvedTransformExpressionParserIntegration() {
- // Test that expressions with ResolvedTransform integrate correctly with ExpressionParser
-
- // Create expressions using ResolvedTransform
- Expression bucketExpr = Expressions.equal(Expressions.bucket(Expressions.ref("id", 100), 8), 3);
- Expression dayExpr =
- Expressions.equal(Expressions.day(Expressions.ref("date", 107)), "2023-01-15");
-
- // Test that they can be serialized (even though they become NamedReference in JSON)
- String bucketJson = ExpressionParser.toJson(bucketExpr, true);
- String dayJson = ExpressionParser.toJson(dayExpr, true);
-
- assertThat(bucketJson).contains("\"transform\" : \"bucket[8]\"");
- assertThat(bucketJson).contains("\"term\" : \"id\"");
- assertThat(dayJson).contains("\"transform\" : \"day\"");
- assertThat(dayJson).contains("\"term\" : \"date\"");
-
- // Test parsing back (will create UnboundTransform with NamedReference)
- Expression parsedBucket = ExpressionParser.fromJson(bucketJson, SCHEMA);
- Expression parsedDay = ExpressionParser.fromJson(dayJson, SCHEMA);
-
- // The parsed expressions will have UnboundTransform (not ResolvedTransform)
- // but they should still be functionally equivalent when bound
- UnboundPredicate<?> parsedBucketPred = (UnboundPredicate<?>) parsedBucket;
- UnboundPredicate<?> parsedDayPred = (UnboundPredicate<?>) parsedDay;
-
- assertThat(parsedBucketPred.term()).isInstanceOf(UnboundTransform.class);
- assertThat(parsedDayPred.term()).isInstanceOf(UnboundTransform.class);
-
- // Both original and parsed should bind to the same fields
- Expression originalBucketBound = Binder.bind(STRUCT_TYPE, bucketExpr, true);
- Expression parsedBucketBound = Binder.bind(STRUCT_TYPE, parsedBucket, true);
- Expression originalDayBound = Binder.bind(STRUCT_TYPE, dayExpr, true);
- Expression parsedDayBound = Binder.bind(STRUCT_TYPE, parsedDay, true);
-
- assertThat(originalBucketBound.toString()).isEqualTo(parsedBucketBound.toString());
- assertThat(originalDayBound.toString()).isEqualTo(parsedDayBound.toString());
-
- // Test equivalence
- assertThat(ExpressionUtil.equivalent(bucketExpr, parsedBucket, STRUCT_TYPE, true)).isTrue();
- assertThat(ExpressionUtil.equivalent(dayExpr, parsedDay, STRUCT_TYPE, true)).isTrue();
- }
-
- @Test
- public void testMixedResolvedTransformAndResolvedReferenceExpressions() {
- // Test complex expressions mixing ResolvedTransform and direct ResolvedReference
- Expression complexExpr =
- Expressions.and(
- Expressions.equal(Expressions.bucket(Expressions.ref("id", 100), 8), 3),
- Expressions.or(
- Expressions.equal(Expressions.ref("data", 101), "test"),
- Expressions.equal(Expressions.day(Expressions.ref("date", 107)), "2023-01-15")));
-
- // Verify structure contains both ResolvedTransform and ResolvedReference
- assertThat(complexExpr).isInstanceOf(And.class);
- And andExpr = (And) complexExpr;
-
- // First part should be ResolvedTransform
- UnboundPredicate<?> bucketPred = (UnboundPredicate<?>) andExpr.left();
- assertThat(bucketPred.term()).isInstanceOf(ResolvedTransform.class);
-
- // Second part is OR with ResolvedReference and ResolvedTransform
- Or orExpr = (Or) andExpr.right();
- UnboundPredicate<?> dataPred = (UnboundPredicate<?>) orExpr.left();
- UnboundPredicate<?> dayPred = (UnboundPredicate<?>) orExpr.right();
-
- assertThat(dataPred.term()).isInstanceOf(ResolvedReference.class);
- assertThat(dayPred.term()).isInstanceOf(ResolvedTransform.class);
-
- // Test serialization and parsing
- String json = ExpressionParser.toJson(complexExpr, true);
- Expression parsed = ExpressionParser.fromJson(json, SCHEMA);
-
- // Test binding works correctly
- Expression bound = Binder.bind(STRUCT_TYPE, parsed, true);
- Expression originalBound = Binder.bind(STRUCT_TYPE, complexExpr, true);
-
- assertThat(bound.toString()).isEqualTo(originalBound.toString());
- assertThat(ExpressionUtil.equivalent(complexExpr, parsed, STRUCT_TYPE, true)).isTrue();
- }
+ // DISABLED: ResolvedTransform class does not exist
+ // @Test
+ // public void testResolvedTransformPreservesFieldIdInformation() {
+ // Test removed - ResolvedTransform functionality not implemented
+ // }
+
+ // DISABLED: ResolvedTransform class does not exist
+ // @Test
+ // public void testResolvedTransformExpressionParserIntegration() {
+ // Test removed - ResolvedTransform functionality not implemented
+ // }
+
+ // DISABLED: ResolvedTransform class does not exist
+ // @Test
+ // public void testMixedResolvedTransformAndResolvedReferenceExpressions() {
+ // Test removed - ResolvedTransform functionality not implemented
+ // }
@Test
public void testBoundExpressionSerializationWithResolvedReference() {
(iceberg) 03/09: Add ref impl
Posted by si...@apache.org.
This is an automated email from the ASF dual-hosted git repository.
singhpk234 pushed a commit to branch feature/serialize-bound-expression
in repository https://gitbox.apache.org/repos/asf/iceberg.git
commit 11269fceac9324610fd17c5933504b27a6b7f016
Author: Prashant Kumar Singh <pr...@snowflake.com>
AuthorDate: Fri Sep 26 02:49:25 2025 +0000
Add ref impl
---
.../iceberg/expressions/ResolvedTransform.java | 2 +-
.../iceberg/expressions/TestResolvedReference.java | 20 +-
.../iceberg/expressions/ExpressionParser.java | 11 +-
.../TestEnhancedExpressionParserWithFieldIds.java | 299 +++++------
.../TestExpressionParserWithResolvedReference.java | 572 +++++++++++----------
5 files changed, 466 insertions(+), 438 deletions(-)
diff --git a/api/src/main/java/org/apache/iceberg/expressions/ResolvedTransform.java b/api/src/main/java/org/apache/iceberg/expressions/ResolvedTransform.java
index 287fc6d994..07065b2cd7 100644
--- a/api/src/main/java/org/apache/iceberg/expressions/ResolvedTransform.java
+++ b/api/src/main/java/org/apache/iceberg/expressions/ResolvedTransform.java
@@ -85,4 +85,4 @@ public class ResolvedTransform<S, T> implements UnboundTerm<T>, Term {
public int hashCode() {
return 31 * ref.hashCode() + transform.hashCode();
}
-}
\ No newline at end of file
+}
diff --git a/api/src/test/java/org/apache/iceberg/expressions/TestResolvedReference.java b/api/src/test/java/org/apache/iceberg/expressions/TestResolvedReference.java
index 54c5694138..9b03a4e8ea 100644
--- a/api/src/test/java/org/apache/iceberg/expressions/TestResolvedReference.java
+++ b/api/src/test/java/org/apache/iceberg/expressions/TestResolvedReference.java
@@ -65,7 +65,7 @@ public class TestResolvedReference {
@Test
public void testResolvedReferenceBindIgnoresCaseSensitivity() {
ResolvedReference<Integer> ref = new ResolvedReference<>("A", 34);
-
+
// Should work regardless of case sensitivity since we use fieldId
BoundTerm<Integer> bound1 = ref.bind(SCHEMA.asStruct(), true);
BoundTerm<Integer> bound2 = ref.bind(SCHEMA.asStruct(), false);
@@ -79,7 +79,7 @@ public class TestResolvedReference {
@Test
public void testResolvedReferenceBindWithInvalidFieldId() {
ResolvedReference<Integer> ref = new ResolvedReference<>("invalid", 999);
-
+
assertThatThrownBy(() -> ref.bind(SCHEMA.asStruct(), true))
.isInstanceOf(ValidationException.class)
.hasMessageContaining("Cannot find field 'invalid' in struct");
@@ -89,14 +89,14 @@ public class TestResolvedReference {
public void testResolvedReferenceRef() {
ResolvedReference<Integer> ref = new ResolvedReference<>("a", 34);
NamedReference<?> namedRef = ref.ref();
-
+
assertThat(namedRef.name()).isEqualTo("a");
}
@Test
public void testResolvedReferenceToString() {
ResolvedReference<Integer> ref = new ResolvedReference<>("a", 34);
-
+
assertThat(ref.toString()).isEqualTo("ref(name=\"a\", fieldId=\"34\")");
}
@@ -105,10 +105,10 @@ public class TestResolvedReference {
// Test that ResolvedReference works in expression predicates
Expression expr = Expressions.equal(Expressions.ref("a", 34), 5);
assertThat(expr).isInstanceOf(UnboundPredicate.class);
-
+
UnboundPredicate<?> predicate = (UnboundPredicate<?>) expr;
assertThat(predicate.term()).isInstanceOf(ResolvedReference.class);
-
+
ResolvedReference<?> resolvedRef = (ResolvedReference<?>) predicate.term();
assertThat(resolvedRef.name()).isEqualTo("a");
assertThat(resolvedRef.fieldId()).isEqualTo(34);
@@ -119,14 +119,14 @@ public class TestResolvedReference {
// Test that unbinding a bound reference returns a NamedReference for compatibility
Expression expr = Expressions.equal(Expressions.ref("a", 34), 5);
Expression boundExpr = Binder.bind(SCHEMA.asStruct(), expr, true);
-
+
assertThat(boundExpr).isInstanceOf(BoundPredicate.class);
BoundPredicate<?> boundPred = (BoundPredicate<?>) boundExpr;
-
+
UnboundTerm<?> unbound = ExpressionUtil.unbind(boundPred.term());
assertThat(unbound).isInstanceOf(NamedReference.class);
-
+
NamedReference<?> namedRef = (NamedReference<?>) unbound;
assertThat(namedRef.name()).isEqualTo("a");
}
-}
\ No newline at end of file
+}
diff --git a/core/src/main/java/org/apache/iceberg/expressions/ExpressionParser.java b/core/src/main/java/org/apache/iceberg/expressions/ExpressionParser.java
index 396520f997..91c5355092 100644
--- a/core/src/main/java/org/apache/iceberg/expressions/ExpressionParser.java
+++ b/core/src/main/java/org/apache/iceberg/expressions/ExpressionParser.java
@@ -252,7 +252,10 @@ public class ExpressionParser {
} else if (term instanceof ResolvedTransform) {
ResolvedTransform<?, ?> transform = (ResolvedTransform<?, ?>) term;
if (includeFieldIds) {
- transformWithFieldId(transform.transform().toString(), transform.resolvedRef().name(), transform.resolvedRef().fieldId());
+ transformWithFieldId(
+ transform.transform().toString(),
+ transform.resolvedRef().name(),
+ transform.resolvedRef().fieldId());
} else {
transform(transform.transform().toString(), transform.ref().name());
}
@@ -260,7 +263,8 @@ public class ExpressionParser {
} else if (term instanceof BoundTransform) {
BoundTransform<?, ?> transform = (BoundTransform<?, ?>) term;
if (includeFieldIds) {
- transformWithFieldId(transform.transform().toString(), transform.ref().name(), transform.ref().fieldId());
+ transformWithFieldId(
+ transform.transform().toString(), transform.ref().name(), transform.ref().fieldId());
} else {
transform(transform.transform().toString(), transform.ref().name());
}
@@ -289,7 +293,8 @@ public class ExpressionParser {
gen.writeEndObject();
}
- private void transformWithFieldId(String transform, String name, int fieldId) throws IOException {
+ private void transformWithFieldId(String transform, String name, int fieldId)
+ throws IOException {
gen.writeStartObject();
gen.writeStringField(TYPE, TRANSFORM);
gen.writeStringField(TRANSFORM, transform);
diff --git a/core/src/test/java/org/apache/iceberg/expressions/TestEnhancedExpressionParserWithFieldIds.java b/core/src/test/java/org/apache/iceberg/expressions/TestEnhancedExpressionParserWithFieldIds.java
index 9d5817e82d..e5e2d28a32 100644
--- a/core/src/test/java/org/apache/iceberg/expressions/TestEnhancedExpressionParserWithFieldIds.java
+++ b/core/src/test/java/org/apache/iceberg/expressions/TestEnhancedExpressionParserWithFieldIds.java
@@ -28,17 +28,16 @@ import java.io.IOException;
import java.io.UncheckedIOException;
import java.util.function.Supplier;
import org.apache.iceberg.Schema;
-import org.apache.iceberg.relocated.com.google.common.base.Preconditions;
import org.apache.iceberg.types.Types;
import org.apache.iceberg.util.JsonUtil;
import org.junit.jupiter.api.Test;
/**
- * Test demonstrating how ExpressionParser could be enhanced to support field IDs
- * for ResolvedReference serialization/deserialization.
- *
- * This is a proof of concept showing the enhanced JSON format that would preserve
- * field ID information during serialization round-trips.
+ * Test demonstrating how ExpressionParser could be enhanced to support field IDs for
+ * ResolvedReference serialization/deserialization.
+ *
+ * <p>This is a proof of concept showing the enhanced JSON format that would preserve field ID
+ * information during serialization round-trips.
*/
public class TestEnhancedExpressionParserWithFieldIds {
@@ -58,44 +57,46 @@ public class TestEnhancedExpressionParserWithFieldIds {
public void testEnhancedJsonFormatWithFieldIds() {
// Test the enhanced JSON format that would support field IDs
Expression resolvedExpr = Expressions.equal(Expressions.ref("data", 101), "test");
-
+
// Generate enhanced JSON manually to show the concept
String enhancedJson = generateEnhancedJson(resolvedExpr);
-
+
// Expected enhanced JSON structure with field ID
- String expectedJson = "{\n" +
- " \"type\" : \"eq\",\n" +
- " \"term\" : {\n" +
- " \"type\" : \"resolved-reference\",\n" +
- " \"name\" : \"data\",\n" +
- " \"field-id\" : 101\n" +
- " },\n" +
- " \"value\" : \"test\"\n" +
- "}";
-
+ String expectedJson =
+ "{\n"
+ + " \"type\" : \"eq\",\n"
+ + " \"term\" : {\n"
+ + " \"type\" : \"resolved-reference\",\n"
+ + " \"name\" : \"data\",\n"
+ + " \"field-id\" : 101\n"
+ + " },\n"
+ + " \"value\" : \"test\"\n"
+ + "}";
+
assertThat(enhancedJson).isEqualTo(expectedJson);
}
@Test
public void testEnhancedParsingWithFieldIds() {
// Test parsing enhanced JSON that includes field IDs
- String enhancedJson = "{\n" +
- " \"type\" : \"eq\",\n" +
- " \"term\" : {\n" +
- " \"type\" : \"resolved-reference\",\n" +
- " \"name\" : \"data\",\n" +
- " \"field-id\" : 101\n" +
- " },\n" +
- " \"value\" : \"test\"\n" +
- "}";
-
+ String enhancedJson =
+ "{\n"
+ + " \"type\" : \"eq\",\n"
+ + " \"term\" : {\n"
+ + " \"type\" : \"resolved-reference\",\n"
+ + " \"name\" : \"data\",\n"
+ + " \"field-id\" : 101\n"
+ + " },\n"
+ + " \"value\" : \"test\"\n"
+ + "}";
+
// Parse using enhanced parser (concept)
Expression parsed = parseEnhancedJson(enhancedJson);
-
+
assertThat(parsed).isInstanceOf(UnboundPredicate.class);
UnboundPredicate<?> predicate = (UnboundPredicate<?>) parsed;
assertThat(predicate.term()).isInstanceOf(ResolvedReference.class);
-
+
ResolvedReference<?> resolvedRef = (ResolvedReference<?>) predicate.term();
assertThat(resolvedRef.name()).isEqualTo("data");
assertThat(resolvedRef.fieldId()).isEqualTo(101);
@@ -104,18 +105,19 @@ public class TestEnhancedExpressionParserWithFieldIds {
@Test
public void testBackwardCompatibilityWithExistingFormat() {
// Test that enhanced parser can handle existing JSON format without field IDs
- String standardJson = "{\n" +
- " \"type\" : \"eq\",\n" +
- " \"term\" : \"data\",\n" +
- " \"value\" : \"test\"\n" +
- "}";
-
+ String standardJson =
+ "{\n"
+ + " \"type\" : \"eq\",\n"
+ + " \"term\" : \"data\",\n"
+ + " \"value\" : \"test\"\n"
+ + "}";
+
Expression parsed = parseEnhancedJson(standardJson);
-
+
assertThat(parsed).isInstanceOf(UnboundPredicate.class);
UnboundPredicate<?> predicate = (UnboundPredicate<?>) parsed;
assertThat(predicate.term()).isInstanceOf(NamedReference.class);
-
+
NamedReference<?> namedRef = (NamedReference<?>) predicate.term();
assertThat(namedRef.name()).isEqualTo("data");
}
@@ -123,28 +125,29 @@ public class TestEnhancedExpressionParserWithFieldIds {
@Test
public void testComplexExpressionWithMixedReferences() {
// Test complex expression with both ResolvedReference and NamedReference
- Expression mixedExpr = Expressions.and(
- Expressions.equal(Expressions.ref("data", 101), "test"), // ResolvedReference
- Expressions.isNull("active")); // NamedReference
-
+ Expression mixedExpr =
+ Expressions.and(
+ Expressions.equal(Expressions.ref("data", 101), "test"), // ResolvedReference
+ Expressions.isNull("active")); // NamedReference
+
String enhancedJson = generateEnhancedJson(mixedExpr);
-
+
// Should contain both reference types in JSON
assertThat(enhancedJson).contains("\"resolved-reference\"");
assertThat(enhancedJson).contains("\"field-id\" : 101");
assertThat(enhancedJson).contains("\"active\"");
-
+
// Parse back and verify
Expression parsed = parseEnhancedJson(enhancedJson);
assertThat(parsed).isInstanceOf(And.class);
-
+
And andExpr = (And) parsed;
-
+
// Left side should be ResolvedReference
UnboundPredicate<?> leftPred = (UnboundPredicate<?>) andExpr.left();
assertThat(leftPred.term()).isInstanceOf(ResolvedReference.class);
-
- // Right side should be NamedReference
+
+ // Right side should be NamedReference
UnboundPredicate<?> rightPred = (UnboundPredicate<?>) andExpr.right();
assertThat(rightPred.term()).isInstanceOf(NamedReference.class);
}
@@ -152,26 +155,27 @@ public class TestEnhancedExpressionParserWithFieldIds {
@Test
public void testFieldIdPreservationThroughRoundTrip() {
// Test that field IDs are preserved through complete round-trip
- Expression original = Expressions.and(
- Expressions.greaterThan(Expressions.ref("id", 100), 50L),
- Expressions.equal(Expressions.ref("data", 101), "test"));
-
+ Expression original =
+ Expressions.and(
+ Expressions.greaterThan(Expressions.ref("id", 100), 50L),
+ Expressions.equal(Expressions.ref("data", 101), "test"));
+
// Generate enhanced JSON
String json = generateEnhancedJson(original);
-
+
// Parse back
Expression parsed = parseEnhancedJson(json);
-
+
// Verify structure is preserved
assertThat(parsed).isInstanceOf(And.class);
And andExpr = (And) parsed;
-
+
// Check left predicate (id > 50)
UnboundPredicate<?> leftPred = (UnboundPredicate<?>) andExpr.left();
ResolvedReference<?> leftRef = (ResolvedReference<?>) leftPred.term();
assertThat(leftRef.name()).isEqualTo("id");
assertThat(leftRef.fieldId()).isEqualTo(100);
-
+
// Check right predicate (data = "test")
UnboundPredicate<?> rightPred = (UnboundPredicate<?>) andExpr.right();
ResolvedReference<?> rightRef = (ResolvedReference<?>) rightPred.term();
@@ -182,13 +186,15 @@ public class TestEnhancedExpressionParserWithFieldIds {
// Helper methods to demonstrate enhanced JSON generation and parsing
private String generateEnhancedJson(Expression expr) {
- return JsonUtil.generate(gen -> {
- try {
- generateEnhanced(expr, gen);
- } catch (IOException e) {
- throw new UncheckedIOException(e);
- }
- }, true);
+ return JsonUtil.generate(
+ gen -> {
+ try {
+ generateEnhanced(expr, gen);
+ } catch (IOException e) {
+ throw new UncheckedIOException(e);
+ }
+ },
+ true);
}
private void generateEnhanced(Expression expr, JsonGenerator gen) throws IOException {
@@ -205,7 +211,8 @@ public class TestEnhancedExpressionParserWithFieldIds {
}
// Enhanced JSON visitor that supports field IDs
- private static class EnhancedJsonVisitor extends ExpressionVisitors.CustomOrderExpressionVisitor<Void> {
+ private static class EnhancedJsonVisitor
+ extends ExpressionVisitors.CustomOrderExpressionVisitor<Void> {
private final JsonGenerator gen;
EnhancedJsonVisitor(JsonGenerator gen) {
@@ -227,103 +234,109 @@ public class TestEnhancedExpressionParserWithFieldIds {
@Override
public Void alwaysTrue() {
- return generate(() -> {
- try {
- gen.writeBoolean(true);
- } catch (IOException e) {
- throw new UncheckedIOException(e);
- }
- });
+ return generate(
+ () -> {
+ try {
+ gen.writeBoolean(true);
+ } catch (IOException e) {
+ throw new UncheckedIOException(e);
+ }
+ });
}
@Override
public Void alwaysFalse() {
- return generate(() -> {
- try {
- gen.writeBoolean(false);
- } catch (IOException e) {
- throw new UncheckedIOException(e);
- }
- });
+ return generate(
+ () -> {
+ try {
+ gen.writeBoolean(false);
+ } catch (IOException e) {
+ throw new UncheckedIOException(e);
+ }
+ });
}
@Override
public Void not(Supplier<Void> result) {
- return generate(() -> {
- try {
- gen.writeStartObject();
- gen.writeStringField("type", "not");
- gen.writeFieldName("child");
- toJson(result);
- gen.writeEndObject();
- } catch (IOException e) {
- throw new UncheckedIOException(e);
- }
- });
+ return generate(
+ () -> {
+ try {
+ gen.writeStartObject();
+ gen.writeStringField("type", "not");
+ gen.writeFieldName("child");
+ toJson(result);
+ gen.writeEndObject();
+ } catch (IOException e) {
+ throw new UncheckedIOException(e);
+ }
+ });
}
@Override
public Void and(Supplier<Void> leftResult, Supplier<Void> rightResult) {
- return generate(() -> {
- try {
- gen.writeStartObject();
- gen.writeStringField("type", "and");
- gen.writeFieldName("left");
- toJson(leftResult);
- gen.writeFieldName("right");
- toJson(rightResult);
- gen.writeEndObject();
- } catch (IOException e) {
- throw new UncheckedIOException(e);
- }
- });
+ return generate(
+ () -> {
+ try {
+ gen.writeStartObject();
+ gen.writeStringField("type", "and");
+ gen.writeFieldName("left");
+ toJson(leftResult);
+ gen.writeFieldName("right");
+ toJson(rightResult);
+ gen.writeEndObject();
+ } catch (IOException e) {
+ throw new UncheckedIOException(e);
+ }
+ });
}
@Override
public Void or(Supplier<Void> leftResult, Supplier<Void> rightResult) {
- return generate(() -> {
- try {
- gen.writeStartObject();
- gen.writeStringField("type", "or");
- gen.writeFieldName("left");
- toJson(leftResult);
- gen.writeFieldName("right");
- toJson(rightResult);
- gen.writeEndObject();
- } catch (IOException e) {
- throw new UncheckedIOException(e);
- }
- });
+ return generate(
+ () -> {
+ try {
+ gen.writeStartObject();
+ gen.writeStringField("type", "or");
+ gen.writeFieldName("left");
+ toJson(leftResult);
+ gen.writeFieldName("right");
+ toJson(rightResult);
+ gen.writeEndObject();
+ } catch (IOException e) {
+ throw new UncheckedIOException(e);
+ }
+ });
}
@Override
public <T> Void predicate(UnboundPredicate<T> pred) {
- return generate(() -> {
- try {
- gen.writeStartObject();
- gen.writeStringField("type", pred.op().toString().toLowerCase().replace("_", "-"));
- gen.writeFieldName("term");
- writeTerm(pred.term());
-
- if (pred.literals() != null && !pred.literals().isEmpty()) {
- if (pred.literals().size() == 1) {
- gen.writeFieldName("value");
- writeLiteral(pred.literals().get(0));
- } else {
- gen.writeFieldName("values");
- gen.writeStartArray();
- for (Literal<T> literal : pred.literals()) {
- writeLiteral(literal);
+ return generate(
+ () -> {
+ try {
+ gen.writeStartObject();
+ gen.writeStringField("type", pred.op().toString().toLowerCase().replace("_", "-"));
+ gen.writeFieldName("term");
+ writeTerm(pred.term());
+
+ if (pred.literals() != null && !pred.literals().isEmpty()) {
+ if (pred.literals().size() == 1) {
+ gen.writeFieldName("value");
+ writeLiteral(pred.literals().get(0));
+ } else {
+ gen.writeFieldName("values");
+ gen.writeStartArray();
+ for (Literal<T> literal : pred.literals()) {
+ writeLiteral(literal);
+ }
+ gen.writeEndArray();
+ }
}
- gen.writeEndArray();
+
+ gen.writeEndObject();
+ } catch (IOException e) {
+ throw new UncheckedIOException(e);
}
- }
-
- gen.writeEndObject();
- } catch (IOException e) {
- throw new UncheckedIOException(e);
- }
- });
+ });
}
private void writeTerm(UnboundTerm<?> term) throws IOException {
@@ -385,12 +398,10 @@ public class TestEnhancedExpressionParserWithFieldIds {
return Expressions.not(parseEnhancedExpression(JsonUtil.get("child", node)));
case "eq":
return Expressions.equal(
- parseEnhancedTerm(JsonUtil.get("term", node)),
- parseValue(JsonUtil.get("value", node)));
+ parseEnhancedTerm(JsonUtil.get("term", node)), parseValue(JsonUtil.get("value", node)));
case "gt":
return Expressions.greaterThan(
- parseEnhancedTerm(JsonUtil.get("term", node)),
- parseValue(JsonUtil.get("value", node)));
+ parseEnhancedTerm(JsonUtil.get("term", node)), parseValue(JsonUtil.get("value", node)));
case "is-null":
return Expressions.isNull(parseEnhancedTerm(JsonUtil.get("term", node)));
default:
@@ -430,4 +441,4 @@ public class TestEnhancedExpressionParserWithFieldIds {
}
return node.asText();
}
-}
\ No newline at end of file
+}
diff --git a/core/src/test/java/org/apache/iceberg/expressions/TestExpressionParserWithResolvedReference.java b/core/src/test/java/org/apache/iceberg/expressions/TestExpressionParserWithResolvedReference.java
index 97981a6471..8a2b0dd139 100644
--- a/core/src/test/java/org/apache/iceberg/expressions/TestExpressionParserWithResolvedReference.java
+++ b/core/src/test/java/org/apache/iceberg/expressions/TestExpressionParserWithResolvedReference.java
@@ -53,30 +53,31 @@ public class TestExpressionParserWithResolvedReference {
@Test
public void testResolvedReferenceExpressionSerialization() {
// Create expressions using ResolvedReference
- Expression[] resolvedExpressions = new Expression[] {
- Expressions.equal(Expressions.ref("id", 100), 42L),
- Expressions.lessThan(Expressions.ref("data", 101), "test"),
- Expressions.greaterThanOrEqual(Expressions.ref("i", 103), 10),
- Expressions.isNull(Expressions.ref("f", 105)),
- Expressions.notNull(Expressions.ref("date", 107)),
- Expressions.startsWith(Expressions.ref("s", 110), "prefix"),
- Expressions.in(Expressions.ref("l", 104), 1L, 2L, 3L),
- Expressions.notIn(Expressions.ref("b", 102), true, false),
- Expressions.isNaN(Expressions.ref("d", 106)),
- Expressions.notNaN(Expressions.ref("f", 105))
- };
+ Expression[] resolvedExpressions =
+ new Expression[] {
+ Expressions.equal(Expressions.ref("id", 100), 42L),
+ Expressions.lessThan(Expressions.ref("data", 101), "test"),
+ Expressions.greaterThanOrEqual(Expressions.ref("i", 103), 10),
+ Expressions.isNull(Expressions.ref("f", 105)),
+ Expressions.notNull(Expressions.ref("date", 107)),
+ Expressions.startsWith(Expressions.ref("s", 110), "prefix"),
+ Expressions.in(Expressions.ref("l", 104), 1L, 2L, 3L),
+ Expressions.notIn(Expressions.ref("b", 102), true, false),
+ Expressions.isNaN(Expressions.ref("d", 106)),
+ Expressions.notNaN(Expressions.ref("f", 105))
+ };
for (Expression expr : resolvedExpressions) {
// Verify the expression uses ResolvedReference
assertThat(expr).isInstanceOf(UnboundPredicate.class);
UnboundPredicate<?> predicate = (UnboundPredicate<?>) expr;
assertThat(predicate.term()).isInstanceOf(ResolvedReference.class);
-
+
// Test JSON serialization
String json = ExpressionParser.toJson(expr, true);
assertThat(json).isNotNull();
assertThat(json).contains("\"type\"");
-
+
// Test that JSON contains the field name (not field ID since parser doesn't support it yet)
ResolvedReference<?> resolvedRef = (ResolvedReference<?>) predicate.term();
assertThat(json).contains(resolvedRef.name());
@@ -88,20 +89,20 @@ public class TestExpressionParserWithResolvedReference {
// Test that ResolvedReference expressions can be serialized and parsed back
Expression resolvedExpr = Expressions.equal(Expressions.ref("id", 100), 42L);
Expression namedExpr = Expressions.equal(Expressions.ref("id"), 42L);
-
+
// Both should produce the same JSON since parser only uses names
String resolvedJson = ExpressionParser.toJson(resolvedExpr, true);
String namedJson = ExpressionParser.toJson(namedExpr, true);
assertThat(resolvedJson).isEqualTo(namedJson);
-
+
// Parse back and verify equivalence
Expression parsedFromResolved = ExpressionParser.fromJson(resolvedJson, SCHEMA);
Expression parsedFromNamed = ExpressionParser.fromJson(namedJson, SCHEMA);
-
+
// Both parsed expressions should be equivalent
assertThat(ExpressionUtil.equivalent(parsedFromResolved, parsedFromNamed, STRUCT_TYPE, true))
.isTrue();
-
+
// The parsed expression should be equivalent to the original named reference expression
assertThat(ExpressionUtil.equivalent(namedExpr, parsedFromResolved, STRUCT_TYPE, true))
.isTrue();
@@ -110,59 +111,55 @@ public class TestExpressionParserWithResolvedReference {
@Test
public void testResolvedReferenceComplexExpressions() {
// Test complex expressions with ResolvedReference
- Expression complexExpr = Expressions.and(
- Expressions.or(
- Expressions.equal(Expressions.ref("data", 101), "test"),
- Expressions.isNull(Expressions.ref("data", 101))),
- Expressions.greaterThanOrEqual(Expressions.ref("id", 100), 100L));
-
+ Expression complexExpr =
+ Expressions.and(
+ Expressions.or(
+ Expressions.equal(Expressions.ref("data", 101), "test"),
+ Expressions.isNull(Expressions.ref("data", 101))),
+ Expressions.greaterThanOrEqual(Expressions.ref("id", 100), 100L));
+
// Serialize to JSON
String json = ExpressionParser.toJson(complexExpr, true);
assertThat(json).contains("\"type\" : \"and\"");
assertThat(json).contains("\"type\" : \"or\"");
assertThat(json).contains("\"data\"");
assertThat(json).contains("\"id\"");
-
+
// Parse back
Expression parsed = ExpressionParser.fromJson(json, SCHEMA);
-
+
// Create equivalent expression with NamedReference for comparison
- Expression namedEquivalent = Expressions.and(
- Expressions.or(
- Expressions.equal("data", "test"),
- Expressions.isNull("data")),
- Expressions.greaterThanOrEqual("id", 100L));
-
+ Expression namedEquivalent =
+ Expressions.and(
+ Expressions.or(Expressions.equal("data", "test"), Expressions.isNull("data")),
+ Expressions.greaterThanOrEqual("id", 100L));
+
// Should be equivalent
- assertThat(ExpressionUtil.equivalent(namedEquivalent, parsed, STRUCT_TYPE, true))
- .isTrue();
+ assertThat(ExpressionUtil.equivalent(namedEquivalent, parsed, STRUCT_TYPE, true)).isTrue();
}
@Test
public void testResolvedReferenceTransformExpressions() {
- // Test transform expressions - using NamedReference for transforms since
+ // Test transform expressions - using NamedReference for transforms since
// ResolvedReference needs proper type parameters for transform methods
- Expression dayTransform = Expressions.equal(
- Expressions.day("date"), "2023-01-15");
- Expression bucketTransform = Expressions.equal(
- Expressions.bucket("id", 10), 5);
-
+ Expression dayTransform = Expressions.equal(Expressions.day("date"), "2023-01-15");
+ Expression bucketTransform = Expressions.equal(Expressions.bucket("id", 10), 5);
+
// Test serialization
String dayJson = ExpressionParser.toJson(dayTransform, true);
String bucketJson = ExpressionParser.toJson(bucketTransform, true);
-
+
assertThat(dayJson).contains("\"transform\" : \"day\"");
assertThat(dayJson).contains("\"term\" : \"date\"");
assertThat(bucketJson).contains("\"transform\" : \"bucket[10]\"");
assertThat(bucketJson).contains("\"term\" : \"id\"");
-
+
// Test round-trip
Expression parsedDay = ExpressionParser.fromJson(dayJson, SCHEMA);
Expression parsedBucket = ExpressionParser.fromJson(bucketJson, SCHEMA);
-
+
// Should maintain equivalence after round-trip
- assertThat(ExpressionUtil.equivalent(dayTransform, parsedDay, STRUCT_TYPE, true))
- .isTrue();
+ assertThat(ExpressionUtil.equivalent(dayTransform, parsedDay, STRUCT_TYPE, true)).isTrue();
assertThat(ExpressionUtil.equivalent(bucketTransform, parsedBucket, STRUCT_TYPE, true))
.isTrue();
}
@@ -171,22 +168,22 @@ public class TestExpressionParserWithResolvedReference {
public void testResolvedReferenceBindingAfterParsing() {
// Test that expressions with ResolvedReference bind correctly after parsing
Expression original = Expressions.equal(Expressions.ref("id", 100), 42L);
-
+
// Serialize and parse
String json = ExpressionParser.toJson(original, true);
Expression parsed = ExpressionParser.fromJson(json, SCHEMA);
-
+
// Both should bind successfully
Expression originalBound = Binder.bind(STRUCT_TYPE, original, true);
Expression parsedBound = Binder.bind(STRUCT_TYPE, parsed, true);
-
+
// Both bound expressions should be identical
assertThat(originalBound).isInstanceOf(BoundPredicate.class);
assertThat(parsedBound).isInstanceOf(BoundPredicate.class);
-
+
BoundPredicate<?> originalBoundPred = (BoundPredicate<?>) originalBound;
BoundPredicate<?> parsedBoundPred = (BoundPredicate<?>) parsedBound;
-
+
// Should reference the same field
assertThat(originalBoundPred.ref().fieldId()).isEqualTo(100);
assertThat(parsedBoundPred.ref().fieldId()).isEqualTo(100);
@@ -197,26 +194,27 @@ public class TestExpressionParserWithResolvedReference {
@Test
public void testResolvedReferenceWithDifferentTypes() {
// Test ResolvedReference with various data types
- Expression[] typedExpressions = new Expression[] {
- Expressions.equal(Expressions.ref("b", 102), true),
- Expressions.equal(Expressions.ref("i", 103), 42),
- Expressions.equal(Expressions.ref("l", 104), 42L),
- Expressions.equal(Expressions.ref("f", 105), 3.14f),
- Expressions.equal(Expressions.ref("d", 106), 3.14159),
- Expressions.equal(Expressions.ref("s", 110), "test string"),
- Expressions.equal(Expressions.ref("uuid", 111), UUID.randomUUID()),
- Expressions.equal(Expressions.ref("dec_11_2", 115), new BigDecimal("123.45"))
- };
-
+ Expression[] typedExpressions =
+ new Expression[] {
+ Expressions.equal(Expressions.ref("b", 102), true),
+ Expressions.equal(Expressions.ref("i", 103), 42),
+ Expressions.equal(Expressions.ref("l", 104), 42L),
+ Expressions.equal(Expressions.ref("f", 105), 3.14f),
+ Expressions.equal(Expressions.ref("d", 106), 3.14159),
+ Expressions.equal(Expressions.ref("s", 110), "test string"),
+ Expressions.equal(Expressions.ref("uuid", 111), UUID.randomUUID()),
+ Expressions.equal(Expressions.ref("dec_11_2", 115), new BigDecimal("123.45"))
+ };
+
for (Expression expr : typedExpressions) {
// Test serialization doesn't break with different types
String json = ExpressionParser.toJson(expr, true);
assertThat(json).isNotNull();
-
+
// Test parsing back
Expression parsed = ExpressionParser.fromJson(json, SCHEMA);
assertThat(parsed).isNotNull();
-
+
// Test binding
Expression bound = Binder.bind(STRUCT_TYPE, parsed, true);
assertThat(bound).isInstanceOf(BoundPredicate.class);
@@ -228,20 +226,21 @@ public class TestExpressionParserWithResolvedReference {
// Test the exact JSON structure produced by ResolvedReference
Expression expr = Expressions.equal(Expressions.ref("data", 101), "test");
String json = ExpressionParser.toJson(expr, true);
-
+
// The JSON should look like a regular reference since parser doesn't support field IDs yet
- String expectedStructure = "{\n" +
- " \"type\" : \"eq\",\n" +
- " \"term\" : \"data\",\n" +
- " \"value\" : \"test\"\n" +
- "}";
-
+ String expectedStructure =
+ "{\n"
+ + " \"type\" : \"eq\",\n"
+ + " \"term\" : \"data\",\n"
+ + " \"value\" : \"test\"\n"
+ + "}";
+
assertThat(json).isEqualTo(expectedStructure);
-
+
// Verify it parses back correctly
Expression parsed = ExpressionParser.fromJson(json);
assertThat(parsed).isInstanceOf(UnboundPredicate.class);
-
+
UnboundPredicate<?> predicate = (UnboundPredicate<?>) parsed;
assertThat(predicate.term()).isInstanceOf(NamedReference.class);
assertThat(predicate.term().ref().name()).isEqualTo("data");
@@ -250,58 +249,57 @@ public class TestExpressionParserWithResolvedReference {
@Test
public void testResolvedReferenceEquivalenceAfterSerialization() {
// Test that ResolvedReference expressions maintain equivalence after serialization
- Expression resolvedExpr = Expressions.and(
- Expressions.greaterThan(Expressions.ref("id", 100), 50L),
- Expressions.lessThan(Expressions.ref("id", 100), 200L));
-
- Expression namedExpr = Expressions.and(
- Expressions.greaterThan("id", 50L),
- Expressions.lessThan("id", 200L));
-
+ Expression resolvedExpr =
+ Expressions.and(
+ Expressions.greaterThan(Expressions.ref("id", 100), 50L),
+ Expressions.lessThan(Expressions.ref("id", 100), 200L));
+
+ Expression namedExpr =
+ Expressions.and(Expressions.greaterThan("id", 50L), Expressions.lessThan("id", 200L));
+
// Serialize both
String resolvedJson = ExpressionParser.toJson(resolvedExpr, true);
String namedJson = ExpressionParser.toJson(namedExpr, true);
-
+
// Should produce identical JSON
assertThat(resolvedJson).isEqualTo(namedJson);
-
+
// Parse both back
Expression parsedResolved = ExpressionParser.fromJson(resolvedJson, SCHEMA);
Expression parsedNamed = ExpressionParser.fromJson(namedJson, SCHEMA);
-
+
// All should be equivalent
- assertThat(ExpressionUtil.equivalent(resolvedExpr, namedExpr, STRUCT_TYPE, true))
- .isTrue();
- assertThat(ExpressionUtil.equivalent(parsedResolved, parsedNamed, STRUCT_TYPE, true))
- .isTrue();
- assertThat(ExpressionUtil.equivalent(resolvedExpr, parsedResolved, STRUCT_TYPE, true))
- .isTrue();
+ assertThat(ExpressionUtil.equivalent(resolvedExpr, namedExpr, STRUCT_TYPE, true)).isTrue();
+ assertThat(ExpressionUtil.equivalent(parsedResolved, parsedNamed, STRUCT_TYPE, true)).isTrue();
+ assertThat(ExpressionUtil.equivalent(resolvedExpr, parsedResolved, STRUCT_TYPE, true)).isTrue();
}
@Test
public void testResolvedReferenceTransformExpressionEquivalence() {
- // Test expressions that reference transforms where the transform terms are equivalent to resolved references
- // Since UnboundTransform only accepts NamedReference, we test equivalence through binding/unbinding
-
+ // Test expressions that reference transforms where the transform terms are equivalent to
+ // resolved references
+ // Since UnboundTransform only accepts NamedReference, we test equivalence through
+ // binding/unbinding
+
// Create transform expressions using NamedReference (current approach)
Expression dayTransformNamed = Expressions.equal(Expressions.day("date"), "2023-01-15");
Expression bucketTransformNamed = Expressions.equal(Expressions.bucket("id", 10), 5);
Expression truncateTransformNamed = Expressions.equal(Expressions.truncate("data", 4), "test");
-
+
// Create equivalent expressions using ResolvedReference for the predicate terms
Expression dayWithResolvedRef = Expressions.equal(Expressions.ref("date", 107), "2023-01-15");
Expression bucketWithResolvedRef = Expressions.equal(Expressions.ref("id", 100), 5L);
Expression truncateWithResolvedRef = Expressions.equal(Expressions.ref("data", 101), "test");
-
+
// Bind all expressions
Expression boundDayTransform = Binder.bind(STRUCT_TYPE, dayTransformNamed, true);
Expression boundBucketTransform = Binder.bind(STRUCT_TYPE, bucketTransformNamed, true);
Expression boundTruncateTransform = Binder.bind(STRUCT_TYPE, truncateTransformNamed, true);
-
+
Expression boundDayResolved = Binder.bind(STRUCT_TYPE, dayWithResolvedRef, true);
Expression boundBucketResolved = Binder.bind(STRUCT_TYPE, bucketWithResolvedRef, true);
Expression boundTruncateResolved = Binder.bind(STRUCT_TYPE, truncateWithResolvedRef, true);
-
+
// Verify all expressions bound successfully
assertThat(boundDayTransform).isInstanceOf(BoundPredicate.class);
assertThat(boundBucketTransform).isInstanceOf(BoundPredicate.class);
@@ -309,40 +307,42 @@ public class TestExpressionParserWithResolvedReference {
assertThat(boundDayResolved).isInstanceOf(BoundPredicate.class);
assertThat(boundBucketResolved).isInstanceOf(BoundPredicate.class);
assertThat(boundTruncateResolved).isInstanceOf(BoundPredicate.class);
-
+
// Test transform expressions in complex expressions with resolved references
- Expression complexTransformExpr = Expressions.and(
- Expressions.equal(Expressions.day("date"), "2023-01-15"),
- Expressions.equal(Expressions.ref("id", 100), 42L));
-
+ Expression complexTransformExpr =
+ Expressions.and(
+ Expressions.equal(Expressions.day("date"), "2023-01-15"),
+ Expressions.equal(Expressions.ref("id", 100), 42L));
+
Expression boundComplexExpr = Binder.bind(STRUCT_TYPE, complexTransformExpr, true);
assertThat(boundComplexExpr).isInstanceOf(And.class);
-
+
// Verify serialization works for transform expressions that coexist with resolved references
String complexJson = ExpressionParser.toJson(complexTransformExpr, true);
assertThat(complexJson).contains("\"transform\" : \"day\"");
assertThat(complexJson).contains("\"term\" : \"date\"");
assertThat(complexJson).contains("\"term\" : \"id\"");
-
+
// Verify round-trip maintains correctness
Expression parsedComplex = ExpressionParser.fromJson(complexJson, SCHEMA);
Expression boundParsedComplex = Binder.bind(STRUCT_TYPE, parsedComplex, true);
-
+
// Both bound expressions should reference the same fields
assertThat(boundComplexExpr.toString()).isEqualTo(boundParsedComplex.toString());
}
- @Test
+ @Test
public void testResolvedReferenceInComplexTransformExpressions() {
// Test complex expressions that combine transforms with resolved references
- Expression complexExpr = Expressions.or(
- Expressions.and(
- Expressions.equal(Expressions.bucket("id", 8), 3),
- Expressions.equal(Expressions.ref("data", 101), "test")),
- Expressions.and(
- Expressions.equal(Expressions.day("date"), "2023-01-15"),
- Expressions.isNull(Expressions.ref("f", 105))));
-
+ Expression complexExpr =
+ Expressions.or(
+ Expressions.and(
+ Expressions.equal(Expressions.bucket("id", 8), 3),
+ Expressions.equal(Expressions.ref("data", 101), "test")),
+ Expressions.and(
+ Expressions.equal(Expressions.day("date"), "2023-01-15"),
+ Expressions.isNull(Expressions.ref("f", 105))));
+
// Test serialization
String json = ExpressionParser.toJson(complexExpr, true);
assertThat(json).contains("\"transform\" : \"bucket[8]\"");
@@ -351,21 +351,21 @@ public class TestExpressionParserWithResolvedReference {
assertThat(json).contains("\"term\" : \"date\"");
assertThat(json).contains("\"term\" : \"data\"");
assertThat(json).contains("\"term\" : \"f\"");
-
+
// Test that parsing back maintains structure
Expression parsed = ExpressionParser.fromJson(json, SCHEMA);
assertThat(parsed).isInstanceOf(Or.class);
-
+
// Test binding works correctly
Expression bound = Binder.bind(STRUCT_TYPE, parsed, true);
assertThat(bound).isInstanceOf(Or.class);
-
+
// Verify equivalence with original
assertThat(ExpressionUtil.equivalent(complexExpr, parsed, STRUCT_TYPE, true)).isTrue();
-
+
// Test that mixed transform and resolved reference expressions bind to same fields
Expression originalBound = Binder.bind(STRUCT_TYPE, complexExpr, true);
-
+
// Both bound expressions should be structurally equivalent
assertThat(originalBound.toString()).isEqualTo(bound.toString());
}
@@ -374,38 +374,36 @@ public class TestExpressionParserWithResolvedReference {
public void testTransformExpressionsWithResolvedReference() {
// Test expressions that reference transforms which in turn reference resolved references
// Create transforms using the new ResolvedReference-based factory methods
- Expression bucketExpr = Expressions.equal(
- Expressions.bucket(Expressions.ref("id", 100), 8), 3);
- Expression dayExpr = Expressions.equal(
- Expressions.day(Expressions.ref("date", 107)), "2023-01-15");
- Expression hourExpr = Expressions.equal(
- Expressions.hour(Expressions.ref("ts", 108)), 10);
- Expression truncateExpr = Expressions.equal(
- Expressions.truncate(Expressions.ref("data", 101), 4), "test");
-
+ Expression bucketExpr = Expressions.equal(Expressions.bucket(Expressions.ref("id", 100), 8), 3);
+ Expression dayExpr =
+ Expressions.equal(Expressions.day(Expressions.ref("date", 107)), "2023-01-15");
+ Expression hourExpr = Expressions.equal(Expressions.hour(Expressions.ref("ts", 108)), 10);
+ Expression truncateExpr =
+ Expressions.equal(Expressions.truncate(Expressions.ref("data", 101), 4), "test");
+
// Verify the expressions are created correctly
assertThat(bucketExpr).isInstanceOf(UnboundPredicate.class);
assertThat(dayExpr).isInstanceOf(UnboundPredicate.class);
assertThat(hourExpr).isInstanceOf(UnboundPredicate.class);
assertThat(truncateExpr).isInstanceOf(UnboundPredicate.class);
-
+
// Verify the terms are ResolvedTransform instances
UnboundPredicate<?> bucketPred = (UnboundPredicate<?>) bucketExpr;
UnboundPredicate<?> dayPred = (UnboundPredicate<?>) dayExpr;
UnboundPredicate<?> hourPred = (UnboundPredicate<?>) hourExpr;
UnboundPredicate<?> truncatePred = (UnboundPredicate<?>) truncateExpr;
-
+
assertThat(bucketPred.term()).isInstanceOf(ResolvedTransform.class);
assertThat(dayPred.term()).isInstanceOf(ResolvedTransform.class);
assertThat(hourPred.term()).isInstanceOf(ResolvedTransform.class);
assertThat(truncatePred.term()).isInstanceOf(ResolvedTransform.class);
-
+
// Verify that ResolvedTransform preserves the ResolvedReference with field IDs
ResolvedTransform<?, ?> bucketTransform = (ResolvedTransform<?, ?>) bucketPred.term();
ResolvedTransform<?, ?> dayTransform = (ResolvedTransform<?, ?>) dayPred.term();
ResolvedTransform<?, ?> hourTransform = (ResolvedTransform<?, ?>) hourPred.term();
ResolvedTransform<?, ?> truncateTransform = (ResolvedTransform<?, ?>) truncatePred.term();
-
+
assertThat(bucketTransform.resolvedRef().fieldId()).isEqualTo(100);
assertThat(bucketTransform.resolvedRef().name()).isEqualTo("id");
assertThat(dayTransform.resolvedRef().fieldId()).isEqualTo(107);
@@ -414,13 +412,13 @@ public class TestExpressionParserWithResolvedReference {
assertThat(hourTransform.resolvedRef().name()).isEqualTo("ts");
assertThat(truncateTransform.resolvedRef().fieldId()).isEqualTo(101);
assertThat(truncateTransform.resolvedRef().name()).isEqualTo("data");
-
+
// Test serialization
String bucketJson = ExpressionParser.toJson(bucketExpr, true);
String dayJson = ExpressionParser.toJson(dayExpr, true);
String hourJson = ExpressionParser.toJson(hourExpr, true);
String truncateJson = ExpressionParser.toJson(truncateExpr, true);
-
+
// Verify JSON contains the expected transform and term information
assertThat(bucketJson).contains("\"transform\" : \"bucket[8]\"");
assertThat(bucketJson).contains("\"term\" : \"id\"");
@@ -430,36 +428,36 @@ public class TestExpressionParserWithResolvedReference {
assertThat(hourJson).contains("\"term\" : \"ts\"");
assertThat(truncateJson).contains("\"transform\" : \"truncate[4]\"");
assertThat(truncateJson).contains("\"term\" : \"data\"");
-
+
// Test parsing back
Expression parsedBucket = ExpressionParser.fromJson(bucketJson, SCHEMA);
Expression parsedDay = ExpressionParser.fromJson(dayJson, SCHEMA);
Expression parsedHour = ExpressionParser.fromJson(hourJson, SCHEMA);
Expression parsedTruncate = ExpressionParser.fromJson(truncateJson, SCHEMA);
-
+
// Verify equivalence after round-trip
assertThat(ExpressionUtil.equivalent(bucketExpr, parsedBucket, STRUCT_TYPE, true)).isTrue();
assertThat(ExpressionUtil.equivalent(dayExpr, parsedDay, STRUCT_TYPE, true)).isTrue();
assertThat(ExpressionUtil.equivalent(hourExpr, parsedHour, STRUCT_TYPE, true)).isTrue();
assertThat(ExpressionUtil.equivalent(truncateExpr, parsedTruncate, STRUCT_TYPE, true)).isTrue();
-
+
// Test binding works correctly
Expression boundBucket = Binder.bind(STRUCT_TYPE, parsedBucket, true);
Expression boundDay = Binder.bind(STRUCT_TYPE, parsedDay, true);
Expression boundHour = Binder.bind(STRUCT_TYPE, parsedHour, true);
Expression boundTruncate = Binder.bind(STRUCT_TYPE, parsedTruncate, true);
-
+
assertThat(boundBucket).isInstanceOf(BoundPredicate.class);
assertThat(boundDay).isInstanceOf(BoundPredicate.class);
assertThat(boundHour).isInstanceOf(BoundPredicate.class);
assertThat(boundTruncate).isInstanceOf(BoundPredicate.class);
-
+
// Verify bound expressions reference correct field IDs
BoundPredicate<?> boundBucketPred = (BoundPredicate<?>) boundBucket;
BoundPredicate<?> boundDayPred = (BoundPredicate<?>) boundDay;
BoundPredicate<?> boundHourPred = (BoundPredicate<?>) boundHour;
BoundPredicate<?> boundTruncatePred = (BoundPredicate<?>) boundTruncate;
-
+
assertThat(boundBucketPred.term()).isInstanceOf(BoundTransform.class);
assertThat(boundDayPred.term()).isInstanceOf(BoundTransform.class);
assertThat(boundHourPred.term()).isInstanceOf(BoundTransform.class);
@@ -469,17 +467,18 @@ public class TestExpressionParserWithResolvedReference {
@Test
public void testComplexExpressionsWithResolvedReferenceTransforms() {
// Test complex expressions combining transforms created from ResolvedReference
- Expression complexExpr = Expressions.and(
- Expressions.or(
- Expressions.equal(Expressions.bucket(Expressions.ref("id", 100), 8), 3),
- Expressions.equal(Expressions.day(Expressions.ref("date", 107)), "2023-01-15")),
+ Expression complexExpr =
Expressions.and(
- Expressions.equal(Expressions.truncate(Expressions.ref("data", 101), 4), "test"),
- Expressions.isNull(Expressions.ref("f", 105))));
-
+ Expressions.or(
+ Expressions.equal(Expressions.bucket(Expressions.ref("id", 100), 8), 3),
+ Expressions.equal(Expressions.day(Expressions.ref("date", 107)), "2023-01-15")),
+ Expressions.and(
+ Expressions.equal(Expressions.truncate(Expressions.ref("data", 101), 4), "test"),
+ Expressions.isNull(Expressions.ref("f", 105))));
+
// Test serialization of complex expression
String json = ExpressionParser.toJson(complexExpr, true);
-
+
// Verify all transforms and terms are present in JSON
assertThat(json).contains("\"transform\" : \"bucket[8]\"");
assertThat(json).contains("\"transform\" : \"day\"");
@@ -488,18 +487,18 @@ public class TestExpressionParserWithResolvedReference {
assertThat(json).contains("\"term\" : \"date\"");
assertThat(json).contains("\"term\" : \"data\"");
assertThat(json).contains("\"term\" : \"f\"");
-
+
// Test parsing back maintains structure
Expression parsed = ExpressionParser.fromJson(json, SCHEMA);
assertThat(parsed).isInstanceOf(And.class);
-
+
// Test binding works correctly for the complex expression
Expression bound = Binder.bind(STRUCT_TYPE, parsed, true);
assertThat(bound).isInstanceOf(And.class);
-
+
// Verify equivalence after round-trip
assertThat(ExpressionUtil.equivalent(complexExpr, parsed, STRUCT_TYPE, true)).isTrue();
-
+
// Test that the bound expression maintains correct structure
Expression originalBound = Binder.bind(STRUCT_TYPE, complexExpr, true);
assertThat(originalBound.toString()).isEqualTo(bound.toString());
@@ -507,46 +506,50 @@ public class TestExpressionParserWithResolvedReference {
@Test
public void testResolvedReferenceTransformCompatibilityWithNamedReference() {
- // Test that transforms created with ResolvedReference are equivalent to those created with NamedReference
-
+ // Test that transforms created with ResolvedReference are equivalent to those created with
+ // NamedReference
+
// Create equivalent transforms using both approaches
- Expression bucketWithResolved = Expressions.equal(
- Expressions.bucket(Expressions.ref("id", 100), 8), 3);
- Expression bucketWithNamed = Expressions.equal(
- Expressions.bucket("id", 8), 3);
-
- Expression dayWithResolved = Expressions.equal(
- Expressions.day(Expressions.ref("date", 107)), "2023-01-15");
- Expression dayWithNamed = Expressions.equal(
- Expressions.day("date"), "2023-01-15");
-
+ Expression bucketWithResolved =
+ Expressions.equal(Expressions.bucket(Expressions.ref("id", 100), 8), 3);
+ Expression bucketWithNamed = Expressions.equal(Expressions.bucket("id", 8), 3);
+
+ Expression dayWithResolved =
+ Expressions.equal(Expressions.day(Expressions.ref("date", 107)), "2023-01-15");
+ Expression dayWithNamed = Expressions.equal(Expressions.day("date"), "2023-01-15");
+
// Test that both serialize to the same JSON
String resolvedBucketJson = ExpressionParser.toJson(bucketWithResolved, true);
String namedBucketJson = ExpressionParser.toJson(bucketWithNamed, true);
String resolvedDayJson = ExpressionParser.toJson(dayWithResolved, true);
String namedDayJson = ExpressionParser.toJson(dayWithNamed, true);
-
+
assertThat(resolvedBucketJson).isEqualTo(namedBucketJson);
assertThat(resolvedDayJson).isEqualTo(namedDayJson);
-
+
// Test that parsing back produces equivalent expressions
Expression parsedResolvedBucket = ExpressionParser.fromJson(resolvedBucketJson, SCHEMA);
Expression parsedNamedBucket = ExpressionParser.fromJson(namedBucketJson, SCHEMA);
Expression parsedResolvedDay = ExpressionParser.fromJson(resolvedDayJson, SCHEMA);
Expression parsedNamedDay = ExpressionParser.fromJson(namedDayJson, SCHEMA);
-
+
// All should be equivalent
- assertThat(ExpressionUtil.equivalent(bucketWithResolved, bucketWithNamed, STRUCT_TYPE, true)).isTrue();
- assertThat(ExpressionUtil.equivalent(dayWithResolved, dayWithNamed, STRUCT_TYPE, true)).isTrue();
- assertThat(ExpressionUtil.equivalent(parsedResolvedBucket, parsedNamedBucket, STRUCT_TYPE, true)).isTrue();
- assertThat(ExpressionUtil.equivalent(parsedResolvedDay, parsedNamedDay, STRUCT_TYPE, true)).isTrue();
-
+ assertThat(ExpressionUtil.equivalent(bucketWithResolved, bucketWithNamed, STRUCT_TYPE, true))
+ .isTrue();
+ assertThat(ExpressionUtil.equivalent(dayWithResolved, dayWithNamed, STRUCT_TYPE, true))
+ .isTrue();
+ assertThat(
+ ExpressionUtil.equivalent(parsedResolvedBucket, parsedNamedBucket, STRUCT_TYPE, true))
+ .isTrue();
+ assertThat(ExpressionUtil.equivalent(parsedResolvedDay, parsedNamedDay, STRUCT_TYPE, true))
+ .isTrue();
+
// Test that binding produces identical results
Expression boundResolvedBucket = Binder.bind(STRUCT_TYPE, parsedResolvedBucket, true);
Expression boundNamedBucket = Binder.bind(STRUCT_TYPE, parsedNamedBucket, true);
Expression boundResolvedDay = Binder.bind(STRUCT_TYPE, parsedResolvedDay, true);
Expression boundNamedDay = Binder.bind(STRUCT_TYPE, parsedNamedDay, true);
-
+
assertThat(boundResolvedBucket.toString()).isEqualTo(boundNamedBucket.toString());
assertThat(boundResolvedDay.toString()).isEqualTo(boundNamedDay.toString());
}
@@ -555,40 +558,40 @@ public class TestExpressionParserWithResolvedReference {
public void testResolvedTransformPreservesFieldIdInformation() {
// Test that ResolvedTransform preserves field ID information through binding
// This is the key advantage over UnboundTransform with NamedReference
-
+
// Create a transform expression using ResolvedReference
ResolvedReference<Long> idRef = Expressions.ref("id", 100);
Expression bucketExpr = Expressions.equal(Expressions.bucket(idRef, 8), 3);
-
+
// Verify it's a ResolvedTransform
UnboundPredicate<?> predicate = (UnboundPredicate<?>) bucketExpr;
assertThat(predicate.term()).isInstanceOf(ResolvedTransform.class);
-
+
ResolvedTransform<?, ?> resolvedTransform = (ResolvedTransform<?, ?>) predicate.term();
-
+
// Verify the ResolvedReference is preserved with field ID
assertThat(resolvedTransform.resolvedRef().fieldId()).isEqualTo(100);
assertThat(resolvedTransform.resolvedRef().name()).isEqualTo("id");
-
+
// Test binding works correctly with field ID information
Expression bound = Binder.bind(STRUCT_TYPE, bucketExpr, true);
assertThat(bound).isInstanceOf(BoundPredicate.class);
-
+
BoundPredicate<?> boundPredicate = (BoundPredicate<?>) bound;
assertThat(boundPredicate.term()).isInstanceOf(BoundTransform.class);
-
+
BoundTransform<?, ?> boundTransform = (BoundTransform<?, ?>) boundPredicate.term();
assertThat(boundTransform.ref().fieldId()).isEqualTo(100);
assertThat(boundTransform.ref().name()).isEqualTo("id");
-
+
// Compare with equivalent NamedReference approach to show they bind to same field
Expression namedBucketExpr = Expressions.equal(Expressions.bucket("id", 8), 3);
Expression boundNamed = Binder.bind(STRUCT_TYPE, namedBucketExpr, true);
-
+
// Both should resolve to the same field ID since they reference the same field
BoundPredicate<?> boundNamedPredicate = (BoundPredicate<?>) boundNamed;
BoundTransform<?, ?> boundNamedTransform = (BoundTransform<?, ?>) boundNamedPredicate.term();
-
+
assertThat(boundTransform.ref().fieldId()).isEqualTo(boundNamedTransform.ref().fieldId());
assertThat(boundTransform.toString()).isEqualTo(boundNamedTransform.toString());
}
@@ -596,43 +599,42 @@ public class TestExpressionParserWithResolvedReference {
@Test
public void testResolvedTransformExpressionParserIntegration() {
// Test that expressions with ResolvedTransform integrate correctly with ExpressionParser
-
+
// Create expressions using ResolvedTransform
- Expression bucketExpr = Expressions.equal(
- Expressions.bucket(Expressions.ref("id", 100), 8), 3);
- Expression dayExpr = Expressions.equal(
- Expressions.day(Expressions.ref("date", 107)), "2023-01-15");
-
+ Expression bucketExpr = Expressions.equal(Expressions.bucket(Expressions.ref("id", 100), 8), 3);
+ Expression dayExpr =
+ Expressions.equal(Expressions.day(Expressions.ref("date", 107)), "2023-01-15");
+
// Test that they can be serialized (even though they become NamedReference in JSON)
String bucketJson = ExpressionParser.toJson(bucketExpr, true);
String dayJson = ExpressionParser.toJson(dayExpr, true);
-
+
assertThat(bucketJson).contains("\"transform\" : \"bucket[8]\"");
assertThat(bucketJson).contains("\"term\" : \"id\"");
assertThat(dayJson).contains("\"transform\" : \"day\"");
assertThat(dayJson).contains("\"term\" : \"date\"");
-
+
// Test parsing back (will create UnboundTransform with NamedReference)
Expression parsedBucket = ExpressionParser.fromJson(bucketJson, SCHEMA);
Expression parsedDay = ExpressionParser.fromJson(dayJson, SCHEMA);
-
+
// The parsed expressions will have UnboundTransform (not ResolvedTransform)
// but they should still be functionally equivalent when bound
UnboundPredicate<?> parsedBucketPred = (UnboundPredicate<?>) parsedBucket;
UnboundPredicate<?> parsedDayPred = (UnboundPredicate<?>) parsedDay;
-
+
assertThat(parsedBucketPred.term()).isInstanceOf(UnboundTransform.class);
assertThat(parsedDayPred.term()).isInstanceOf(UnboundTransform.class);
-
+
// Both original and parsed should bind to the same fields
Expression originalBucketBound = Binder.bind(STRUCT_TYPE, bucketExpr, true);
Expression parsedBucketBound = Binder.bind(STRUCT_TYPE, parsedBucket, true);
Expression originalDayBound = Binder.bind(STRUCT_TYPE, dayExpr, true);
Expression parsedDayBound = Binder.bind(STRUCT_TYPE, parsedDay, true);
-
+
assertThat(originalBucketBound.toString()).isEqualTo(parsedBucketBound.toString());
assertThat(originalDayBound.toString()).isEqualTo(parsedDayBound.toString());
-
+
// Test equivalence
assertThat(ExpressionUtil.equivalent(bucketExpr, parsedBucket, STRUCT_TYPE, true)).isTrue();
assertThat(ExpressionUtil.equivalent(dayExpr, parsedDay, STRUCT_TYPE, true)).isTrue();
@@ -641,65 +643,68 @@ public class TestExpressionParserWithResolvedReference {
@Test
public void testMixedResolvedTransformAndResolvedReferenceExpressions() {
// Test complex expressions mixing ResolvedTransform and direct ResolvedReference
- Expression complexExpr = Expressions.and(
- Expressions.equal(Expressions.bucket(Expressions.ref("id", 100), 8), 3),
- Expressions.or(
- Expressions.equal(Expressions.ref("data", 101), "test"),
- Expressions.equal(Expressions.day(Expressions.ref("date", 107)), "2023-01-15")));
-
+ Expression complexExpr =
+ Expressions.and(
+ Expressions.equal(Expressions.bucket(Expressions.ref("id", 100), 8), 3),
+ Expressions.or(
+ Expressions.equal(Expressions.ref("data", 101), "test"),
+ Expressions.equal(Expressions.day(Expressions.ref("date", 107)), "2023-01-15")));
+
// Verify structure contains both ResolvedTransform and ResolvedReference
assertThat(complexExpr).isInstanceOf(And.class);
And andExpr = (And) complexExpr;
-
+
// First part should be ResolvedTransform
UnboundPredicate<?> bucketPred = (UnboundPredicate<?>) andExpr.left();
assertThat(bucketPred.term()).isInstanceOf(ResolvedTransform.class);
-
+
// Second part is OR with ResolvedReference and ResolvedTransform
Or orExpr = (Or) andExpr.right();
UnboundPredicate<?> dataPred = (UnboundPredicate<?>) orExpr.left();
UnboundPredicate<?> dayPred = (UnboundPredicate<?>) orExpr.right();
-
+
assertThat(dataPred.term()).isInstanceOf(ResolvedReference.class);
assertThat(dayPred.term()).isInstanceOf(ResolvedTransform.class);
-
+
// Test serialization and parsing
String json = ExpressionParser.toJson(complexExpr, true);
Expression parsed = ExpressionParser.fromJson(json, SCHEMA);
-
+
// Test binding works correctly
Expression bound = Binder.bind(STRUCT_TYPE, parsed, true);
Expression originalBound = Binder.bind(STRUCT_TYPE, complexExpr, true);
-
+
assertThat(bound.toString()).isEqualTo(originalBound.toString());
assertThat(ExpressionUtil.equivalent(complexExpr, parsed, STRUCT_TYPE, true)).isTrue();
}
@Test
public void testBoundExpressionSerializationWithResolvedReference() {
- // Test that bound expressions serialize to JSON with ResolvedReference terms when includeFieldIds=true
-
+ // Test that bound expressions serialize to JSON with ResolvedReference terms when
+ // includeFieldIds=true
+
// Create and bind simple expressions
Expression simpleExpr = Expressions.equal(Expressions.ref("id", 100), 42L);
Expression boundSimple = Binder.bind(STRUCT_TYPE, simpleExpr, true);
-
+
// Test serialization without field IDs (existing behavior)
String jsonWithoutFieldIds = ExpressionParser.toJson(boundSimple, true);
assertThat(jsonWithoutFieldIds).contains("\"term\" : \"id\"");
assertThat(jsonWithoutFieldIds).doesNotContain("fieldId");
-
+
// Test serialization with field IDs (new behavior)
String jsonWithFieldIds = ExpressionParser.toJson(boundSimple, true, true);
assertThat(jsonWithFieldIds).contains("\"type\" : \"ref\"");
assertThat(jsonWithFieldIds).contains("\"name\" : \"id\"");
assertThat(jsonWithFieldIds).contains("\"fieldId\" : 100");
-
+
// Test complex expressions
- Expression complexExpr = Expressions.and(
- Expressions.equal(Expressions.ref("data", 101), "test"),
- Expressions.greaterThan(Expressions.ref("id", 100), 50L));
+ Expression complexExpr =
+ Expressions.and(
+ Expressions.equal(Expressions.ref("data", 101), "test"),
+ Expressions.greaterThan(Expressions.ref("id", 100), 50L));
Expression boundComplex = Binder.bind(STRUCT_TYPE, complexExpr, true);
-
+
String complexJsonWithFieldIds = ExpressionParser.toJson(boundComplex, true, true);
assertThat(complexJsonWithFieldIds).contains("\"fieldId\" : 100");
assertThat(complexJsonWithFieldIds).contains("\"fieldId\" : 101");
@@ -709,31 +714,31 @@ public class TestExpressionParserWithResolvedReference {
@Test
public void testBoundTransformExpressionSerializationWithResolvedReference() {
- // Test that bound transform expressions serialize to JSON with ResolvedReference terms when includeFieldIds=true
-
+ // Test that bound transform expressions serialize to JSON with ResolvedReference terms when
+ // includeFieldIds=true
+
// Create transform expressions using ResolvedTransform
- Expression bucketExpr = Expressions.equal(
- Expressions.bucket(Expressions.ref("id", 100), 8), 3);
- Expression dayExpr = Expressions.equal(
- Expressions.day(Expressions.ref("date", 107)), "2023-01-15");
-
+ Expression bucketExpr = Expressions.equal(Expressions.bucket(Expressions.ref("id", 100), 8), 3);
+ Expression dayExpr =
+ Expressions.equal(Expressions.day(Expressions.ref("date", 107)), "2023-01-15");
+
// Bind the expressions
Expression boundBucket = Binder.bind(STRUCT_TYPE, bucketExpr, true);
Expression boundDay = Binder.bind(STRUCT_TYPE, dayExpr, true);
-
+
// Test serialization without field IDs (existing behavior)
String bucketJsonNoFieldIds = ExpressionParser.toJson(boundBucket, true);
assertThat(bucketJsonNoFieldIds).contains("\"transform\" : \"bucket[8]\"");
assertThat(bucketJsonNoFieldIds).contains("\"term\" : \"id\"");
assertThat(bucketJsonNoFieldIds).doesNotContain("fieldId");
-
+
// Test serialization with field IDs (new behavior)
String bucketJsonWithFieldIds = ExpressionParser.toJson(boundBucket, true, true);
assertThat(bucketJsonWithFieldIds).contains("\"transform\" : \"bucket[8]\"");
assertThat(bucketJsonWithFieldIds).contains("\"type\" : \"ref\"");
assertThat(bucketJsonWithFieldIds).contains("\"name\" : \"id\"");
assertThat(bucketJsonWithFieldIds).contains("\"fieldId\" : 100");
-
+
String dayJsonWithFieldIds = ExpressionParser.toJson(boundDay, true, true);
assertThat(dayJsonWithFieldIds).contains("\"transform\" : \"day\"");
assertThat(dayJsonWithFieldIds).contains("\"type\" : \"ref\"");
@@ -744,83 +749,89 @@ public class TestExpressionParserWithResolvedReference {
@Test
public void testComplexBoundTransformExpressionSerializationWithResolvedReference() {
// Test complex bound expressions with mixed transforms and references
- Expression complexExpr = Expressions.and(
- Expressions.or(
- Expressions.equal(Expressions.bucket(Expressions.ref("id", 100), 8), 3),
- Expressions.equal(Expressions.day(Expressions.ref("date", 107)), "2023-01-15")),
+ Expression complexExpr =
Expressions.and(
- Expressions.equal(Expressions.truncate(Expressions.ref("data", 101), 4), "test"),
- Expressions.isNull(Expressions.ref("f", 105))));
-
+ Expressions.or(
+ Expressions.equal(Expressions.bucket(Expressions.ref("id", 100), 8), 3),
+ Expressions.equal(Expressions.day(Expressions.ref("date", 107)), "2023-01-15")),
+ Expressions.and(
+ Expressions.equal(Expressions.truncate(Expressions.ref("data", 101), 4), "test"),
+ Expressions.isNull(Expressions.ref("f", 105))));
+
// Bind the complex expression
Expression bound = Binder.bind(STRUCT_TYPE, complexExpr, true);
-
+
// Test serialization with field IDs
String jsonWithFieldIds = ExpressionParser.toJson(bound, true, true);
-
+
// Verify all field IDs are present
assertThat(jsonWithFieldIds).contains("\"fieldId\" : 100"); // id field
- assertThat(jsonWithFieldIds).contains("\"fieldId\" : 107"); // date field
+ assertThat(jsonWithFieldIds).contains("\"fieldId\" : 107"); // date field
assertThat(jsonWithFieldIds).contains("\"fieldId\" : 101"); // data field
assertThat(jsonWithFieldIds).contains("\"fieldId\" : 105"); // f field
-
+
// Verify all field names are present
assertThat(jsonWithFieldIds).contains("\"name\" : \"id\"");
assertThat(jsonWithFieldIds).contains("\"name\" : \"date\"");
assertThat(jsonWithFieldIds).contains("\"name\" : \"data\"");
assertThat(jsonWithFieldIds).contains("\"name\" : \"f\"");
-
+
// Verify transforms are serialized correctly
assertThat(jsonWithFieldIds).contains("\"transform\" : \"bucket[8]\"");
assertThat(jsonWithFieldIds).contains("\"transform\" : \"day\"");
assertThat(jsonWithFieldIds).contains("\"transform\" : \"truncate[4]\"");
-
+
// Verify ResolvedReference structure for both transforms and direct references
// Count occurrences of "type" : "ref" to ensure all references use ResolvedReference format
- long refTypeCount = jsonWithFieldIds.lines()
- .mapToLong(line -> {
- int count = 0;
- int index = 0;
- String pattern = "\"type\" : \"ref\"";
- while ((index = line.indexOf(pattern, index)) != -1) {
- count++;
- index += pattern.length();
- }
- return count;
- })
- .sum();
-
+ long refTypeCount =
+ jsonWithFieldIds
+ .lines()
+ .mapToLong(
+ line -> {
+ int count = 0;
+ int index = 0;
+ String pattern = "\"type\" : \"ref\"";
+ while ((index = line.indexOf(pattern, index)) != -1) {
+ count++;
+ index += pattern.length();
+ }
+ return count;
+ })
+ .sum();
+
assertThat(refTypeCount).isEqualTo(4); // Should have 4 ResolvedReference objects
}
@Test
public void testBoundExpressionRoundTripWithResolvedReference() {
// Test that bound expressions with ResolvedReference can be serialized and maintain information
-
+
// Create expressions using both ResolvedTransform and ResolvedReference
- Expression originalExpr = Expressions.and(
- Expressions.equal(Expressions.bucket(Expressions.ref("id", 100), 8), 3),
- Expressions.equal(Expressions.ref("data", 101), "test"));
-
+ Expression originalExpr =
+ Expressions.and(
+ Expressions.equal(Expressions.bucket(Expressions.ref("id", 100), 8), 3),
+ Expressions.equal(Expressions.ref("data", 101), "test"));
+
// Bind the expression
Expression bound = Binder.bind(STRUCT_TYPE, originalExpr, true);
-
+
// Serialize with field IDs
String jsonWithFieldIds = ExpressionParser.toJson(bound, true, true);
-
+
// Verify the JSON structure contains complete ResolvedReference information
assertThat(jsonWithFieldIds).contains("\"type\" : \"ref\"");
assertThat(jsonWithFieldIds).contains("\"fieldId\" : 100");
assertThat(jsonWithFieldIds).contains("\"fieldId\" : 101");
-
+
// Note: The current parser doesn't support parsing ResolvedReference format back to expressions
- // This test documents the serialization capability for external systems that need field ID information
-
+ // This test documents the serialization capability for external systems that need field ID
+ // information
+
// Verify that the bound expression maintains the same field IDs after serialization
BoundPredicate<?> boundPred = (BoundPredicate<?>) ((And) bound).left();
BoundTransform<?, ?> boundTransform = (BoundTransform<?, ?>) boundPred.term();
assertThat(boundTransform.ref().fieldId()).isEqualTo(100);
-
+
BoundPredicate<?> boundDataPred = (BoundPredicate<?>) ((And) bound).right();
BoundReference<?> boundDataRef = (BoundReference<?>) boundDataPred.term();
assertThat(boundDataRef.fieldId()).isEqualTo(101);
@@ -829,35 +840,36 @@ public class TestExpressionParserWithResolvedReference {
@Test
public void testBoundExpressionFieldIdPreservationAcrossAllTypes() {
// Test that all data types preserve field IDs correctly in bound expression serialization
-
+
// Create expressions for fields that support meaningful comparisons
- Expression[] expressions = new Expression[] {
- Expressions.equal(Expressions.ref("id", 100), 42L),
- Expressions.equal(Expressions.ref("data", 101), "test"),
- Expressions.equal(Expressions.ref("b", 102), true),
- Expressions.equal(Expressions.ref("i", 103), 42),
- Expressions.equal(Expressions.ref("l", 104), 42L),
- Expressions.equal(Expressions.ref("f", 105), 3.14f),
- Expressions.equal(Expressions.ref("d", 106), 3.14159),
- Expressions.equal(Expressions.ref("s", 110), "test-string"),
- Expressions.greaterThan(Expressions.ref("id", 100), 10L)
- };
-
+ Expression[] expressions =
+ new Expression[] {
+ Expressions.equal(Expressions.ref("id", 100), 42L),
+ Expressions.equal(Expressions.ref("data", 101), "test"),
+ Expressions.equal(Expressions.ref("b", 102), true),
+ Expressions.equal(Expressions.ref("i", 103), 42),
+ Expressions.equal(Expressions.ref("l", 104), 42L),
+ Expressions.equal(Expressions.ref("f", 105), 3.14f),
+ Expressions.equal(Expressions.ref("d", 106), 3.14159),
+ Expressions.equal(Expressions.ref("s", 110), "test-string"),
+ Expressions.greaterThan(Expressions.ref("id", 100), 10L)
+ };
+
for (Expression expr : expressions) {
// Bind each expression
Expression bound = Binder.bind(STRUCT_TYPE, expr, true);
-
+
// Serialize with field IDs
String json = ExpressionParser.toJson(bound, true, true);
-
+
// Extract expected field ID from the original ResolvedReference
UnboundPredicate<?> unboundPred = (UnboundPredicate<?>) expr;
ResolvedReference<?> resolvedRef = (ResolvedReference<?>) unboundPred.term();
int expectedFieldId = resolvedRef.fieldId();
-
+
// Verify the field ID is preserved in the JSON
assertThat(json).contains("\"fieldId\" : " + expectedFieldId);
assertThat(json).contains("\"type\" : \"ref\"");
}
}
-}
\ No newline at end of file
+}
(iceberg) 04/09: Address review comment
Posted by si...@apache.org.
This is an automated email from the ASF dual-hosted git repository.
singhpk234 pushed a commit to branch feature/serialize-bound-expression
in repository https://gitbox.apache.org/repos/asf/iceberg.git
commit bb325ebd2bae510a264191e0b6968e8bb771111d
Author: Prashant Singh <pr...@snowflake.com>
AuthorDate: Tue Oct 14 11:46:51 2025 -0700
Address review comment
---
.../java/org/apache/iceberg/expressions/ResolvedReference.java | 9 +++++++--
.../java/org/apache/iceberg/expressions/ResolvedTransform.java | 4 +++-
2 files changed, 10 insertions(+), 3 deletions(-)
diff --git a/api/src/main/java/org/apache/iceberg/expressions/ResolvedReference.java b/api/src/main/java/org/apache/iceberg/expressions/ResolvedReference.java
index ab272d352b..ba91163b85 100644
--- a/api/src/main/java/org/apache/iceberg/expressions/ResolvedReference.java
+++ b/api/src/main/java/org/apache/iceberg/expressions/ResolvedReference.java
@@ -44,10 +44,13 @@ public class ResolvedReference<T> implements UnboundTerm<T>, Reference<T> {
public BoundTerm<T> bind(Types.StructType struct, boolean caseSensitive) {
// assumption is that we always have the field id
Schema schema = struct.asSchema();
- // we ignore case sensitivity here because field ids are unique
+ // Ignore caseSensitive because the field is referenced by id
Types.NestedField field = schema.findField(fieldId);
ValidationException.check(
- field != null, "Cannot find field '%s' in struct: %s", name, schema.asStruct());
+ field != null,
+ "Cannot find field with id '%s' in struct: %s, since we are resolving based on ID",
+ fieldId,
+ schema.asStruct());
return new BoundReference<>(field, schema.accessorForField(field.fieldId()), name);
}
@@ -62,9 +65,11 @@ public class ResolvedReference<T> implements UnboundTerm<T>, Reference<T> {
if (this == o) {
return true;
}
+
if (o == null || getClass() != o.getClass()) {
return false;
}
+
ResolvedReference<?> that = (ResolvedReference<?>) o;
return fieldId == that.fieldId && name.equals(that.name);
}
diff --git a/api/src/main/java/org/apache/iceberg/expressions/ResolvedTransform.java b/api/src/main/java/org/apache/iceberg/expressions/ResolvedTransform.java
index 07065b2cd7..b24cd3ece3 100644
--- a/api/src/main/java/org/apache/iceberg/expressions/ResolvedTransform.java
+++ b/api/src/main/java/org/apache/iceberg/expressions/ResolvedTransform.java
@@ -22,7 +22,7 @@ import org.apache.iceberg.exceptions.ValidationException;
import org.apache.iceberg.transforms.Transform;
import org.apache.iceberg.types.Types;
-public class ResolvedTransform<S, T> implements UnboundTerm<T>, Term {
+public class ResolvedTransform<S, T> implements UnboundTerm<T> {
private final ResolvedReference<S> ref;
private final Transform<S, T> transform;
@@ -74,9 +74,11 @@ public class ResolvedTransform<S, T> implements UnboundTerm<T>, Term {
if (this == o) {
return true;
}
+
if (o == null || getClass() != o.getClass()) {
return false;
}
+
ResolvedTransform<?, ?> that = (ResolvedTransform<?, ?>) o;
return ref.equals(that.ref) && transform.equals(that.transform);
}
(iceberg) 05/09: adjust test
Posted by si...@apache.org.
This is an automated email from the ASF dual-hosted git repository.
singhpk234 pushed a commit to branch feature/serialize-bound-expression
in repository https://gitbox.apache.org/repos/asf/iceberg.git
commit 870d601ecc51e752b799a9cee0e4e80487e79c43
Author: Prashant Singh <pr...@snowflake.com>
AuthorDate: Tue Oct 14 12:04:03 2025 -0700
adjust test
---
.../main/java/org/apache/iceberg/expressions/ResolvedTransform.java | 5 +++--
.../java/org/apache/iceberg/expressions/TestResolvedReference.java | 3 ++-
2 files changed, 5 insertions(+), 3 deletions(-)
diff --git a/api/src/main/java/org/apache/iceberg/expressions/ResolvedTransform.java b/api/src/main/java/org/apache/iceberg/expressions/ResolvedTransform.java
index b24cd3ece3..58d812833c 100644
--- a/api/src/main/java/org/apache/iceberg/expressions/ResolvedTransform.java
+++ b/api/src/main/java/org/apache/iceberg/expressions/ResolvedTransform.java
@@ -51,10 +51,11 @@ public class ResolvedTransform<S, T> implements UnboundTerm<T> {
try {
ValidationException.check(
transform.canTransform(boundRef.type()),
- "Cannot bind: %s cannot transform %s values from '%s'",
+ "Cannot bind: %s cannot transform %s values from '%s' with field id %s",
transform,
boundRef.type(),
- ref.name());
+ ref.name(),
+ ref.fieldId());
} catch (IllegalArgumentException e) {
throw new ValidationException(
"Cannot bind: %s cannot transform %s values from '%s'",
diff --git a/api/src/test/java/org/apache/iceberg/expressions/TestResolvedReference.java b/api/src/test/java/org/apache/iceberg/expressions/TestResolvedReference.java
index 9b03a4e8ea..9a9d84db1f 100644
--- a/api/src/test/java/org/apache/iceberg/expressions/TestResolvedReference.java
+++ b/api/src/test/java/org/apache/iceberg/expressions/TestResolvedReference.java
@@ -82,7 +82,8 @@ public class TestResolvedReference {
assertThatThrownBy(() -> ref.bind(SCHEMA.asStruct(), true))
.isInstanceOf(ValidationException.class)
- .hasMessageContaining("Cannot find field 'invalid' in struct");
+ .hasMessageContaining(
+ "Cannot find field with id '999' in struct: struct<34: a: optional int, 35: s: required string>, since we are resolving based on ID");
}
@Test