srielau commented on code in PR #55523:
URL: https://github.com/apache/spark/pull/55523#discussion_r3142367840
##########
sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/analysis/v2ResolutionPlans.scala:
##########
@@ -68,15 +68,39 @@ case class UnresolvedView(
allowTemp: Boolean,
suggestAlternative: Boolean = false) extends UnresolvedLeafNode
+/**
+ * Controls which search path is shown in `TABLE_OR_VIEW_NOT_FOUND` for
+ * [[UnresolvedTableOrView]] (see
[[org.apache.spark.sql.catalyst.analysis.CheckAnalysis]]).
+ */
+sealed trait UnresolvedTableOrViewSearchPathMode
+
+object UnresolvedTableOrViewSearchPathMode {
+ /** DDL on catalog objects: `system.session` and current catalog namespace
only. */
+ case object Ddl extends UnresolvedTableOrViewSearchPathMode
+ /**
+ * Like `SELECT` / DML: full `sqlResolutionPathEntries` order; fully
qualified
+ * `system.session.*` names still use the temp-view-only path in errors.
+ */
+ case object QueryLike extends UnresolvedTableOrViewSearchPathMode
+ /** Commands that only target temp views (e.g. some `DROP TEMPORARY VIEW`
paths). */
+ case object TempViewOnly extends UnresolvedTableOrViewSearchPathMode
+}
Review Comment:
Done. Wired `QueryLike` from `SparkSqlParser.visitDescribeRelation`. Updated
`describe.sql.out` golden file. Added `tableNotFoundSearchPathMode` parameter
to `AstBuilder.createUnresolvedTableOrView`.
##########
sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/analysis/resolver/ResolverGuard.scala:
##########
@@ -474,10 +474,10 @@ class ResolverGuard(
private def checkUnresolvedFunction(unresolvedFunction: UnresolvedFunction)
= {
val nameParts = unresolvedFunction.nameParts
val funcName = nameParts.last.toLowerCase(Locale.ROOT)
-
- if (nameParts.length == 1) {
- // Unqualified: same as master (unsupported, non-builtin, or check
children)
- if (isUnsupportedFunction(funcName)) {
+ if (nameParts.size == 1) {
+ // Unqualified: reject if unsupported, else non-builtin or check
children (same as master)
+ if (ResolverGuard.UNSUPPORTED_FUNCTION_NAMES.contains(funcName) ||
+ isUnsupportedFunction(funcName)) {
Review Comment:
Done. Simplified to single `isUnsupportedFunction(funcName)` call.
##########
sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/analysis/RelationResolution.scala:
##########
@@ -110,34 +114,64 @@ class RelationResolution(
/**
* Scope in the relation resolution search path. Used to interpret
- * [[SQLConf.resolutionSearchPath]] when resolving unqualified table/view
names.
+ * [[SQLConf.sqlResolutionPathEntries]] when resolving unqualified
table/view names.
*/
- private sealed trait RelationResolutionScope
- private case object SessionScope extends RelationResolutionScope
- private case object PersistentScope extends RelationResolutionScope
+ private sealed trait RelationResolutionStep
+ private case object SessionScopeStep extends RelationResolutionStep
+ private case class PersistentCatalogStep(catalogAndNamespace: Seq[String])
+ extends RelationResolutionStep
+
+ /**
+ * Path entries for unqualified relation resolution.
+ *
+ * Inside a view, [[AnalysisContext.resolutionPathEntries]] is usually a
snapshot from
+ * [[AnalysisContext.snapshotViewResolutionPath]]: either
[[CatalogTable.VIEW_RESOLUTION_PATH]]
+ * (frozen at creation, no virtual markers or `system.session` for persisted
views) or, if absent,
+ * [[SQLConf.sqlResolutionPathEntries]] with view default catalog/namespace
and live marker
+ * expansion. The snapshot is honored only when [[SQLConf.PATH_ENABLED]] is
true; when PATH is
+ * disabled, legacy [[SQLConf.sqlResolutionPathEntries]] rules apply.
Review Comment:
Done. Replaced `snapshotViewResolutionPath` ref with description of
follow-up PR. Fixed all `SQLConf.sqlResolutionPathEntries` to
`CatalogManager.sqlResolutionPathEntries`. Simplified Scaladoc.
##########
sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/analysis/RelationResolution.scala:
##########
@@ -110,34 +114,64 @@ class RelationResolution(
/**
* Scope in the relation resolution search path. Used to interpret
- * [[SQLConf.resolutionSearchPath]] when resolving unqualified table/view
names.
+ * [[SQLConf.sqlResolutionPathEntries]] when resolving unqualified
table/view names.
Review Comment:
Done.
##########
sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/analysis/RelationResolution.scala:
##########
@@ -195,15 +229,15 @@ class RelationResolution(
).orElse(tryResolvePersistent(u, identifier, finalTimeTravelSpec))
}
- // 1-part name: try each scope in relationResolutionSearchPath order (from
- // [[SQLConf.resolutionSearchPath]]).
- val candidates = relationResolutionSearchPath
- for (scope <- candidates) {
- val result = scope match {
- case SessionScope =>
+ // 1-part name: try each step in [[relationResolutionSteps]] order (from
+ // [[SQLConf.sqlResolutionPathEntries]]).
Review Comment:
Done.
##########
sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/analysis/Analyzer.scala:
##########
@@ -139,17 +139,22 @@ object FakeV2SessionCatalog extends TableCatalog with
FunctionCatalog with Suppo
* even if a temp view `t` has been created.
* @param outerPlan The query plan from the outer query that can be used to
resolve star
* expressions in a subquery.
+ * @param resolutionPathEntries When resolving a view body, the ordered path
for unqualified
+ * relation names (see
[[AnalysisContext.withAnalysisContext]]).
+ * [[None]] outside views: compute from session
+ * [[SQLConf.sqlResolutionPathEntries]].
Review Comment:
Done. Fixed ref and improved wording.
##########
sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/analysis/CheckAnalysis.scala:
##########
@@ -77,11 +80,13 @@ trait CheckAnalysis extends LookupCatalog with
QueryErrorsBase with PlanToString
}
/**
- * `SQLConf.resolutionSearchPath` entries formatted with [[toSQLId]] for
TABLE_OR_VIEW_NOT_FOUND.
- * Same ordering as relation resolution and routine resolution search paths.
+ * [[SQLConf.sqlResolutionPathEntries]] formatted with [[toSQLId]] for
TABLE_OR_VIEW_NOT_FOUND.
+ * Same ordering as relation and routine resolution search paths.
Review Comment:
Done. Fixed ref and rewrote as complete sentence.
##########
sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/analysis/FunctionResolution.scala:
##########
@@ -83,23 +87,41 @@ class FunctionResolution(
/**
* Produces the ordered list of candidate names for resolution. Expansion
happens in two cases:
*
- * 1. Single-part names: expanded via the search path, where each search
path entry is
- * fully qualified so appending the name produces fully qualified
candidates.
+ * 1. Single-part names: expanded via [[SQLConf.sqlResolutionPathEntries]]
(same list as
Review Comment:
Done.
##########
sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/analysis/Analyzer.scala:
##########
@@ -2078,9 +2093,19 @@ 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 ctx = AnalysisContext.get.catalogAndNamespace
+ val pathDefault =
+ if (ctx.nonEmpty) ctx
+ else {
+ (catalogManager.currentCatalog.name +:
+ catalogManager.currentNamespace).toSeq
+ }
+ val searchPath = catalogManager
+ .sqlResolutionPathEntries(
+ pathDefault.head,
+ pathDefault.tail.toSeq,
+ catalogManager.currentCatalog.name,
+ catalogManager.currentNamespace.toSeq)
Review Comment:
Done. Consolidated: `catalogPathForError` now consults
`AnalysisContext.catalogAndNamespace`, and the `UnresolvedFunctionName` handler
uses `fullSearchPathForError(catalogPathForError)` instead of the duplicated
inline logic.
##########
sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/analysis/FunctionResolution.scala:
##########
@@ -592,6 +634,60 @@ class FunctionResolution(
errorClass = errorClass,
messageParameters = messageParameters)
}
+
+ /**
+ * Resolves [[UnresolvedProcedure]] for `CALL` / `DESCRIBE PROCEDURE` using
the same multipart
+ * candidates as SQL functions and relations ([[resolutionCandidates]] /
+ * [[sqlResolutionPathEntriesForAnalysis]]). Catalogs that do not implement
+ * [[ProcedureCatalog]] are skipped for unqualified names; an explicitly
catalog-qualified name
+ * that targets a non-[[ProcedureCatalog]] still raises
+ * [[QueryCompilationErrors.missingCatalogProceduresAbilityError]].
+ */
+ def resolveProcedure(unresolved: UnresolvedProcedure): LogicalPlan = {
+ val candidates = resolutionCandidates(unresolved.nameParts)
+ for (multipart <- candidates) {
+ val expandedOpt =
+ try {
+ Some(relationResolution.expandIdentifier(multipart))
+ } catch {
+ case NonFatal(_) => None
+ }
+ expandedOpt.foreach { expanded =>
+ CatalogAndIdentifier.unapply(expanded).foreach { case (catalog, ident)
=>
+ catalog match {
+ case pc: ProcedureCatalog =>
+ try {
+ val procedure = pc.loadProcedure(ident)
+ return ResolvedProcedure(pc, ident, procedure)
+ } catch {
+ case e: AnalysisException => throw e
+ case e: SparkThrowable => throw e
+ case NonFatal(e) =>
+ if (unresolved.nameParts.length > 1) {
+ val cause = e match {
+ case ex: Exception => ex
+ case th => new RuntimeException(th)
+ }
+ throw QueryCompilationErrors.failedToLoadRoutineError(
+ catalog.name +: ident.asMultipartIdentifier,
+ cause)
+ }
Review Comment:
Done. `NonFatal` for 1-part names now wraps in `failedToLoadRoutineError`
same as multi-part, so the underlying catalog failure is preserved.
##########
sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/analysis/CheckAnalysis.scala:
##########
@@ -45,6 +43,11 @@ trait CheckAnalysis extends LookupCatalog with
QueryErrorsBase with PlanToString
protected def isView(nameParts: Seq[String]): Boolean
+ protected def conf: org.apache.spark.sql.internal.SQLConf
+
+ /** Conf for routine resolution/errors; override in Analyzer to use session
conf. */
+ protected def confForRoutineResolution: SQLConf = conf
Review Comment:
Done. Removed `confForRoutineResolution` definition from `CheckAnalysis` and
override from `Analyzer` -- no read sites.
--
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]
