zahed1994 opened a new pull request, #59184: URL: https://github.com/apache/spark/pull/59184
### What changes were proposed in this pull request? Prior to this PR, user-defined function (SQL UDF) creation was represented across two redundant Catalyst logical plan abstractions introduced in [SPARK-48730](https://issues.apache.org/jira/browse/SPARK-48730): 1. A Catalyst logical plan node `CreateUserDefinedFunction` (`case class` in `v2Commands.scala`), and 2. An execution-layer command `CreateUserDefinedFunctionCommand` extending `LeafRunnableCommand` (in `sql/core/.../command/`). This separation resulted in architectural divergence from other DDL commands (such as `UnsetNamespacePropertiesCommand`), duplicate AST structures, and precluded standard Catalyst rule-based identifier resolution across catalogs. This PR unifies SQL UDF creation under `UnaryRunnableCommand` as recommended in upstream review comments for [SPARK-50698](https://issues.apache.org/jira/browse/SPARK-50698): 1. **Catalyst Logical Plan Abstraction (`v2Commands.scala`)**: - Replaces the standalone `case class CreateUserDefinedFunction` with `trait CreateUserDefinedFunctionLike extends Command`. - Defines a unified trait contract (`name: LogicalPlan`, `userDefinedFunction: UserDefinedFunction`, `ignoreIfExists: Boolean`, `replace: Boolean`). 2. **Unified Execution Command (`CreateUserDefinedFunctionCommand.scala` & `CreateSQLFunctionCommand.scala`)**: - Refactors `CreateUserDefinedFunctionCommand` (and concrete subclass `CreateSQLFunctionCommand`) to extend `UnaryRunnableCommand with CapturesConfig with CreateUserDefinedFunctionLike`. - Exposes `child: LogicalPlan` to represent the function identifier AST, where `name` delegates to `child`. - Implements `withNewChildInternal(newChild: LogicalPlan)` for immutable tree transformations. - In `run(SparkSession)`, extracts `(catalog, ident)` directly from the analyzed `ResolvedIdentifier` child. 3. **Identifier Resolution & Catalog Isolation (`SparkSqlParser.scala`, `ResolveCatalogs.scala`, `ResolveSessionCatalog.scala`)**: - **Persistent SQL Functions**: `SparkSqlParser` wraps the parsed multipart identifier in an `UnresolvedIdentifier`. During Catalyst analysis: - `ResolveCatalogs` matches `CreateUserDefinedFunctionLike` with `UnresolvedIdentifier(ident)`. - `ResolveSessionCatalog` matches `ResolvedIdentifier(SessionCatalogAndIdentifier(catalog, ident))`, ensuring proper catalog delegation to `V2SessionCatalog` (`spark_catalog`). - **Temporary SQL Functions**: Session-scoped temporary functions cannot be delegated to an external or default catalog namespace. `SparkSqlParser` wraps the identifier as `ResolvedIdentifier(FakeSystemCatalog, Identifier.of(Array("session"), funcName))`. Because the node is pre-resolved, `ResolveCatalogs` bypasses namespace re-qualification, preserving strict session-local semantics. 4. **Analyzer Rule Updates & Binary Compatibility**: - Updates `ApplyDefaultCollation` pattern matches to handle `CreateUserDefinedFunctionCommand`. - Adds necessary MiMa binary compatibility exclusions under `v44excludes` in `project/MimaExcludes.scala` for the refactored constructors and methods. - Regenerates analyzer golden test files to reflect the unified `UnaryRunnableCommand` logical plan representation. ### Why are the changes needed? In [SPARK-48730](https://issues.apache.org/jira/browse/SPARK-48730) (PR #49126), SQL UDF DDL was introduced with split representations across Catalyst and SQL execution. In the original PR review (discussion [r1890214852](https://github.com/apache/spark/pull/49126/files#r1890214852)), committers noted that `CreateUserDefinedFunction` should extend `UnaryRunnableCommand` rather than maintaining duplicate plan nodes. This refactoring: - Eliminates duplicate plan definitions between Catalyst and SQL execution layers. - Integrates function identifier resolution cleanly into standard Catalyst analyzer rules (`ResolveCatalogs`, `ResolveSessionCatalog`). - Preserves session isolation for temporary functions while supporting catalog-qualified resolution for persistent SQL UDFs. - Lays the groundwork for unified default value validation via `ResolveDefaultColumns` ([SPARK-50698](https://issues.apache.org/jira/browse/SPARK-50698)). ### Does this PR introduce _any_ user-facing change? No. This is strictly an internal Catalyst plan and execution command refactoring. SQL syntax, behavior, and query semantics remain identical. ### How was this patch tested? - Unit parser suites: - `CreateSQLFunctionParserSuite` - `DDLParserSuite` - End-to-end SQL query golden file suites: - `SQLQueryTestSuite` (`sql-udf`, `sql-udf-name-precedence`, `sql-udf-name-precedence-legacy`) - Execution & catalog resolution suites: - `SQLFunctionSuite` - `FunctionQualificationSuite` - `SetPathSuite` - `DataSourceV2FunctionSuite` - `ShowFunctionsSuite` - Static analysis & style: - `catalyst/scalastyle` - `sql/scalastyle` ### Was this patch authored or co-authored using generative AI tooling? No. -- 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]
