feat(csharp/src/Drivers/Databricks): Default catalog + schema support#2806
Conversation
ac168eb to
75256e8
Compare
75256e8 to
1f7a279
Compare
| Properties.TryGetValue(DatabricksParameters.DefaultCatalog, out defaultCatalog); | ||
| Properties.TryGetValue(DatabricksParameters.DefaultSchema, out defaultSchema); | ||
|
|
||
| if (!string.IsNullOrEmpty(defaultCatalog)) |
There was a problem hiding this comment.
use string.IsNullOrWhitespace
CurtHagenlocher
left a comment
There was a problem hiding this comment.
Thanks! New properties should be described in readme.md. One thing that I find non-obvious is the relationship between these properties and the standard ADBC properties adbc.connection.catalog and adbc.connection.db_schema. It looks like those properties aren't supported by the driver at all, whereas these Databricks-specific properties are only supported when creating a new session. Can the current catalog or schema be changed after the session is established? And at that point the initial values don't matter any more?
I think some clarity is required here in the docs even if the code doesn't change.
| using Thrift.Transport; | ||
| using Xunit; | ||
| using Xunit.Abstractions; | ||
| using System.Reflection; |
There was a problem hiding this comment.
nit: namespace ordering
| Assert.NotNull(connection); | ||
| Assert.IsType<DatabricksConnection>(connection); | ||
|
|
||
| var defaultNamespaceProperty = typeof(DatabricksConnection).GetProperty("DefaultNamespace", BindingFlags.NonPublic | BindingFlags.Instance | BindingFlags.Public); |
There was a problem hiding this comment.
With InternalsVisibleToAttribute and an internal property, it shouldn't be necessary to use Reflection to get at this property value.
|
I think my expectation as an ADBC user would be that if I set |
Makes sense - sorry I didn't even realize that these parameters were already there 😅 |
This initial catalog/schema should only be used when opening session, in powerbi when user signed in and pick the initial catalog/schema there is no way to change that unless user make brand new connection. |
44aee95 to
702f174
Compare
702f174 to
1a52d88
Compare
7c25e5c to
adda68f
Compare
Notwithstanding that (at least in principle) this driver should be useful outside of a narrow Power BI context, Power BI does make use of the ability to set the current catalog; it's a prerequisite for being able to use |
Allows users to specify an optional default catalog and default schema that will take affect throughout the session.
Includes tests that verifies basic behavior. To test, include in the DATABRICKS_TEST_CONFIG_FILE environment variable:
NOTE: using Namespace to set catalogs is only in DBR 8.4+