feat(csharp/src/Drivers/Databricks): Default catalogs edge cases #2896

toddmeng-db · 2025-05-30T18:38:33Z

This PR adds support for edge cases that the ODBC driver supported. It also allows users to specify a default schema without a default catalog (relying on the default catalog set by backend server)

Default schema fallback

If spark protocol version is too low, initial namespace will not be respected. This PR sets USE <schema> as a backup immediately after TOpenSessionResp. SET CATALOG <catalog> is not implemented since it is introduced at the same time as InitialNamespace.

Additionally, schema can now be provided in the OpenSessionReq without Catalog.

This is relevant for setting default schemas for dbr < 10.4. It means that we can now provide a default schema for pre-unity-catalog

Testing

To test this add to DATABRICKS_TEST_CONFIG_FILE:

// no catalog
"db_schema": "default_schema",

And use a cluster running dbr < 10.4, run OlderDBRVersion_ShouldSetSchemaViaUseStatement

Default catalog legacy compatibility

If default catalog or default schema is provided in the TOpenSessionResp, we want subsequent getTables calls to use the default namespace. This enables powerbi to act as if it is pre-UC, since it will automatically make all getTables requests with default catalog, which is often hive_metastore.

If no schema is provided in OpenSessionReq, only default catalog will be returned. This means whether or not metadata queries have a default schema provided is dependent on if "db_schema" is provided

So, in powerbi, when only a default schema is provided but not a default catalog, it will automatically restrict itself to the default catalog.

Testing

Added MetadataQuery_ShouldUseResponseNamespace

toddmeng-db · 2025-05-30T19:13:38Z

csharp/src/Drivers/Databricks/DatabricksStatement.cs


        public DatabricksStatement(DatabricksConnection connection)
            : base(connection)
        {


@eric-wang-1990 I think it makes more sense to do the schema filtering here, instead of in powerbi*. Do you think this makes sense?
edit: nevermind, there is no default schema provided in OpenSession response namespace if not specified by client

On second thought, I think this PR prevents us from making blanket metadata queries with no catalog and schema (like GetTables). Is that acceptable?

Why this PR prevent GetTables metadata query?
This would just try to replace null with initial namespace value but should not affect any existing metadata queries

What I meant, is that with this PR, it is impossible to make GetTables call with no catalog attached to the request, since OpenSessionResp seems to typically have a default catalog in the response (even if not user provided). Seems like this behavior happens in odbc too.

So if I wanted to fetch all tables in the metastore, I couldn't use GetTables after this PR. And the way this PR is written, same for schemas, etc.

Discussed offline: user can use % in metadata call to override and list all

csharp/test/Drivers/Databricks/StatementTests.cs

eric-wang-1990 · 2025-05-30T22:12:30Z

csharp/src/Drivers/Databricks/DatabricksConnection.cs

+
+        private async Task SetSchema(string schemaName) {
+            using var statement = new DatabricksStatement(this);
+            statement.SqlQuery = $"USE {schemaName}";


Should this be use database xxx?

odbc does use <xxx>

Should the identifier here be escaped?

It's my vague understanding that the behavior of USE depends on whether or not there is a current catalog. Will there always be one when we get here or is my understanding wrong?

@toddmeng-db Let's make another ticket to handle the escape, all the metadata call does not escape now but should be doing that.
@CurtHagenlocher Regarding whether there is a current catalog, this will only be called on legacy DBR where there is no catalog concept, in such scenarios we should be able to just call use schema.

I see this is now being quoted. Is it possible to use a backtick in the identifier and if so does it need to be doubled when quoted? e.g. should a literal name of ab`cd be represented as `ab``cd` or is such a name not possible?

I'll do what @eric-wang-1990 mentioned and cut a ticket to handle escape

eric-wang-1990 · 2025-05-30T22:27:54Z

csharp/src/Drivers/Databricks/DatabricksStatement.cs

+                }
+                if (SchemaName == null)
+                {
+                    SchemaName = defaultNamespace.SchemaName;


Is this expected? Does simba fill schema with default schema as well?

I need to check. The server will not return a default schema by default, though. So this only affects queries were the user has set the schema. Which seems reasonable

Just tested - odbc does not do this. Should I remove?

How did you confirm this? In the metadata call the schema name can be either null or %, if it's % then the driver will not do anything, it will only replace if it's null. What metadata query did you see for simba?

I ran the call directly to odbc (not via powerbi). If both catalog and schema are provided, schema is still % by default

Listing tables 'catalog : peco, schemaPattern : %, tableTypes : null, tableName : %'

eric-wang-1990 · 2025-05-30T23:20:53Z

csharp/src/Drivers/Databricks/DatabricksConnection.cs

+        {
+            await base.HandleOpenSessionResponse(session);
+            if (session != null)
+            {


Since you are already here, can you also override _enableMultipleCatalogSupport with correct value from the response?

eric-wang-1990 · 2025-06-02T22:27:19Z

csharp/src/Drivers/Databricks/DatabricksConnection.cs

            {
                _defaultNamespace = new TNamespace
                {
                    CatalogName = defaultCatalog,


Should we only set ifcatalog/schema is not null?

I see that odbc allows for setting catalog to whitespace; but I also think this will make the query broken, right? since there are no whitespace catalogs/schemas?:
Listing tables 'catalog : , schemaPattern : %, tableTypes : null, tableName : %'

eric-wang-1990 · 2025-06-02T22:30:36Z

csharp/src/Drivers/Databricks/DatabricksConnection.cs

+                {
+                    _defaultNamespace = session.InitialNamespace;
+                }
+                else if (_defaultNamespace != null && !string.IsNullOrEmpty(_defaultNamespace.SchemaName))


Let's add some more comments around this else clause, basically this only happens for when there is no initialNamespace response, which means it's an old DBR + Also user pass in a default schema name.
We are not doing it for catalog since multipleCatalog is needed for specifying catalogs, and that is the same protocol version with which we support for initialNamespace, which means if the response does not contain initialNamespace means we do not support specifying catalog at all.

CurtHagenlocher

Thanks! A few minor questions/comments; feel free to push back.

CurtHagenlocher · 2025-06-02T23:08:02Z

csharp/src/Drivers/Databricks/DatabricksStatement.cs

+            // set the catalog name for legacy compatibility
+            // TODO: use catalog and schema fields in hiveserver2 connection instad of DefaultNamespace so we don't need to cast
+            var defaultNamespace = ((DatabricksConnection)Connection).DefaultNamespace;
+            if (defaultNamespace != null) {


nit: move opening brace to new line

CurtHagenlocher · 2025-06-02T23:08:26Z

csharp/src/Drivers/Databricks/readme.md

 Basic (username and password) authentication is not supported at this time.

-Optional default catalog and default schema can be set for the session with `adbc.connection.catalog` and `adbc.connection.db_schema` (catalog must be set if default schema is provided).
+Optional default catalog and default schema can be set for the session with `adbc.connection.catalog` and `adbc.connection.db_schema`. The default catalog and schema will be used for subsequent metadata calls unless user specified different catalog/schema to use.


nit: consider wrapping onto a second line

CurtHagenlocher · 2025-06-02T23:12:14Z

csharp/src/Drivers/Databricks/DatabricksConnection.cs

+            }
+        }
+
+        private async Task SetSchema(string schemaName) {


nit: move opening brace to new line

CurtHagenlocher · 2025-06-02T23:15:04Z

csharp/src/Drivers/Databricks/DatabricksConnection.cs

+
+        private async Task SetSchema(string schemaName) {
+            using var statement = new DatabricksStatement(this);
+            statement.SqlQuery = $"USE {schemaName}";


Should the identifier here be escaped?

It's my vague understanding that the behavior of USE depends on whether or not there is a current catalog. Will there always be one when we get here or is my understanding wrong?

toddmeng-db · 2025-06-03T06:27:25Z

It's my vague understanding that the behavior of USE depends on whether or not there is a current catalog

Thats a good point - here's the docs. UC comes with a default catalog that does not need to be set explicitly, so the call should work. Pre-UC, USE is also still appropriate

For some workspaces that were enabled for Unity Catalog automatically, the workspace catalog was set as the default catalog. See Automatic enablement of Unity Catalog.
For all other workspaces, the hive_metastore catalog was set as the default catalog.

toddmeng-db changed the title ~~Default catalogs edge cases~~ feat(csharp/src/Drivers/Databricks): Default catalogs edge cases May 30, 2025

toddmeng-db commented May 30, 2025

View reviewed changes

csharp/test/Drivers/Databricks/StatementTests.cs Show resolved Hide resolved

toddmeng-db force-pushed the toddmeng-db/namespace-fallback branch 5 times, most recently from 269da71 to c19c65e Compare May 30, 2025 22:21

eric-wang-1990 reviewed May 30, 2025

View reviewed changes

toddmeng-db requested a review from eric-wang-1990 June 1, 2025 22:45

toddmeng-db force-pushed the toddmeng-db/namespace-fallback branch from 0548dfd to 652e82b Compare June 2, 2025 16:57

toddmeng-db marked this pull request as ready for review June 2, 2025 17:23

toddmeng-db requested a review from CurtHagenlocher as a code owner June 2, 2025 17:23

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

eric-wang-1990 reviewed Jun 2, 2025

View reviewed changes

toddmeng-db added 6 commits June 2, 2025 16:07

schema fallback

8909621

add support for default catalog in GetTables response

a3a5777

store schema name

906621a

add edge test cases

d0a131e

Set EnableMultipleCatalogSupport from open sesison resp

f5af54c

update new information to readme

8a05e12

toddmeng-db force-pushed the toddmeng-db/namespace-fallback branch from 652e82b to 8a05e12 Compare June 2, 2025 23:11

CurtHagenlocher requested changes Jun 2, 2025

View reviewed changes

add comment

36809d3

toddmeng-db force-pushed the toddmeng-db/namespace-fallback branch from db76c16 to 6d5ff2c Compare June 3, 2025 06:43

escape ticks

83b9ba3

toddmeng-db force-pushed the toddmeng-db/namespace-fallback branch from 6d5ff2c to 83b9ba3 Compare June 3, 2025 06:49

toddmeng-db requested a review from CurtHagenlocher June 3, 2025 06:49

toddmeng-db requested a review from eric-wang-1990 June 3, 2025 06:49

revert escape backtick

e3dd382

CurtHagenlocher approved these changes Jun 3, 2025

View reviewed changes

CurtHagenlocher merged commit db253b1 into apache:main Jun 3, 2025
8 of 10 checks passed

serramatutu mentioned this pull request Aug 12, 2025

Sync upstream dbt-labs/arrow-adbc#52

Closed

feat(csharp/src/Drivers/Databricks): Default catalogs edge cases #2896

feat(csharp/src/Drivers/Databricks): Default catalogs edge cases #2896

Uh oh!

Conversation

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

Uh oh!

Default schema fallback

Testing

Default catalog legacy compatibility

Testing

Uh oh!

toddmeng-db May 30, 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!

Choose a reason for hiding this comment

Uh oh!

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

Uh oh!

Choose a reason for hiding this comment

Uh oh!

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

Choose a reason for hiding this comment

Uh oh!

CurtHagenlocher Jun 3, 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!

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 3, 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!

Choose a reason for hiding this comment

Uh oh!

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

Choose a reason for hiding this comment

Uh oh!

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

Uh oh!

Uh oh!

Uh oh!

Reviewers

toddmeng-db commented May 30, 2025 •

edited

Loading

toddmeng-db May 30, 2025 •

edited

Loading

toddmeng-db Jun 1, 2025 •

edited

Loading

toddmeng-db Jun 1, 2025 •

edited

Loading

CurtHagenlocher Jun 3, 2025 •

edited

Loading

toddmeng-db Jun 3, 2025 •

edited

Loading

toddmeng-db Jun 3, 2025 •

edited

Loading

toddmeng-db commented Jun 3, 2025 •

edited

Loading