This is an automated email from the ASF dual-hosted git repository. voonhous pushed a commit to branch release-1.2.1 in repository https://gitbox.apache.org/repos/asf/hudi.git
commit 51a52547568ae8d9c7b90fbcd485ff0b399e385e 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]> (cherry picked from commit bc874774af1759c16d7ff42781c842485352cd03) --- .../org/apache/hudi/expression/BindVisitor.java | 2 +- .../org/apache/hudi/expression/Predicates.java | 10 +++- .../apache/hudi/expression/TestBindVisitor.java | 53 ++++++++++++++++++++++ .../org/apache/hudi/expression/TestPredicates.java | 21 +++++++++ 4 files changed, 84 insertions(+), 2 deletions(-) diff --git a/hudi-common/src/main/java/org/apache/hudi/expression/BindVisitor.java b/hudi-common/src/main/java/org/apache/hudi/expression/BindVisitor.java index 2b7e589af213..5d4ea2fd5235 100644 --- a/hudi-common/src/main/java/org/apache/hudi/expression/BindVisitor.java +++ b/hudi-common/src/main/java/org/apache/hudi/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/expression/Predicates.java b/hudi-common/src/main/java/org/apache/hudi/expression/Predicates.java index e5dd07e2bfe6..e03522a910a3 100644 --- a/hudi-common/src/main/java/org/apache/hudi/expression/Predicates.java +++ b/hudi-common/src/main/java/org/apache/hudi/expression/Predicates.java @@ -233,7 +233,7 @@ public class Predicates { @Override public String toString() { - return getLeft().toString() + ".startWith(" + getRight().toString() + ")"; + return getLeft().toString() + ".startsWith(" + getRight().toString() + ")"; } @Override @@ -449,6 +449,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/expression/TestBindVisitor.java b/hudi-common/src/test/java/org/apache/hudi/expression/TestBindVisitor.java new file mode 100644 index 000000000000..6ef464422492 --- /dev/null +++ b/hudi-common/src/test/java/org/apache/hudi/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.expression; + +import org.apache.hudi.internal.schema.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/expression/TestPredicates.java b/hudi-common/src/test/java/org/apache/hudi/expression/TestPredicates.java index e2a83872cdd4..85481fdfd714 100644 --- a/hudi-common/src/test/java/org/apache/hudi/expression/TestPredicates.java +++ b/hudi-common/src/test/java/org/apache/hudi/expression/TestPredicates.java @@ -22,6 +22,7 @@ package org.apache.hudi.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()); + } }
