[SPARK-54760][SQL] DelegatingCatalogExtension as session catalog supports both V1 and V2 functions by pan3793 · Pull Request #53531 · apache/spark

pan3793 · 2025-12-18T18:07:12Z

What changes were proposed in this pull request?

This PR fixes a bug that occurs when the user uses a custom DelegatingCatalogExtension as the session catalog, Spark can not load the v2 function properly provided by the catalog. A typical use case is Iceberg's SparkSessionCatalog

$ spark-sql \
  --conf spark.sql.catalog.spark_catalog=org.apache.iceberg.spark.SparkSessionCatalog \
  ...

spark-sql (default)> SELECT spark_catalog.system.iceberg_version();
[ROUTINE_NOT_FOUND] The routine `system`.`iceberg_version` cannot be found. Verify the spelling and correctness of the schema and catalog.
If you did not qualify the name with a schema and catalog, verify the current_schema() output, or qualify the name with the correct schema and catalog.
To tolerate the error on drop use DROP ... IF EXISTS. SQLSTATE: 42883; line 1 pos 7

Why are the changes needed?

Fix bug.

Does this PR introduce any user-facing change?

Yes, it fixes a bug.

How was this patch tested?

Add new UT. Also manually tested with Iceberg.

spark-sql (default)> SELECT spark_catalog.system.iceberg_version();
1.10.0
Time taken: 1.715 seconds, Fetched 1 row(s)

Was this patch authored or co-authored using generative AI tooling?

No.

pan3793 · 2025-12-18T18:09:55Z

-  "errorClass" : "IDENTIFIER_TOO_MANY_NAME_PARTS",
-  "sqlState" : "42601",
+  "errorClass" : "REQUIRES_SINGLE_PART_NAMESPACE",
+  "sqlState" : "42K05",


either before or after this change, the error condition of the query VALUES(IDENTIFIER('a.b.c.d')()) makes sense.

before stacktrace

org.apache.spark.sql.AnalysisException: [IDENTIFIER_TOO_MANY_NAME_PARTS] `a`.`b`.`c`.`d` is not a valid identifier as it has more than 2 name parts. SQLSTATE: 42601; line 1 pos 7 at org.apache.spark.sql.errors.QueryCompilationErrors$.identifierTooManyNamePartsError(QueryCompilationErrors.scala:2330) at org.apache.spark.sql.connector.catalog.CatalogV2Implicits$IdentifierHelper.asFunctionIdentifier(CatalogV2Implicits.scala:197) at org.apache.spark.sql.catalyst.analysis.FunctionResolution.$anonfun$resolveFunction$2(FunctionResolution.scala:56) at scala.Option.getOrElse(Option.scala:201) at org.apache.spark.sql.catalyst.analysis.FunctionResolution.$anonfun$resolveFunction$1(FunctionResolution.scala:52) at org.apache.spark.sql.catalyst.analysis.package$.withPosition(package.scala:104) at org.apache.spark.sql.catalyst.analysis.FunctionResolution.resolveFunction(FunctionResolution.scala:52) ...

after stacktrace

org.apache.spark.sql.AnalysisException: [REQUIRES_SINGLE_PART_NAMESPACE] spark_catalog requires a single-part namespace, but got `a`.`b`.`c`. SQLSTATE: 42K05; line 1 pos 7 at org.apache.spark.sql.errors.QueryCompilationErrors$.requiresSinglePartNamespaceError(QueryCompilationErrors.scala:1535) at org.apache.spark.sql.execution.datasources.v2.V2SessionCatalog$TableIdentifierHelper.asFunctionIdentifier(V2SessionCatalog.scala:381) at org.apache.spark.sql.execution.datasources.v2.V2SessionCatalog.loadFunction(V2SessionCatalog.scala:478) at org.apache.spark.sql.connector.catalog.DelegatingCatalogExtension.loadFunction(DelegatingCatalogExtension.java:182) at org.apache.spark.sql.connector.InMemoryTableSessionCatalog.org$apache$spark$sql$connector$TestV2SessionCatalogBase$$super$loadFunction(DataSourceV2DataFrameSessionCatalogSuite.scala:101) at org.apache.spark.sql.connector.TestV2SessionCatalogBase.loadFunction(TestV2SessionCatalogBase.scala:139) at org.apache.spark.sql.connector.TestV2SessionCatalogBase.loadFunction$(TestV2SessionCatalogBase.scala:135) at org.apache.spark.sql.connector.InMemoryTableSessionCatalog.loadFunction(DataSourceV2DataFrameSessionCatalogSuite.scala:101) at org.apache.spark.sql.catalyst.analysis.FunctionResolution.$anonfun$resolveFunction$2(FunctionResolution.scala:54) at scala.Option.getOrElse(Option.scala:201) at org.apache.spark.sql.catalyst.analysis.FunctionResolution.$anonfun$resolveFunction$1(FunctionResolution.scala:51) at org.apache.spark.sql.catalyst.analysis.package$.withPosition(package.scala:104) at org.apache.spark.sql.catalyst.analysis.FunctionResolution.resolveFunction(FunctionResolution.scala:51) ...

pan3793 · 2025-12-18T18:52:18Z

  }
+}

-  private case object StrLenDefault extends ScalarFunction[Int] {


all changes in this file are just exposing those V2 functions to allow reusing in more test cases

pan3793 · 2025-12-18T18:54:20Z

+    sessionCatalog.createFunction(Identifier.of(Array("ns"), "strlen"), StrLen(StrLenDefault))
+    checkAnswer(
+      sql("SELECT char_length('Hello') as v1, ns.strlen('Spark') as v2"),
+      Row(5, 5))


it's the effective test case, previously it could not load the v2 func ns.strlen

pan3793 · 2025-12-18T19:00:49Z

-        if (CatalogV2Util.isSessionCatalog(catalog)) {
-          resolveV1Function(ident.asFunctionIdentifier, u.arguments, u)
-        } else {
-          resolveV2Function(catalog.asFunctionCatalog, ident, u.arguments, u)
+        catalog.asFunctionCatalog.loadFunction(ident) match {
+          case V1Function(_) =>
+            resolveV1Function(ident.asFunctionIdentifier, u.arguments, u)
+          case unboundV2Func =>
+            resolveV2Function(unboundV2Func, u.arguments, u)


here is the key change - the session catalog also uses V2 catalog API to load the function, the only difference is it may return V1Function, which is a special V2 UnboundFunction and must be resolved differently.

pan3793 · 2025-12-18T19:26:29Z

cc @szehon-ho @viirya @cloud-fan

pan3793 · 2025-12-19T07:17:00Z

@HyukjinKwon, it would be nice if you can consider including this bugfix in the upcoming 4.1.1

cloud-fan · 2025-12-22T05:37:22Z

-          resolveV2Function(catalog.asFunctionCatalog, ident, u.arguments, u)
+        catalog.asFunctionCatalog.loadFunction(ident) match {
+          case V1Function(_) =>
+            resolveV1Function(ident.asFunctionIdentifier, u.arguments, u)


does it mean we look up v1 function twice now? Checking V2SessionCatalog#loadFunction, it already looks up the function.

@cloud-fan yes, the second time lookup looks cheap, do you have any concerns about that?

@cloud-fan, as discussed offline, the second time resolution is just a memory operation and won't produce an expensive RPC call to external catalog, I added a comment to explain it.

cloud-fan · 2025-12-23T07:54:10Z

thanks, merging to master/4.1!

…orts both V1 and V2 functions ### What changes were proposed in this pull request? This PR fixes a bug that occurs when the user uses a custom `DelegatingCatalogExtension` as the session catalog, Spark can not load the v2 function properly provided by the catalog. A typical use case is Iceberg's `SparkSessionCatalog` ``` $ spark-sql \ --conf spark.sql.catalog.spark_catalog=org.apache.iceberg.spark.SparkSessionCatalog \ ... ``` ``` spark-sql (default)> SELECT spark_catalog.system.iceberg_version(); [ROUTINE_NOT_FOUND] The routine `system`.`iceberg_version` cannot be found. Verify the spelling and correctness of the schema and catalog. If you did not qualify the name with a schema and catalog, verify the current_schema() output, or qualify the name with the correct schema and catalog. To tolerate the error on drop use DROP ... IF EXISTS. SQLSTATE: 42883; line 1 pos 7 ``` ### Why are the changes needed? Fix bug. ### Does this PR introduce _any_ user-facing change? Yes, it fixes a bug. ### How was this patch tested? Add new UT. Also manually tested with Iceberg. ``` spark-sql (default)> SELECT spark_catalog.system.iceberg_version(); 1.10.0 Time taken: 1.715 seconds, Fetched 1 row(s) ``` ### Was this patch authored or co-authored using generative AI tooling? No. Closes #53531 from pan3793/SPARK-54760. Authored-by: Cheng Pan <chengpan@apache.org> Signed-off-by: Wenchen Fan <wenchen@databricks.com> (cherry picked from commit 512099b) Signed-off-by: Wenchen Fan <wenchen@databricks.com>

dongjoon-hyun · 2025-12-23T21:20:49Z

Thank you, @pan3793 and @cloud-fan .

To @pan3793 , could you make backporting PRs for branch-4.0 and branch-3.5?

pan3793 · 2025-12-24T03:24:44Z

@dongjoon-hyun, I think @cloud-fan made a fair decision to backport it only to 4.1, because this is a feature-missing issue instead of a regression or correctness issue. Does the Spark have a clear backport policy, e.g., backport all bug fixes to all active branches?

dongjoon-hyun · 2025-12-24T03:30:49Z

'branch-4.0' is open for 4.1.1 not a 4.1.0.

So, in case of applicable bug fix patches, all live release branches are maintained in the same way. It's only a matter of time when we backport a bug fix from 'master' to 'branch-4.1', 'branch-4.1' to 'branch-4.0', 'branch-4.0' to 'branch-3.5'. It's clear.

cloud-fan · 2025-12-24T04:27:02Z

It's a feature gap rather than a bug. I did the 4.1 backport so that Iceberg doesn't need to wait for another 6 months, but it's not a serious bug that we need to backport to all the active branches.

cloud-fan · 2025-12-27T05:54:50Z

This actually adds an extra function lookup request. Let me revert it from 4.1 first. I'm working on a refactor for master branch to avoid the extra lookup.

### What changes were proposed in this pull request? This PR adds `loadPersistentScalarFunction` method to `SessionCatalog`, to unify the scalar function lookup and resolution between v1 and v2 catalogs. The key difference between v1 and v2 catalogs about function is: - v1 catalog provides different APIs for describing function and invoking function to do function lookup: `lookupPersistentFunction` and `resolvePersistentFunction`. - v2 catalog only has one single lookup API: `loadFunction`. This API returns `UnboundFunction`, for invocation, it needs to enter the second phase and call `UnboundFunction.bind`. The newly added `loadPersistentScalarFunction` method follows the v2 catalog style and handles function invocation (load resources and create function builder) in a second phase. ### Why are the changes needed? Code cleanup, also also avoid redundant function lookup from catalog introduced by #53531 ### Does this PR introduce _any_ user-facing change? No ### How was this patch tested? existing tests ### Was this patch authored or co-authored using generative AI tooling? cursor 2.2.44 Closes #53627 from cloud-fan/func. Authored-by: Wenchen Fan <wenchen@databricks.com> Signed-off-by: Ruifeng Zheng <ruifengz@apache.org>

pan3793 added 3 commits December 19, 2025 01:36

load v2 func then resolve v1 func

420db63

test

3179d79

update golden files

887c593

github-actions Bot added the SQL label Dec 18, 2025

pan3793 commented Dec 18, 2025

View reviewed changes

fix test and revert unnecessary changes

781ff8b

cloud-fan reviewed Dec 22, 2025

View reviewed changes

github-actions Bot added the PYTHON label Dec 22, 2025

cloud-fan reviewed Dec 22, 2025

View reviewed changes

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

cloud-fan reviewed Dec 22, 2025

View reviewed changes

Comment thread sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/analysis/FunctionRegistry.scala Outdated

pan3793 added 2 commits December 23, 2025 10:55

comment

eac256d

nit

90be70f

pan3793 force-pushed the SPARK-54760 branch from 6e05e51 to 90be70f Compare December 23, 2025 02:59

github-actions Bot removed the PYTHON label Dec 23, 2025

pan3793 requested a review from cloud-fan December 23, 2025 05:10

cloud-fan approved these changes Dec 23, 2025

View reviewed changes

cloud-fan closed this in 512099b Dec 23, 2025

cloud-fan mentioned this pull request Dec 27, 2025

[SPARK-54856][SQL] Refactor scalar function lookup and resolution #53627

Closed

akshaytayal mentioned this pull request Sep 17, 2026

[VL] Spark 4.2: Add new UTs — SQL DataSource V2 connector suites apache/gluten#13049

Open

3 tasks

Conversation

pan3793 commented Dec 18, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

What changes were proposed in this pull request?

Why are the changes needed?

Does this PR introduce any user-facing change?

How was this patch tested?

Was this patch authored or co-authored using generative AI tooling?

Uh oh!

pan3793 Dec 18, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

pan3793 Dec 18, 2025

Choose a reason for hiding this comment

Uh oh!

pan3793 Dec 18, 2025

Choose a reason for hiding this comment

Uh oh!

pan3793 Dec 18, 2025

Choose a reason for hiding this comment

Uh oh!

pan3793 commented Dec 18, 2025

Uh oh!

pan3793 commented Dec 19, 2025

Uh oh!

cloud-fan Dec 22, 2025

Choose a reason for hiding this comment

Uh oh!

pan3793 Dec 22, 2025

Choose a reason for hiding this comment

Uh oh!

pan3793 Dec 23, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

cloud-fan commented Dec 23, 2025

Uh oh!

dongjoon-hyun commented Dec 23, 2025

Uh oh!

pan3793 commented Dec 24, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

dongjoon-hyun commented Dec 24, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

cloud-fan commented Dec 24, 2025

Uh oh!

cloud-fan commented Dec 27, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

pan3793 commented Dec 18, 2025 •

edited

Loading

pan3793 Dec 18, 2025 •

edited

Loading

pan3793 Dec 23, 2025 •

edited

Loading

pan3793 commented Dec 24, 2025 •

edited

Loading

dongjoon-hyun commented Dec 24, 2025 •

edited

Loading