This is an automated email from the ASF dual-hosted git repository.
voonhous pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/hudi.git
The following commit(s) were added to refs/heads/master by this push:
new bc874774af17 fix(common): name the offending expression in
BindVisitor's unsupported-predicate error (#19241)
bc874774af17 is described below
commit bc874774af1759c16d7ff42781c842485352cd03
Author: Vova Kolmakov <[email protected]>
AuthorDate: Fri Jul 10 18:05:25 2026 +0700
fix(common): name the offending expression in BindVisitor's
unsupported-predicate error (#19241)
* fix(common): name the offending expression in BindVisitor's
unsupported-predicate error
BindVisitor#visitPredicate interpolated `this`, the visitor, rather than
the predicate into its IllegalArgumentException, and omitted the space before
"cannot". Since BindVisitor has no toString() override the message read "The
expression org.apache.hudi.common.expression.BindVisitor@41a90fa8cannot be
visited as predicate". The sibling PartialBindVisitor#visitPredicate already
interpolates the predicate correctly.
StringStartsWithAny is the only Predicate that can reach that guard, and
the only class in Predicates.java without a toString() override, so the
corrected message would still have printed an identity hash. Added a null-safe
toString() to it, since HoodieBackedTableMetadata constructs it with a null
left operand.
Added TestBindVisitor covering the error message, and two TestPredicates
cases covering the new toString(), including the null left operand shape.
Closes #19240
* addressed review comments: fix toString rendering in Predicates
Drop the prefix and suffix from Collectors.joining in
StringStartsWithAny.toString() so the rendering matches the method-call shape
of the sibling StringStartsWith.toString(), and scope its null-safety comment
to the left operand, which is the only one HoodieBackedTableMetadata passes as
null. Correct the startWith typo in StringStartsWith.toString() and pin the
rendering with a test.
---------
Co-authored-by: Vova Kolmakov <[email protected]>
---
.../apache/hudi/common/expression/BindVisitor.java | 2 +-
.../apache/hudi/common/expression/Predicates.java | 10 +++-
.../hudi/common/expression/TestBindVisitor.java | 53 ++++++++++++++++++++++
.../hudi/common/expression/TestPredicates.java | 21 +++++++++
4 files changed, 84 insertions(+), 2 deletions(-)
diff --git
a/hudi-common/src/main/java/org/apache/hudi/common/expression/BindVisitor.java
b/hudi-common/src/main/java/org/apache/hudi/common/expression/BindVisitor.java
index 7ea2755922dc..2ee412dd4a77 100644
---
a/hudi-common/src/main/java/org/apache/hudi/common/expression/BindVisitor.java
+++
b/hudi-common/src/main/java/org/apache/hudi/common/expression/BindVisitor.java
@@ -182,6 +182,6 @@ public class BindVisitor implements
ExpressionVisitor<Expression> {
return Predicates.contains(left, right);
}
- throw new IllegalArgumentException("The expression " + this + "cannot be
visited as predicate");
+ throw new IllegalArgumentException("The expression " + predicate + "
cannot be visited as predicate");
}
}
diff --git
a/hudi-common/src/main/java/org/apache/hudi/common/expression/Predicates.java
b/hudi-common/src/main/java/org/apache/hudi/common/expression/Predicates.java
index 35ac60af973f..dd38a83e14d2 100644
---
a/hudi-common/src/main/java/org/apache/hudi/common/expression/Predicates.java
+++
b/hudi-common/src/main/java/org/apache/hudi/common/expression/Predicates.java
@@ -263,7 +263,7 @@ public class Predicates {
@Override
public String toString() {
- return getLeft().toString() + ".startWith(" + getRight().toString() +
")";
+ return getLeft().toString() + ".startsWith(" + getRight().toString() +
")";
}
@Override
@@ -486,6 +486,14 @@ public class Predicates {
return false;
}
+ @Override
+ public String toString() {
+ // Only the left expression is absent: HoodieBackedTableMetadata passes
null for the metadata table
+ // key filters, so it must not be dereferenced. The right operands are
always non-null literals.
+ return left + ".startsWithAny("
+ +
right.stream().map(Expression::toString).collect(Collectors.joining(",")) + ")";
+ }
+
public List<Expression> getRightChildren() {
return right;
}
diff --git
a/hudi-common/src/test/java/org/apache/hudi/common/expression/TestBindVisitor.java
b/hudi-common/src/test/java/org/apache/hudi/common/expression/TestBindVisitor.java
new file mode 100644
index 000000000000..ca29eada7b8e
--- /dev/null
+++
b/hudi-common/src/test/java/org/apache/hudi/common/expression/TestBindVisitor.java
@@ -0,0 +1,53 @@
+/*
+ * 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.hudi.common.expression;
+
+import org.apache.hudi.common.schema.internal.Types;
+
+import org.junit.jupiter.api.Test;
+
+import java.util.ArrayList;
+import java.util.Collections;
+
+import static org.junit.jupiter.api.Assertions.assertFalse;
+import static org.junit.jupiter.api.Assertions.assertThrows;
+import static org.junit.jupiter.api.Assertions.assertTrue;
+
+class TestBindVisitor {
+
+ private static Types.RecordType schema() {
+ ArrayList<Types.Field> fields = new ArrayList<>(1);
+ fields.add(Types.Field.get(0, true, "a", Types.StringType.get()));
+ return Types.RecordType.get(fields, "schema");
+ }
+
+ @Test
+ void testUnsupportedPredicateErrorNamesTheExpression() {
+ BindVisitor bindVisitor = new BindVisitor(schema(), true);
+ Predicates.StringStartsWithAny startsWithAny =
+ Predicates.startsWithAny(new NameReference("a"),
Collections.singletonList(Literal.from("Ja")));
+
+ IllegalArgumentException e =
+ assertThrows(IllegalArgumentException.class, () ->
startsWithAny.accept(bindVisitor));
+
+
assertTrue(e.getMessage().contains("NameReference(name=a).startsWithAny(Ja)"),
e::getMessage);
+ assertFalse(e.getMessage().contains(BindVisitor.class.getName()),
e::getMessage);
+ assertTrue(e.getMessage().contains(" cannot be visited as predicate"),
e::getMessage);
+ }
+}
diff --git
a/hudi-common/src/test/java/org/apache/hudi/common/expression/TestPredicates.java
b/hudi-common/src/test/java/org/apache/hudi/common/expression/TestPredicates.java
index 34eb80e792be..3c5b9f1c24c4 100644
---
a/hudi-common/src/test/java/org/apache/hudi/common/expression/TestPredicates.java
+++
b/hudi-common/src/test/java/org/apache/hudi/common/expression/TestPredicates.java
@@ -22,6 +22,7 @@ package org.apache.hudi.common.expression;
import org.junit.jupiter.api.Test;
import java.util.Arrays;
+import java.util.Collections;
import java.util.List;
import static org.junit.jupiter.api.Assertions.assertEquals;
@@ -29,6 +30,12 @@ import static org.junit.jupiter.api.Assertions.assertFalse;
import static org.junit.jupiter.api.Assertions.assertTrue;
class TestPredicates {
+ @Test
+ void testStringStartsWithToString() {
+ Predicates.StringStartsWith predicate =
Predicates.startsWith(Literal.from("key"), Literal.from("k1"));
+ assertEquals("key.startsWith(k1)", predicate.toString());
+ }
+
@Test
void testStringStartsWithAnyWhenMatched() {
Expression left = Literal.from("key2_any");
@@ -52,4 +59,18 @@ class TestPredicates {
assertEquals(Expression.Operator.STARTS_WITH, predicate.getOperator());
assertFalse((boolean) predicate.eval(null));
}
+
+ @Test
+ void testStringStartsWithAnyToString() {
+ Predicates.StringStartsWithAny predicate =
+ Predicates.startsWithAny(Literal.from("key"),
Arrays.asList(Literal.from("k1"), Literal.from("k2")));
+ assertEquals("key.startsWithAny(k1,k2)", predicate.toString());
+ }
+
+ @Test
+ void testStringStartsWithAnyToStringIsNullSafeForAbsentLeft() {
+ Predicates.StringStartsWithAny predicate =
+ Predicates.startsWithAny(null,
Collections.singletonList(Literal.from("key1")));
+ assertEquals("null.startsWithAny(key1)", predicate.toString());
+ }
}
