vladimirg-db commented on code in PR #53570:
URL: https://github.com/apache/spark/pull/53570#discussion_r2712073937
##########
sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/analysis/resolver/ResolverGuard.scala:
##########
@@ -345,14 +345,42 @@ class ResolverGuard(catalogManager: CatalogManager)
extends SQLConfHelper {
createNamedStruct.children.forall(checkExpression)
}
- private def checkUnresolvedFunction(unresolvedFunction: UnresolvedFunction) =
- unresolvedFunction.nameParts.size == 1 &&
-
!ResolverGuard.UNSUPPORTED_FUNCTION_NAMES.contains(unresolvedFunction.nameParts.head)
&&
- // UDFs are not supported
- FunctionRegistry.functionSet.contains(
-
FunctionIdentifier(unresolvedFunction.nameParts.head.toLowerCase(Locale.ROOT))
- ) &&
- unresolvedFunction.children.forall(checkExpression)
+ private def checkUnresolvedFunction(unresolvedFunction: UnresolvedFunction)
= {
Review Comment:
Please avoid the inline comments and create a holistic yet succinct method
doc
##########
sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/analysis/resolver/ResolverGuard.scala:
##########
@@ -345,14 +345,42 @@ class ResolverGuard(catalogManager: CatalogManager)
extends SQLConfHelper {
createNamedStruct.children.forall(checkExpression)
}
- private def checkUnresolvedFunction(unresolvedFunction: UnresolvedFunction) =
- unresolvedFunction.nameParts.size == 1 &&
-
!ResolverGuard.UNSUPPORTED_FUNCTION_NAMES.contains(unresolvedFunction.nameParts.head)
&&
- // UDFs are not supported
- FunctionRegistry.functionSet.contains(
-
FunctionIdentifier(unresolvedFunction.nameParts.head.toLowerCase(Locale.ROOT))
- ) &&
- unresolvedFunction.children.forall(checkExpression)
+ private def checkUnresolvedFunction(unresolvedFunction: UnresolvedFunction)
= {
+ val nameParts = unresolvedFunction.nameParts
+
+ // Only accept unqualified names or names explicitly qualified as builtin.
+ // Session/temporary functions are UDFs and not supported by single-pass
analyzer.
+ // Persistent functions from external catalogs are also not supported.
+ val isBuiltinOrUnqualified = nameParts.length match {
+ case 1 =>
+ // Unqualified: "count" - check if it's a builtin
+ true
+ case 2 =>
+ // Two parts: must be "builtin.count"
+ nameParts.head.equalsIgnoreCase(CatalogManager.BUILTIN_NAMESPACE)
+ case 3 =>
+ // Three parts: must be "system.builtin.count"
+ nameParts(0).equalsIgnoreCase(CatalogManager.SYSTEM_CATALOG_NAME) &&
+ nameParts(1).equalsIgnoreCase(CatalogManager.BUILTIN_NAMESPACE)
+ case _ =>
+ // More than 3 parts is not valid
+ false
+ }
+
+ if (!isBuiltinOrUnqualified) {
+ // This is session.func, catalog.db.function, or invalid - not supported
+ false
Review Comment:
Why wouldn't we support these in single-pass analyzer?
##########
sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/analysis/resolver/ResolverGuard.scala:
##########
@@ -345,14 +345,42 @@ class ResolverGuard(catalogManager: CatalogManager)
extends SQLConfHelper {
createNamedStruct.children.forall(checkExpression)
}
- private def checkUnresolvedFunction(unresolvedFunction: UnresolvedFunction) =
- unresolvedFunction.nameParts.size == 1 &&
-
!ResolverGuard.UNSUPPORTED_FUNCTION_NAMES.contains(unresolvedFunction.nameParts.head)
&&
- // UDFs are not supported
- FunctionRegistry.functionSet.contains(
-
FunctionIdentifier(unresolvedFunction.nameParts.head.toLowerCase(Locale.ROOT))
- ) &&
- unresolvedFunction.children.forall(checkExpression)
+ private def checkUnresolvedFunction(unresolvedFunction: UnresolvedFunction)
= {
+ val nameParts = unresolvedFunction.nameParts
+
+ // Only accept unqualified names or names explicitly qualified as builtin.
+ // Session/temporary functions are UDFs and not supported by single-pass
analyzer.
+ // Persistent functions from external catalogs are also not supported.
+ val isBuiltinOrUnqualified = nameParts.length match {
Review Comment:
Please create a private method for this logic
--
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]
