Added support for enableMultipleCatalogSupport param #1025
Conversation
src/main/java/com/databricks/jdbc/dbclient/impl/sqlexec/DatabricksMetadataSdkClient.java
Show resolved
Hide resolved
src/main/java/com/databricks/jdbc/dbclient/impl/thrift/DatabricksThriftServiceClient.java
Outdated
Show resolved
Hide resolved
src/main/java/com/databricks/jdbc/dbclient/impl/thrift/DatabricksThriftServiceClient.java
Show resolved
Hide resolved
…ipleCatalogSupport is disabled. Same behaviour as thrift
| String benchfoodHost = getDatabricksBenchfoodHost(); | ||
| if (benchfoodHost != null && !benchfoodHost.isEmpty()) { | ||
| String fullUrl = | ||
| benchfoodHost.startsWith("http") ? benchfoodHost : "https://" + benchfoodHost; |
There was a problem hiding this comment.
nit : Can we remove being too specific to benchfood in the naming across this file? (similar to dogfood in other files)
There was a problem hiding this comment.
Made the change. Thanks
| List<List<Object>> singleCatalogRows = new ArrayList<>(); | ||
| List<Object> catalogRow = new ArrayList<>(); | ||
| catalogRow.add(currentCatalog); | ||
| singleCatalogRows.add(catalogRow); |
There was a problem hiding this comment.
nit : reuse line on 82
| ENABLE_METRIC_VIEW_METADATA("EnableMetricViewMetadata", "Enable metric view metadata", "0"); | ||
|
|
||
| ENABLE_METRIC_VIEW_METADATA("EnableMetricViewMetadata", "Enable metric view metadata", "0"), | ||
| ENABLE_MULTIPLE_CATALOG_SUPPORT( |
There was a problem hiding this comment.
Should we add this inDatabricksDriverPropertyUtil parameters too?
| } | ||
|
|
||
| private boolean isMultipleCatalogSupportDisabled() { | ||
| return sdkClient.getConnectionContext() != null |
There was a problem hiding this comment.
follow the google function placement in this file. See this for more info : https://docs.google.com/document/d/1Ov6nGADV0TYI5DrbdALlAmL-SZZ_4AowGP1x1gMgJwE/edit?tab=t.0#heading=h.93vjlqf53xh7
| private String autoFillCatalog(String catalog, IDatabricksSession session) throws SQLException { | ||
| if (isMultipleCatalogSupportDisabled()) { | ||
| String currentCatalog = session.getCurrentCatalog(); | ||
| return (currentCatalog != null && !currentCatalog.isEmpty()) ? currentCatalog : ""; |
There was a problem hiding this comment.
any reason this isn't a util?
There was a problem hiding this comment.
The change is very specific to this file. It won't be used for other files so I've just kept it here
| if (isMultipleCatalogSupportDisabled()) { | ||
| String currentCatalog = session.getCurrentCatalog(); | ||
| if (currentCatalog == null || currentCatalog.isEmpty()) { | ||
| currentCatalog = ""; |
There was a problem hiding this comment.
why check for empty if you have to assign an empty anyway
There was a problem hiding this comment.
Updated. Thanks
3 participants
Description
Added support for the connection param of enableMultipleCatalogSupport.
The default value is kept to 1.
The behaviour remains the same when the param is enabled. When the param
is disabled, the behaviour is as follows:
To maintain the current catalog, we make the query to get the current
catalog everytime when this param is disabled.
Testing
Additional Notes to the Reviewer
PECOBLR-938