cloud-fan commented on code in PR #58450:
URL: https://github.com/apache/spark/pull/58450#discussion_r3958937286
##########
sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/analysis/FunctionResolution.scala:
##########
@@ -404,6 +388,95 @@ class FunctionResolution(
}
}
+ /**
+ * Returns whether an unqualified function name reaches `system.builtin`
before any temp or
+ * persistent function in the effective SQL PATH. When a temp or persistent
function shadows the
+ * builtin, special-syntax handling that only applies to Spark's builtins
must not fire, since the
+ * name no longer refers to the builtin -- e.g. rejecting a bare `*` in a
routed SQL/JSON function
+ * or the `count(tbl.*)` guard. Parser-built `count(*)` is normalized to
`count(1)` in
+ * `AstBuilder` so it skips this probe, but a DataFrame `count("*")` keeps
its star and does reach
+ * the probe during analyzer normalization.
+ */
+ def unqualifiedFunctionResolvesToBuiltinBeforeAnyShadow(functionName:
String): Boolean = {
+ // Walk the PATH in order and stop at the first entry that owns the name.
The default order puts
+ // system.builtin first, so the common case returns on the first entry
with no catalog lookup;
+ // only a custom PATH that lists a persistent catalog ahead of
system.builtin reaches the probe
+ // below (one lookup per such preceding entry, recomputed on each call --
not cached).
+ sqlResolutionPathEntriesForAnalysis.foreach { pathEntry =>
+ val candidate = pathEntry :+ functionName
+ FunctionResolution.sessionNamespaceKind(candidate) match {
+ case
Some(org.apache.spark.sql.catalyst.catalog.SessionCatalog.Builtin) =>
+ return true
+ case Some(org.apache.spark.sql.catalyst.catalog.SessionCatalog.Temp) =>
+ // Honor stored-view temp visibility so this probe picks the same
owner the resolver
+ // would: a temp not captured by the view is hidden here too, just
as the persistent
+ // branch below expands through the view's frozen catalog.
+ if
(v1SessionCatalog.isTemporaryScalarFunctionVisible(FunctionIdentifier(functionName)))
{
Review Comment:
**Non-blocking (P2):** This probe checks only temporary scalar functions,
but scalar resolution treats a table-only match at the same PATH entry as
terminal and raises `NOT_A_SCALAR_FUNCTION`. With a temporary table function
named `json_array` before `system.builtin`, `json_array(*)` is misclassified as
the builtin and raises `INVALID_USAGE_OF_STAR_OR_REGEX` first. Please preserve
the resolver's cross-type first-owner outcome here and add session-first PATH
coverage for both analyzer modes.
**Recommended change:** Restore cross-type temporary-routine ownership in
the builtin preprocessing probe and verify it end to end.
**Why this works:** Use one side-effect-free temporary-owner visibility
decision that applies the stored-view filter and checks both scalar and table
registries, matching resolveFunctionCandidate's scalar-miss/table-terminal
sequence before the probe can continue to system.builtin.
**Scope:** FunctionResolution's system.session owner probe, the
SessionCatalog visibility helper and its focused tests, plus fixed-point and
single-pass PATH regressions for JSON star handling.
**Compatibility:** Visible temporary scalar routines keep shadowing
builtins, table-function lookup semantics stay unchanged, stored-view capture
filtering is preserved, and table-only first matches regain the established
NOT_A_SCALAR_FUNCTION result.
**Risks:** The visibility check must remain side-effect-free during
stored-view analysis. A table routine outside the effective PATH or hidden by a
stored view must not suppress a later builtin.
**Constraints:** Match every terminal outcome of resolveFunctionCandidate
for a system.session candidate. Do not change table-function resolution or
expose builtin-only star rewrites to non-builtin owners. Exercise both
fixed-point and single-pass analyzer paths with system.session before
system.builtin.
**Success:** With a same-named temporary table routine first on PATH,
json_array(*) reaches ordinary resolution and reports NOT_A_SCALAR_FUNCTION in
both analyzer modes; builtin-first, scalar-shadow, and stored-view cases retain
their current results.
--
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]
