cloud-fan commented on code in PR #55523:
URL: https://github.com/apache/spark/pull/55523#discussion_r3152513496
##########
sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/analysis/resolver/Resolver.scala:
##########
@@ -585,7 +585,9 @@ class Resolver(
val multipartId = unresolvedRelation.multipartIdentifier
val catalogPath = (catalogManager.currentCatalog.name() +:
catalogManager.currentNamespace).toSeq
- val searchPath =
SQLConf.get.resolutionSearchPath(catalogPath).map(toSQLId)
+ val searchPath = catalogManager
Review Comment:
Late catch — this was already in `4f300f7` and I missed it last round. The
new `searchPath = catalogManager.sqlResolutionPathEntries(catalogPath.head,
...)` is built from a `catalogPath` (lines 586-587) that's constructed from
session-live `catalogManager.currentCatalog.name() / currentNamespace`. The
dual fix landed in `CheckAnalysis.catalogPathForError` (lines 97-101) and was
wired into the iterative path in `Analyzer.LookupFunctions` (line 2103) — both
consult `AnalysisContext.catalogAndNamespace`. Single-pass `Resolver` doesn't
extend `CheckAnalysis`, so the consolidated helper isn't reusable here, and the
inline construction was missed.
User-visible effect: inside a view body, single-pass
`TABLE_OR_VIEW_NOT_FOUND` from this site reports the session's current
catalog/namespace instead of the view's defining one — divergent from the
iterative path. Mirroring the `catalogPathForError` logic (`if
AnalysisContext.get.catalogAndNamespace.nonEmpty use that, else session live`)
— perhaps via a shared helper accessible to both `CheckAnalysis` and `Resolver`
— would close the gap. Fine to land as-is and address in a small follow-up if
you prefer.
##########
sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/analysis/Analyzer.scala:
##########
@@ -2078,9 +2100,9 @@ class Analyzer(
throw
QueryCompilationErrors.notAScalarFunctionError(nameParts.mkString("."), f)
case FunctionType.NotFound =>
- val catalogPath =
- catalogManager.currentCatalog.name +:
catalogManager.currentNamespace
- val searchPath =
SQLConf.get.resolutionSearchPath(catalogPath.toSeq)
+ val catalogPath = catalogPathForError
+ val searchPath = catalogManager
Review Comment:
Late catch, low priority / future-proofing. The error path here calls
`catalogManager.sqlResolutionPathEntries(catalogPath.head,
catalogPath.tail.toSeq)` — the 2-arg overload that does **not** consult
`AnalysisContext.resolutionPathEntries`. The matching resolution path
(`functionResolution.lookupFunctionType` → `resolutionCandidates` →
`sqlResolutionPathEntriesForAnalysis`) **does** consult it.
No observable bug today since `resolutionPathEntries` is `None` throughout
this PR. But once the frozen-path follow-up wires it, the error-message search
path will diverge from the actual lookup path used by `lookupFunctionType`.
Could be pre-empted by exposing
`FunctionResolution.sqlResolutionPathEntriesForAnalysis` (or moving the
search-path formatting into `FunctionResolution`) and calling that here. Worth
handling either now or in the follow-up that wires the frozen path.
--
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]
