wombatu-kun opened a new pull request, #19241:
URL: https://github.com/apache/hudi/pull/19241

   ### Describe the issue this Pull Request addresses
   
   Closes #19240
   
   `BindVisitor#visitPredicate` interpolates `this`, the visitor, rather than 
the offending predicate into the `IllegalArgumentException` it throws for an 
unsupported predicate, and it omits the space before `cannot`. `BindVisitor` 
has no `toString()` override, so the message reads:
   
   ```
   The expression org.apache.hudi.common.expression.BindVisitor@41a90fa8cannot 
be visited as predicate
   ```
   
   The sibling `PartialBindVisitor#visitPredicate` already interpolates 
`predicate` correctly, which is what makes this a typo rather than a deliberate 
choice.
   
   ### Summary and Changelog
   
   Fixed the message so it names the expression that could not be bound, 
matching `PartialBindVisitor`. Fixes #19240.
   
   - `BindVisitor#visitPredicate` now interpolates `predicate` instead of 
`this`, and restores the missing space before `cannot`.
   - Added a null-safe `toString()` to `Predicates.StringStartsWithAny`. It is 
the only `Predicate` that can reach the guard, because every other 
implementation is either handled earlier in `visitPredicate` or overrides 
`accept` and never enters it, and it is the only class in `Predicates.java` 
without a `toString()`. Without it the corrected message would still print an 
identity hash. It is deliberately null safe because `HoodieBackedTableMetadata` 
constructs it with a `null` left operand for metadata table key filters.
   - Added `TestBindVisitor` with a regression test that fails on the current 
`master` on each of its three assertions and passes with this change.
   - Added two `TestPredicates` cases for the new `toString()`, one of them 
covering the `null` left operand shape that production actually constructs.
   
   No code was copied from elsewhere.
   
   Note: the regression test naturally belongs in a new `TestBindVisitor`, 
which the open PR #19204 also creates, and both PRs append to `TestPredicates`. 
Whichever lands second needs a trivial merge that keeps both sets of tests.
   
   ### Impact
   
   None on behavior. The guard is a defensive branch, and as far as I can tell 
no current call site can reach it: `BindVisitor` is only constructed for 
partition pruning in `HoodieBackedTableMetadata` and 
`FileSystemBackedTableMetadata`, while `StringStartsWithAny` is only 
constructed as a metadata table key filter. I did not trace every engine-side 
filter translation path, so treat that as an inference rather than a proven 
fact. The only user-visible difference is the text of an exception that nothing 
currently throws, plus a readable `toString()` on `StringStartsWithAny`.
   
   No public API, config, or performance impact.
   
   ### Risk Level
   
   none
   
   ### Documentation Update
   
   none
   
   ### Contributor's checklist
   
   - [x] Read through [contributor's 
guide](https://hudi.apache.org/contribute/how-to-contribute)
   - [x] Enough context is provided in the sections above
   - [x] Adequate tests were added if applicable
   


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]

Reply via email to