voonhous commented on code in PR #19853:
URL: https://github.com/apache/hudi/pull/19853#discussion_r3968390592
##########
hudi-spark-datasource/hudi-spark/src/main/scala/org/apache/spark/sql/hudi/command/procedures/HoodieProcedureFilterUtils.scala:
##########
@@ -411,8 +500,17 @@ object HoodieProcedureFilterUtils {
// Spark raises SparkArithmeticException for an overflowing ANSI cast or
arithmetic, and
// SparkNumberFormatException or SparkDateTimeException for an ANSI cast
of a malformed
// string; each extends the matching JDK type. Swallowing one would
silently drop a row the
- // same query keeps, so let it out and let the caller fail the way the
equivalent query does.
+ // same query keeps, so let it out unconditionally, exactly as before
the registry fallback.
case Failure(e @ (_: ArithmeticException | _: NumberFormatException | _:
DateTimeException)) => throw e
+ // SparkThrowable covers the equivalent runtime errors from
registry-resolved functions
+ // (to_number/bit_get out-of-range, ...), and IllegalArgumentException
covers a bad regex
+ // pattern - both newly reachable through the registry fallback, so this
guard only applies
+ // to these two: a caller that skips validateFilterExpression and
evaluates a genuinely
+ // unsupported function directly still no-matches instead of hitting the
same
+ // SparkThrowable-family INTERNAL_ERROR that Unevaluable.eval() raises
for an unrelated
+ // reason, so this method stays safe to call on its own.
+ case Failure(e @ (_: SparkThrowable | _: IllegalArgumentException)) if
!boundExpr.exists(_.isInstanceOf[Unevaluable]) =>
Review Comment:
Confirmed intended, and the head now says so: the comment at :513-521 covers
the hardcoded-table functions explicitly, and the new test at :706-714 pins
`regexp_like(name, '[')` and `regexp_extract(name, '[', 1)` raising. Both raise
`IllegalArgumentException` through `evaluateFilter` on 3.5.5, where the
equivalent Spark query also fails.
One asymmetry the foldable probe at :487 introduces on the same head: with
all-literal arguments a registry-path call is foldable, so the probe evals it
during resolution, catches the throw, and the filter is rejected rather than
raised. On 3.5.5, `to_number('abc', '999') > 0`, `regexp_replace('a', '[', 'x')
= 'x'` and `bit_get(1, 99) = 0` raised `IllegalArgumentException` at
97c30d35dfd7 and now come back as `Unsupported functions: to_number` and
friends. The column-argument spellings still raise, and the hardcoded path
never reaches the probe, so `regexp_like('clean', '[')` and
`regexp_extract('clean', '[', 1)` still raise too. Fail-closed either way, so
not blocking - but is rejection the intended answer for a literal-only call,
given the message names the function rather than the parse error?
##########
hudi-spark-datasource/hudi-spark/src/main/scala/org/apache/spark/sql/hudi/command/procedures/HoodieProcedureFilterUtils.scala:
##########
@@ -92,10 +99,13 @@ object HoodieProcedureFilterUtils {
}
}
- // Second pass: resolve functions
- val functionResolved = attributeBound.transform {
+ // Second pass: resolve functions. transformUp so a nested call's
arguments (e.g. upper(name)
+ // inside instr(upper(name), 'A')) are already resolved by the time the
outer function's case
+ // runs - otherwise resolved/checkInputDataTypes below would see an
unresolved child and
+ // reject a call that's actually fine.
+ val functionResolved = attributeBound.transformUp {
case unresolvedFunc:
org.apache.spark.sql.catalyst.analysis.UnresolvedFunction =>
- unresolvedFunc.nameParts.head.toLowerCase(Locale.ROOT) match {
+ val hardcodedResolved =
unresolvedFunc.nameParts.head.toLowerCase(Locale.ROOT) match {
Review Comment:
Addressed at 467472bdeed7 - the inner match is `resolveOrFallback` (:379)
now, so the `transformUp` case ends in a single call.
--
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]
