cloud-fan commented on code in PR #56239:
URL: https://github.com/apache/spark/pull/56239#discussion_r3334752138
##########
sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/catalog/SessionCatalog.scala:
##########
@@ -2040,13 +2046,20 @@ class SessionCatalog(
}
}
+ val funcInputMetadata = new MetadataBuilder()
+ .putBoolean(SessionCatalog.SQL_FUNCTION_PARAMETER_ALIAS_METADATA_KEY,
true)
+ .build()
val inputCast = paddedInput.zip(param.fields).map {
case (expr, param) =>
// Add outer references to all attributes in the function input.
val outer = expr.transform {
case a: Attribute => OuterReference(a)
}
- Alias(Cast(outer, param.dataType), param.name)(qualifier = qualifier)
+ // Mark the alias as function input so name resolution can give a
parameterless
Review Comment:
The marker stamped here on `makeSQLTableFunctionPlan` parameter aliases is
never consumed. A table UDF's body is a query inside
`LateralJoin(Project(params), LateralSubquery(body))`, so a body reference to a
parameter resolves as an *outer* reference (the param lives in the
lateral-join's left child, not the body's local scope). In
`ColumnResolutionHelper`, `resolveColumnByName` returns `None` for it, so
`LiteralFunctionResolution` already wins via the pre-existing "function beats
outer reference" precedence — `isSQLFunctionParameterAlias` is never evaluated
on this path.
Consequences:
- The legacy flag does **not** restore param-shadowing for table UDFs. The
legacy golden `sql-udf-name-precedence-legacy.sql.out` shows `function_won =
true` for the `tvf_paramless_vs_param` case, identical to the non-legacy golden
— table UDFs were never buggy here.
- The PR description ("`makeSQLTableFunctionPlan` ... CALL time") and the
test comment "same rule must apply on the makeSQLTableFunctionPlan path" are
misleading: the function wins via an unrelated, pre-existing rule, not this
PR's marker logic. The same applies to the CREATE-time marker as applied to
table functions.
Suggest either dropping the `makeSQLTableFunctionPlan` marker change (keep
the TVF test as a regression guard, but retitle its comment to note table UDFs
are already correct via function-vs-outer-reference precedence), or — if the
marker is intended as defensive future-proofing — adding a comment saying it is
not consumed today and not claiming the table-function path is fixed/restorable
by the flag. Non-blocking: default behavior is correct on all tested paths.
--
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]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
