gengliangwang commented on code in PR #53570:
URL: https://github.com/apache/spark/pull/53570#discussion_r2718233081
##########
sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/analysis/Analyzer.scala:
##########
@@ -2003,24 +2069,55 @@ class Analyzer(
plan.resolveExpressionsWithPruning(_.containsAnyPattern(UNRESOLVED_FUNCTION)) {
case f @ UnresolvedFunction(nameParts, _, _, _, _, _, _) =>
- if (functionResolution.lookupBuiltinOrTempFunction(nameParts,
Some(f)).isDefined) {
+ // For builtin/temp functions, we can do a quick check without
catalog lookup
+ val quickCheck = if (nameParts.size == 1) {
+ functionResolution.lookupBuiltinOrTempFunction(nameParts, Some(f))
+ } else if (FunctionResolution.maybeBuiltinFunctionName(nameParts) ||
+ FunctionResolution.maybeTempFunctionName(nameParts)) {
+ functionResolution.lookupBuiltinOrTempFunction(nameParts, Some(f))
+ } else {
+ None
+ }
+
+ if (quickCheck.isDefined) {
+ // It's a builtin or temp function - no need for catalog lookup or
caching
f
} else {
+ // Might be a persistent function - compute full name and check
cache first
val CatalogAndIdentifier(catalog, ident) =
relationResolution.expandIdentifier(nameParts)
- val fullName =
- normalizeFuncName((catalog.name +: ident.namespace :+
ident.name).toImmutableArraySeq)
+ val fullName = normalizeFuncName(
+ (catalog.name +: ident.namespace :+
ident.name).toImmutableArraySeq)
+
if (externalFunctionNameSet.contains(fullName)) {
- f
- } else if (catalog.asFunctionCatalog.functionExists(ident)) {
- externalFunctionNameSet.add(fullName)
+ // Already validated this function exists - skip lookup
f
} else {
- val catalogPath = (catalog.name() +:
catalogManager.currentNamespace).mkString(".")
- throw QueryCompilationErrors.unresolvedRoutineError(
- nameParts,
- Seq("system.builtin", "system.session", catalogPath),
- f.origin)
+ // Not in cache - do full lookup to determine type
+ val functionType =
functionResolution.lookupFunctionType(nameParts, Some(f))
+
+ functionType match {
+ case FunctionType.Builtin | FunctionType.Temporary =>
+ // This shouldn't happen since we checked above, but handle
it
Review Comment:
If this is un-reachable, shall we add warning logs?
--
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]
