hudi-agent commented on code in PR #19853:
URL: https://github.com/apache/hudi/pull/19853#discussion_r3959222548
##########
hudi-spark-datasource/hudi-spark/src/main/scala/org/apache/spark/sql/hudi/command/procedures/HoodieProcedureFilterUtils.scala:
##########
@@ -389,6 +398,49 @@ object HoodieProcedureFilterUtils {
}
}
+ // didn't match anything above, so ask Spark itself before we give up -
saves us from having
Review Comment:
🤖 nit: the original two-line "didn't match anything above, so ask Spark
itself..." comment is still here above the new block, so the rationale is
stated twice — probably meant to be replaced?
<sub><i>⚠️ AI-generated; verify before applying. React 👍/👎 to flag
quality.</i></sub>
##########
hudi-spark-datasource/hudi-spark/src/main/scala/org/apache/spark/sql/hudi/command/procedures/HoodieProcedureFilterUtils.scala:
##########
@@ -389,6 +398,49 @@ object HoodieProcedureFilterUtils {
}
}
+ // didn't match anything above, so ask Spark itself before we give up -
saves us from having
+ // to hand-list every builtin (concat, instr, if, ...) one by one
+ // Resolves a function not covered by the hardcoded table above via Spark's
own FunctionRegistry.
+ // A resolved result is only usable if it can actually be eval()'d one row
at a time, which
+ // several categories of otherwise-valid expressions cannot:
RuntimeReplaceable placeholders
+ // (nvl, ifnull, left, right) need substitution the analyzer normally
performs but skips here;
+ // aggregates (percentile, collect_list) only make sense across real
aggregation; generators
+ // (explode, inline) only work inside a projection; non-deterministic
functions (rand, uuid,
+ // spark_partition_id) expect per-partition initialization; and a type
mismatch the analyzer's
+ // implicit-cast pass would normally have caught (e.g. concat on a
non-string column) still
+ // fails checkInputDataTypes. Anything in one of those categories is treated
as still-unresolved
+ // so it falls through to the existing rejection path instead of silently
dropping every row.
+ private def resolveViaFunctionRegistry(unresolvedFunc: UnresolvedFunction,
sparkSession: SparkSession): Expression = {
+ Try {
+ // Filter expressions only ever call plain or db-qualified builtins. A
3+ part name
+ // (catalog.db.func) isn't safe to look up: FunctionIdentifier only
carries one qualifier,
+ // and guessing by dropping the extra parts risks matching an unrelated
same-named function.
+ val functionIdentifier = unresolvedFunc.nameParts match {
+ case Seq(funcName) => Some(FunctionIdentifier(funcName))
+ case Seq(db, funcName) => Some(FunctionIdentifier(funcName, Some(db)))
+ case _ => None
+ }
+ val resolved = functionIdentifier
+ .map(sparkSession.sessionState.functionRegistry.lookupFunction(_,
unresolvedFunc.arguments))
+ .getOrElse(unresolvedFunc)
+ val unwrapped = resolved.transformUp { case r: RuntimeReplaceable =>
r.replacement }
+ val stillUnsupported =
+
unwrapped.isInstanceOf[org.apache.spark.sql.catalyst.expressions.aggregate.AggregateFunction]
||
+
unwrapped.isInstanceOf[org.apache.spark.sql.catalyst.expressions.Generator] ||
+ !unwrapped.deterministic ||
+ !unwrapped.resolved ||
Review Comment:
🤖 Since the second pass uses `transform` (top-down), a registry function
whose argument is itself a function is visited while that argument is still an
`UnresolvedFunction`, so `unwrapped.resolved` is false and the whole call gets
pushed back to the rejection path. Does `instr(upper(name), 'A') = 1` (or
`concat(upper(name), 'x')`) still validate on this branch? Resolving functions
bottom-up (`transformUp`) — or checking `resolved`/`checkInputDataTypes` only
after the full tree is resolved — would avoid it.
<sub><i>⚠️ AI-generated; verify before applying. React 👍/👎 to flag
quality.</i></sub>
--
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]
