feat(csharp/src/Drivers/Databricks): Multiple catalogs with default database #2921

toddmeng-db · 2025-06-05T00:13:01Z

Changes

This PR makes changes to be consistent with odbc behavior

ODBC behavior:

Does not attach default schema (from OpenSessionResp) in subsequent queries. This PR changes to be like ODBC here, just to not break anything.
If catalog == "SPARK", do not set catalog in initial namespace; let server return default catalog

ODBC does the following when `EnableMultipleCatalogsSupport=0`

Do not set catalog in the initial namespace.
Do not use the OpenSessionResp catalog for subsequent queries (including metadata queries)

ODBC Driver Behavior Testing

Connection String Catalog/Schema	EnableMultipleCatalogs=False	EnableMultipleCatalogs=True
Only default catalog = x	OpenSession(null, “default”) GetColumns(null, null, null, null)	OpenSession(x, “default”) GetColumns(x, null, null, null)
Only default schema = x	OpenSession(null, x) GetColumns(null, null, null, null)	OpenSession(null, x) GetColumns(hive_metastore, null, null, null)
Both catalog + schema x, y	OpenSession(null, y) GetColumns(null, null, null, null)	OpenSession(x, y) GetColumns(x, null, null, null)
Only default Catalog “SPARK”, schema = null	OpenSessionReq(null, “default”) GetColumns(null, null, null, null)	OpenSessionReq(null, “default”) GetColumns(hive_metastore, null, null, null)
Both with default catalog “SPARK”, schema = x	OpenSessionReq: null Catalog, x schema GetColumns(null, null, null, null)	OpenSessionReq(null, x) GetColumns(hive_metastore, null, null, null)

Testing

Adds an extensive grid test to validate different edge cases during creation

TODO

Make initial_namespace.schema "default" as default behavior
CatalogName + Foreign CatlogName
Add testing for old dbr (dbr 7.3, 9.1)

csharp/src/Drivers/Databricks/DatabricksStatement.cs

eric-wang-1990 · 2025-06-05T06:43:45Z

csharp/src/Drivers/Databricks/DatabricksConnection.cs

            Properties.TryGetValue(AdbcOptions.Connection.CurrentDbSchema, out defaultSchema);

+            // SPARK is a special catalog name - rely on server to return a default catalog if there is one
+            if (defaultCatalog.Equals("SPARK", StringComparison.OrdinalIgnoreCase))


Can you reuse this:

arrow-adbc/csharp/src/Drivers/Databricks/DatabricksStatement.cs

Line 191 in 9bda49a

private void HandleSparkCatalog()

toddmeng-db · 2025-06-05T23:35:44Z

csharp/src/Drivers/Databricks/DatabricksStatement.cs

-                {
-                    SchemaName = defaultNamespace.SchemaName;
-                }
            }


Since ODBC does not use schema for subsequent queries, removing it here to be consistent

eric-wang-1990 · 2025-06-05T23:37:20Z

csharp/src/Drivers/Databricks/DatabricksConnection.cs

            Properties.TryGetValue(AdbcOptions.Connection.CurrentDbSchema, out defaultSchema);

+            // SPARK is a special catalog name - rely on server to return a default catalog if there is one
+            defaultCatalog = HandleSparkCatalog(defaultCatalog);


Just curious: How would some use pass in SPARK as initial catalog? They are passing SPARK in PowerBI UI?

I don't understand this question-and-answer. Someone can pass SPARK as the initial catalog but then it gets ignored? And this is the correct behavior?

This maintains backward compatibility with older workspaces, where the Hive metastore was accessed via the spark catalog name.
In newer DBR versions with Unity Catalog, the default catalog is typically hive_metastore.
Passing null here allows the runtime to fall back to the workspace-defined default catalog for the session.

Let me add a comment to be clearer. Does the above make sense?

eric-wang-1990 · 2025-06-05T23:40:37Z

csharp/src/Drivers/Databricks/DatabricksStatement.cs

-                if (CatalogName == null)
+                if (CatalogName == null && connection.EnableMultipleCatalogSupport)
                {
                    CatalogName = defaultNamespace.CatalogName;


So I realize that for GetCrossReference, it has a CatalogName and ForeignCatalog name, in our case where we will only assign foreign catalog/schema/table to the call, and in those case the catalogName is null and we should not override.
I've tested it, it seems working fine even we assign the catalogName where the actual parentTable is in another catalog. Let's add a TODO here just noting that.

CurtHagenlocher

Thanks! I have one question and one nitpick.

CurtHagenlocher · 2025-06-06T21:28:18Z

csharp/src/Drivers/Databricks/DatabricksConnection.cs

            }
        }
+
+        internal static String? HandleSparkCatalog(String? CatalogName)


Suggested change

internal static String? HandleSparkCatalog(String? CatalogName)

internal static string? HandleSparkCatalog(string? catalogName)

That is, use the type alias string instead of the type name String and use a lower-case argument name.

ah, good catch thanks!

CurtHagenlocher · 2025-06-06T21:29:35Z

csharp/src/Drivers/Databricks/DatabricksConnection.cs

            Properties.TryGetValue(AdbcOptions.Connection.CurrentDbSchema, out defaultSchema);

+            // SPARK is a special catalog name - rely on server to return a default catalog if there is one
+            defaultCatalog = HandleSparkCatalog(defaultCatalog);


I don't understand this question-and-answer. Someone can pass SPARK as the initial catalog but then it gets ignored? And this is the correct behavior?

refactor refactor test more fixes todo comment

toddmeng-db changed the title ~~Fix multiple catalogs with default database~~ feat(csharp/src/Drivers/Databricks): Fix multiple catalogs with default database Jun 5, 2025

toddmeng-db changed the title ~~feat(csharp/src/Drivers/Databricks): Fix multiple catalogs with default database~~ feat(csharp/src/Drivers/Databricks): Multiple catalogs with default database Jun 5, 2025

toddmeng-db force-pushed the todd-meng_db/multiple-catalogs-default-database branch from 50ae107 to 7c511cd Compare June 5, 2025 00:17

toddmeng-db commented Jun 5, 2025

View reviewed changes

csharp/src/Drivers/Databricks/DatabricksStatement.cs Show resolved Hide resolved

toddmeng-db marked this pull request as ready for review June 5, 2025 00:35

toddmeng-db requested a review from CurtHagenlocher as a code owner June 5, 2025 00:35

github-actions bot added this to the ADBC Libraries 19 milestone Jun 5, 2025

toddmeng-db marked this pull request as draft June 5, 2025 00:37

toddmeng-db force-pushed the todd-meng_db/multiple-catalogs-default-database branch 2 times, most recently from 9b86054 to d28726c Compare June 5, 2025 06:22

eric-wang-1990 reviewed Jun 5, 2025

View reviewed changes

toddmeng-db force-pushed the todd-meng_db/multiple-catalogs-default-database branch 3 times, most recently from 300f0d0 to c415169 Compare June 5, 2025 23:35

toddmeng-db commented Jun 5, 2025

View reviewed changes

eric-wang-1990 reviewed Jun 5, 2025

View reviewed changes

eric-wang-1990 approved these changes Jun 5, 2025

View reviewed changes

toddmeng-db marked this pull request as ready for review June 6, 2025 00:27

CurtHagenlocher requested changes Jun 6, 2025

View reviewed changes

toddmeng-db added 2 commits June 6, 2025 15:28

Fix multiple catalogs with default database

b23bf3f

use null catalog if user supplies SPARK

3bf027d

refactor refactor test more fixes todo comment

toddmeng-db force-pushed the todd-meng_db/multiple-catalogs-default-database branch from c8ea6e6 to 3bf027d Compare June 6, 2025 23:13

toddmeng-db requested a review from CurtHagenlocher June 6, 2025 23:47

CurtHagenlocher approved these changes Jun 7, 2025

View reviewed changes

CurtHagenlocher merged commit 95391d0 into apache:main Jun 7, 2025
9 checks passed

serramatutu mentioned this pull request Aug 12, 2025

Sync upstream dbt-labs/arrow-adbc#52

Closed

	internal static String? HandleSparkCatalog(String? CatalogName)
	internal static string? HandleSparkCatalog(string? catalogName)

feat(csharp/src/Drivers/Databricks): Multiple catalogs with default database #2921

feat(csharp/src/Drivers/Databricks): Multiple catalogs with default database #2921

Uh oh!

Conversation

toddmeng-db commented Jun 5, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Changes

ODBC behavior:

ODBC does the following when EnableMultipleCatalogsSupport=0

ODBC Driver Behavior Testing

Testing

TODO

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

toddmeng-db Jun 6, 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!

Choose a reason for hiding this comment

Uh oh!

CurtHagenlocher left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

toddmeng-db commented Jun 5, 2025 •

edited

Loading

ODBC does the following when `EnableMultipleCatalogsSupport=0`

toddmeng-db Jun 6, 2025 •

edited

Loading