Skip to content

Commit f0643c4

Browse files
committed
Followup cleanup: remove Extension namespace, fix stale comments, fix test bugs
1 parent 241a125 commit f0643c4

17 files changed

Lines changed: 173 additions & 386 deletions

File tree

sql/api/src/main/scala/org/apache/spark/sql/errors/QueryParsingErrors.scala

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -674,6 +674,20 @@ private[sql] object QueryParsingErrors extends DataTypeErrorsBase {
674674
new ParseException(errorClass = "INVALID_SQL_SYNTAX.CREATE_TEMP_FUNC_WITH_IF_NOT_EXISTS", ctx)
675675
}
676676

677+
def invalidTempObjQualifierError(
678+
objectType: String,
679+
objectName: String,
680+
qualifier: String,
681+
ctx: ParserRuleContext): Throwable = {
682+
new ParseException(
683+
"INVALID_TEMP_OBJ_QUALIFIER",
684+
Map(
685+
"objectType" -> objectType,
686+
"objectName" -> toSQLId(objectName),
687+
"qualifier" -> toSQLId(qualifier)),
688+
ctx)
689+
}
690+
677691
def unsupportedFunctionNameError(funcName: Seq[String], ctx: ParserRuleContext): Throwable = {
678692
new ParseException(
679693
errorClass = "INVALID_SQL_SYNTAX.MULTI_PART_NAME",

sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/analysis/Analyzer.scala

Lines changed: 7 additions & 42 deletions
Original file line numberDiff line numberDiff line change
@@ -47,7 +47,7 @@ import org.apache.spark.sql.catalyst.trees.AlwaysProcess
4747
import org.apache.spark.sql.catalyst.trees.CurrentOrigin.withOrigin
4848
import org.apache.spark.sql.catalyst.trees.TreePattern._
4949
import org.apache.spark.sql.catalyst.types.DataTypeUtils
50-
import org.apache.spark.sql.catalyst.util.{quoteIfNeeded, toPrettySQL, trimTempResolvedColumn, CharVarcharUtils}
50+
import org.apache.spark.sql.catalyst.util.{toPrettySQL, trimTempResolvedColumn, CharVarcharUtils}
5151
import org.apache.spark.sql.catalyst.util.ResolveDefaultColumns._
5252
import org.apache.spark.sql.connector.catalog.{View => _, _}
5353
import org.apache.spark.sql.connector.catalog.CatalogV2Implicits._
@@ -2016,9 +2016,8 @@ class Analyzer(
20162016
plan.resolveExpressionsWithPruning(_.containsAnyPattern(UNRESOLVED_FUNCTION)) {
20172017
case f @ UnresolvedFunction(nameParts, _, _, _, _, _, _) =>
20182018
// For builtin/temp functions, we can do a quick check without catalog lookup
2019-
val quickCheck = if (nameParts.size == 1) {
2020-
functionResolution.lookupBuiltinOrTempFunction(nameParts, Some(f))
2021-
} else if (FunctionResolution.sessionNamespaceKind(nameParts).isDefined) {
2019+
val quickCheck = if (nameParts.size == 1 ||
2020+
FunctionResolution.sessionNamespaceKind(nameParts).isDefined) {
20222021
functionResolution.lookupBuiltinOrTempFunction(nameParts, Some(f))
20232022
} else {
20242023
None
@@ -2029,25 +2028,8 @@ class Analyzer(
20292028
f
20302029
} else {
20312030
// Might be a persistent function - compute full name and check cache first
2032-
val (catalog, ident) = try {
2033-
val CatalogAndIdentifier(cat, id) = relationResolution.expandIdentifier(nameParts)
2034-
(cat, id)
2035-
} catch {
2036-
case e: AnalysisException if e.getCondition == "REQUIRES_SINGLE_PART_NAMESPACE" =>
2037-
// Only convert for 2–3 part names; 4+ parts keep REQUIRES_SINGLE_PART_NAMESPACE
2038-
if (nameParts.size <= 3) {
2039-
val catalogPath =
2040-
catalogManager.currentCatalog.name +: catalogManager.currentNamespace
2041-
val searchPath = SQLConf.get.resolutionSearchPath(catalogPath.toSeq)
2042-
.map(_.map(quoteIfNeeded).mkString("."))
2043-
throw QueryCompilationErrors.unresolvedRoutineError(
2044-
nameParts,
2045-
searchPath,
2046-
f.origin)
2047-
} else {
2048-
throw e
2049-
}
2050-
}
2031+
val CatalogAndIdentifier(catalog, ident) =
2032+
relationResolution.expandIdentifier(nameParts)
20512033

20522034
val fullName = normalizeFuncName(
20532035
(catalog.name +: ident.namespace :+ ident.name).toImmutableArraySeq)
@@ -2057,25 +2039,8 @@ class Analyzer(
20572039
f
20582040
} else {
20592041
// Not in cache - do full lookup to determine type
2060-
// Session catalog may throw REQUIRES_SINGLE_PART_NAMESPACE for multi-part namespace
2061-
val functionType = try {
2042+
val functionType =
20622043
functionResolution.lookupFunctionType(nameParts, Some(f))
2063-
} catch {
2064-
case e: AnalysisException if e.getCondition == "REQUIRES_SINGLE_PART_NAMESPACE" =>
2065-
// Only convert for 3-part names; 4+ parts keep REQUIRES_SINGLE_PART_NAMESPACE
2066-
if (nameParts.size == 3) {
2067-
val catalogPath =
2068-
catalogManager.currentCatalog.name +: catalogManager.currentNamespace
2069-
val searchPath = SQLConf.get.resolutionSearchPath(catalogPath.toSeq)
2070-
.map(_.map(quoteIfNeeded).mkString("."))
2071-
throw QueryCompilationErrors.unresolvedRoutineError(
2072-
nameParts,
2073-
searchPath,
2074-
f.origin)
2075-
} else {
2076-
throw e
2077-
}
2078-
}
20792044

20802045
functionType match {
20812046
case FunctionType.Local =>
@@ -2095,7 +2060,7 @@ class Analyzer(
20952060
val catalogPath =
20962061
catalogManager.currentCatalog.name +: catalogManager.currentNamespace
20972062
val searchPath = SQLConf.get.resolutionSearchPath(catalogPath.toSeq)
2098-
.map(_.map(quoteIfNeeded).mkString("."))
2063+
.map(_.quoted)
20992064
throw QueryCompilationErrors.unresolvedRoutineError(
21002065
nameParts,
21012066
searchPath,

sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/analysis/FunctionResolution.scala

Lines changed: 22 additions & 76 deletions
Original file line numberDiff line numberDiff line change
@@ -25,7 +25,6 @@ import org.apache.spark.internal.Logging
2525
import org.apache.spark.sql.AnalysisException
2626
import org.apache.spark.sql.catalyst.FunctionIdentifier
2727
import org.apache.spark.sql.catalyst.analysis.FunctionRegistry
28-
import org.apache.spark.sql.catalyst.catalog.SessionCatalog
2928
import org.apache.spark.sql.catalyst.expressions._
3029
import org.apache.spark.sql.catalyst.expressions.aggregate._
3130
import org.apache.spark.sql.catalyst.plans.logical.LogicalPlan
@@ -69,6 +68,13 @@ class FunctionResolution(
6968

7069
private val trimWarningEnabled = new AtomicBoolean(true)
7170

71+
/** Returns the current catalog path, preferring the view's context if resolving a view. */
72+
private def currentCatalogPath: Seq[String] = {
73+
val ctx = AnalysisContext.get.catalogAndNamespace
74+
if (ctx.nonEmpty) ctx
75+
else (Seq(catalogManager.currentCatalog.name) ++ catalogManager.currentNamespace).toSeq
76+
}
77+
7278
/**
7379
* Produces the ordered list of fully qualified candidate names for resolution.
7480
*
@@ -77,12 +83,7 @@ class FunctionResolution(
7783
*/
7884
private def resolutionCandidates(nameParts: Seq[String]): Seq[Seq[String]] = {
7985
if (nameParts.size == 1) {
80-
val catalogPath = if (AnalysisContext.get.catalogAndNamespace.nonEmpty) {
81-
AnalysisContext.get.catalogAndNamespace
82-
} else {
83-
(Seq(catalogManager.currentCatalog.name) ++ catalogManager.currentNamespace).toSeq
84-
}
85-
val searchPath = SQLConf.get.resolutionSearchPath(catalogPath)
86+
val searchPath = SQLConf.get.resolutionSearchPath(currentCatalogPath)
8687
searchPath.map(_ ++ nameParts)
8788
} else {
8889
nameParts.size match {
@@ -100,7 +101,7 @@ class FunctionResolution(
100101
unresolvedFunc: UnresolvedFunction): Option[Expression] = {
101102
if (nameParts.length == 3 &&
102103
nameParts.head.equalsIgnoreCase(CatalogManager.SYSTEM_CATALOG_NAME)) {
103-
// Try resolving as a session-namespace function (builtin, temp, extension)
104+
// Try resolving as a session-namespace function (builtin or temp)
104105
FunctionResolution.sessionNamespaceKind(nameParts).flatMap { kind =>
105106
val funcName = nameParts.last
106107
val expr = v1SessionCatalog.resolveScalarFunction(kind, funcName, unresolvedFunc.arguments)
@@ -126,16 +127,6 @@ class FunctionResolution(
126127
resolveV2Function(unboundV2Func, unresolvedFunc.arguments, unresolvedFunc)
127128
})
128129
} catch {
129-
case e: AnalysisException if e.getCondition == "REQUIRES_SINGLE_PART_NAMESPACE" &&
130-
nameParts.size == 3 =>
131-
val catalogPath = if (AnalysisContext.get.catalogAndNamespace.nonEmpty) {
132-
AnalysisContext.get.catalogAndNamespace
133-
} else {
134-
(Seq(catalogManager.currentCatalog.name) ++ catalogManager.currentNamespace).toSeq
135-
}
136-
val searchPath = SQLConf.get.resolutionSearchPath(catalogPath)
137-
throw QueryCompilationErrors.unresolvedRoutineError(
138-
unresolvedFunc.nameParts, searchPath.map(toSQLId), unresolvedFunc.origin)
139130
case _: NoSuchFunctionException =>
140131
None
141132
case _: NoSuchNamespaceException =>
@@ -176,12 +167,7 @@ class FunctionResolution(
176167
case None =>
177168
}
178169
}
179-
val catalogPath = if (AnalysisContext.get.catalogAndNamespace.nonEmpty) {
180-
AnalysisContext.get.catalogAndNamespace
181-
} else {
182-
(Seq(catalogManager.currentCatalog.name) ++ catalogManager.currentNamespace).toSeq
183-
}
184-
val searchPath = SQLConf.get.resolutionSearchPath(catalogPath)
170+
val searchPath = SQLConf.get.resolutionSearchPath(currentCatalogPath)
185171
throw QueryCompilationErrors.unresolvedRoutineError(
186172
unresolvedFunc.nameParts, searchPath.map(toSQLId), unresolvedFunc.origin)
187173
}
@@ -300,46 +286,19 @@ class FunctionResolution(
300286
/**
301287
* Determines the type/location of a function (builtin, temporary, persistent, etc.).
302288
* This is used by the LookupFunctions analyzer rule for early validation and optimization.
303-
* This method only performs the lookup and classification - it does not throw errors.
289+
*
290+
* Note: may throw for malformed identifiers (e.g. REQUIRES_SINGLE_PART_NAMESPACE).
304291
*
305292
* @param nameParts The function name parts.
306293
* @param unresolvedFunc Optional UnresolvedFunction node for lookups that may need it.
307-
* @return The type of the function (Builtin, Temporary, Persistent, TableOnly, or NotFound).
294+
* @return The type of the function (Local, Persistent, TableOnly, or NotFound).
308295
*/
309296
def lookupFunctionType(
310297
nameParts: Seq[String],
311298
unresolvedFunc: Option[UnresolvedFunction] = None): FunctionType = {
312299

313-
// Check if it's explicitly qualified as extension, builtin, or temp
314-
FunctionResolution.sessionNamespaceKind(nameParts) match {
315-
case Some(SessionCatalog.Extension) | Some(SessionCatalog.Builtin) =>
316-
if (lookupBuiltinOrTempFunction(nameParts, unresolvedFunc).isDefined) {
317-
return FunctionType.Local // Extension and builtin both as Local
318-
}
319-
case Some(SessionCatalog.Temp) =>
320-
if (lookupBuiltinOrTempFunction(nameParts, unresolvedFunc).isDefined) {
321-
return FunctionType.Local
322-
}
323-
case None =>
324-
// Unqualified or qualified with a catalog
325-
// Use lookupBuiltinOrTempFunction which handles internal functions correctly
326-
val funcInfoOpt = lookupBuiltinOrTempFunction(nameParts, unresolvedFunc)
327-
funcInfoOpt match {
328-
case Some(info) =>
329-
// Determine if it's extension, temp, or builtin from the ExpressionInfo
330-
if (info.getDb == CatalogManager.EXTENSION_NAMESPACE) {
331-
return FunctionType.Local
332-
} else if (info.getDb == CatalogManager.SESSION_NAMESPACE) {
333-
if (nameParts.size == 1 && unresolvedFunc.exists(_.isInternal)) {
334-
return FunctionType.Local
335-
} else {
336-
return FunctionType.Local
337-
}
338-
} else {
339-
return FunctionType.Local
340-
}
341-
case None =>
342-
}
300+
if (lookupBuiltinOrTempFunction(nameParts, unresolvedFunc).isDefined) {
301+
return FunctionType.Local
343302
}
344303

345304
// Check if function exists as table function only
@@ -601,46 +560,33 @@ class FunctionResolution(
601560
* Companion object with shared utility methods for function name qualification checks.
602561
*/
603562
object FunctionResolution {
604-
/**
605-
* Check if a function name is qualified as an extension function.
606-
* Valid forms: extension.func or system.extension.func
607-
*/
608-
private def maybeExtensionFunctionName(nameParts: Seq[String]): Boolean = {
609-
isQualifiedWithNamespace(nameParts, CatalogManager.EXTENSION_NAMESPACE)
610-
}
611-
612563
/**
613564
* Check if a function name is qualified as a builtin function.
614565
* Valid forms: builtin.func or system.builtin.func
615566
*/
616567
private def maybeBuiltinFunctionName(nameParts: Seq[String]): Boolean = {
617-
isQualifiedWithNamespace(nameParts, CatalogManager.BUILTIN_NAMESPACE)
568+
isQualifiedWithSystemNamespace(nameParts, CatalogManager.BUILTIN_NAMESPACE)
618569
}
619570

620571
/**
621572
* Check if a function name is qualified as a session temporary function.
622573
* Valid forms: session.func or system.session.func
623574
*/
624575
private def maybeTempFunctionName(nameParts: Seq[String]): Boolean = {
625-
isQualifiedWithNamespace(nameParts, CatalogManager.SESSION_NAMESPACE)
576+
isQualifiedWithSystemNamespace(nameParts, CatalogManager.SESSION_NAMESPACE)
626577
}
627578

628579
/**
629580
* Single qualification result for session namespaces: returns the kind when nameParts
630-
* is explicitly qualified as extension, builtin, or session; None otherwise.
631-
* Use this instead of calling maybeBuiltinFunctionName/maybeTempFunctionName/
632-
* maybeExtensionFunctionName in multiple places so namespace rules live in one place.
581+
* is explicitly qualified as builtin or session; None otherwise.
633582
*
634583
* @param nameParts The function name parts (e.g. Seq("builtin", "abs"), Seq("session", "my_udf"))
635-
* @return Some(Builtin), Some(Temp), or Some(Extension) for 2/3-part session qualification;
636-
* None otherwise
584+
* @return Some(Builtin) or Some(Temp) for 2/3-part session qualification; None otherwise
637585
*/
638586
def sessionNamespaceKind(nameParts: Seq[String])
639587
: Option[org.apache.spark.sql.catalyst.catalog.SessionCatalog.SessionFunctionKind] = {
640588
if (nameParts.length <= 1) None
641-
else if (maybeExtensionFunctionName(nameParts)) {
642-
Some(org.apache.spark.sql.catalyst.catalog.SessionCatalog.Extension)
643-
} else if (maybeBuiltinFunctionName(nameParts)) {
589+
else if (maybeBuiltinFunctionName(nameParts)) {
644590
Some(org.apache.spark.sql.catalyst.catalog.SessionCatalog.Builtin)
645591
} else if (maybeTempFunctionName(nameParts)) {
646592
Some(org.apache.spark.sql.catalyst.catalog.SessionCatalog.Temp)
@@ -653,10 +599,10 @@ object FunctionResolution {
653599
* Validates both the namespace prefix AND that a function name is present.
654600
*
655601
* @param nameParts The multi-part name to check
656-
* @param namespace The namespace to check for (e.g., "extension", "builtin", "session")
602+
* @param namespace The namespace to check for (e.g., "builtin", "session")
657603
* @return true if qualified with the given namespace and has a non-empty function name
658604
*/
659-
private def isQualifiedWithNamespace(nameParts: Seq[String], namespace: String): Boolean = {
605+
private def isQualifiedWithSystemNamespace(nameParts: Seq[String], namespace: String): Boolean = {
660606
nameParts.length match {
661607
case 2 =>
662608
// Format: namespace.funcName (e.g., "builtin.abs")

sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/analysis/ResolveCatalogs.scala

Lines changed: 32 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -22,7 +22,6 @@ import scala.jdk.CollectionConverters._
2222
import org.apache.spark.SparkException
2323
import org.apache.spark.sql.AnalysisException
2424
import org.apache.spark.sql.catalyst.{FunctionIdentifier, SqlScriptingContextManager}
25-
import org.apache.spark.sql.catalyst.catalog.SessionCatalog
2625
import org.apache.spark.sql.catalyst.expressions.AttributeReference
2726
import org.apache.spark.sql.catalyst.plans.logical._
2827
import org.apache.spark.sql.catalyst.rules.Rule
@@ -86,9 +85,30 @@ class ResolveCatalogs(val catalogManager: CatalogManager)
8685
assertValidSessionVariableNameParts(nameParts, resolved)
8786
d.copy(name = resolved)
8887

88+
case CreateFunction(UnresolvedIdentifier(nameParts, _), _, _, _, _)
89+
if isSystemBuiltinName(nameParts) =>
90+
throw QueryCompilationErrors.operationNotAllowedOnBuiltinFunctionError(
91+
"CREATE", nameParts.last)
92+
93+
case CreateUserDefinedFunction(UnresolvedIdentifier(nameParts, _),
94+
_, _, _, _, _, _, _, _, _, _, _)
95+
if isSystemBuiltinName(nameParts) =>
96+
throw QueryCompilationErrors.operationNotAllowedOnBuiltinFunctionError(
97+
"CREATE", nameParts.last)
98+
99+
case DropFunction(UnresolvedIdentifier(nameParts, _), _)
100+
if isSystemBuiltinName(nameParts) =>
101+
throw QueryCompilationErrors.operationNotAllowedOnBuiltinFunctionError(
102+
"DROP", nameParts.last)
103+
89104
case d @ DropFunction(u @ UnresolvedIdentifier(nameParts, _), _) =>
90105
d.copy(child = resolveFunctionIdentifier(nameParts, u.origin))
91106

107+
case RefreshFunction(UnresolvedIdentifier(nameParts, _))
108+
if isSystemBuiltinName(nameParts) =>
109+
throw QueryCompilationErrors.operationNotAllowedOnBuiltinFunctionError(
110+
"REFRESH", nameParts.last)
111+
92112
case r @ RefreshFunction(u @ UnresolvedIdentifier(nameParts, _)) =>
93113
r.copy(child = resolveFunctionIdentifier(nameParts, u.origin))
94114

@@ -135,11 +155,16 @@ class ResolveCatalogs(val catalogManager: CatalogManager)
135155
}
136156
}
137157

158+
private def isSystemBuiltinName(nameParts: Seq[String]): Boolean = {
159+
nameParts.length == 3 &&
160+
nameParts(0).equalsIgnoreCase(CatalogManager.SYSTEM_CATALOG_NAME) &&
161+
nameParts(1).equalsIgnoreCase(CatalogManager.BUILTIN_NAMESPACE)
162+
}
163+
138164
/**
139165
* Resolves a function identifier, checking for builtin and temp functions first.
140-
* Builtin and temp functions are only registered with unqualified names, but can be
141-
* referenced with qualified names like builtin.abs, system.builtin.abs, session.func,
142-
* or system.session.func.
166+
* Only unqualified (1-part) names get special builtin/temp handling; multi-part names
167+
* go through standard catalog resolution.
143168
*/
144169
private def resolveFunctionIdentifier(
145170
nameParts: Seq[String],
@@ -157,18 +182,9 @@ class ResolveCatalogs(val catalogManager: CatalogManager)
157182
val CatalogAndIdentifier(catalog, ident) = nameParts
158183
ResolvedIdentifier(catalog, ident)
159184
}
160-
} else FunctionResolution.sessionNamespaceKind(nameParts) match {
161-
case Some(SessionCatalog.Builtin) | Some(SessionCatalog.Extension) =>
162-
// Explicitly qualified as builtin or extension (extension stored as builtin)
163-
val ident = Identifier.of(Array(CatalogManager.BUILTIN_NAMESPACE), nameParts.last)
164-
ResolvedIdentifier(FakeSystemCatalog, ident)
165-
case Some(SessionCatalog.Temp) =>
166-
// Explicitly qualified as temp (e.g., session.func or system.session.func)
167-
val ident = Identifier.of(Array(CatalogManager.SESSION_NAMESPACE), nameParts.last)
168-
ResolvedIdentifier(FakeSystemCatalog, ident)
169-
case None =>
170-
val CatalogAndIdentifier(catalog, ident) = nameParts
171-
ResolvedIdentifier(catalog, ident)
185+
} else {
186+
val CatalogAndIdentifier(catalog, ident) = nameParts
187+
ResolvedIdentifier(catalog, ident)
172188
}
173189
}
174190

0 commit comments

Comments
 (0)