fix(csharp): treat catalog as a literal name on the Thrift metadata path (#525)#574
Open
eric-wang-1990 wants to merge 1 commit into
Open
fix(csharp): treat catalog as a literal name on the Thrift metadata path (#525)#574eric-wang-1990 wants to merge 1 commit into
eric-wang-1990 wants to merge 1 commit into
Conversation
…ath (#525) On the Thrift metadata path (GetSchemas/GetTables/GetColumns) the catalog argument was sent to the server's GetSchemas/GetTables/GetColumns RPC as a SQL-LIKE pattern, so `_`/`%` acted as wildcards and over-matched — e.g. catalog "comparator_tests" also matched "comparator-tests". Per the JDBC/ODBC contract (and the SEA path), catalog is an exact-name identifier, not a pattern. DatabricksStatement now normalizes the catalog before the base RPC call: - a bare match-all wildcard ("%"/"*") becomes null ("all catalogs"), mirroring the SEA path (StatementExecutionStatement.EffectiveCatalog) / PR #536; - any other catalog name has its "_"/"%" escaped so the server matches it literally (the Thrift server honors the "\_"/"\%" escape — verified against a live warehouse). When escape_pattern_wildcards is already enabled the base class escapes catalog itself, so we skip here to avoid double-escaping. Adds a unit test covering the transform (bare wildcard -> null, escaping with the flag off, and no double-escape with the flag on). Co-authored-by: Isaac
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Makes the Thrift metadata path treat the
catalogargument ofGetSchemas/GetTables/GetColumnsas an exact-name identifier, not a SQL-LIKE pattern — matching the SEA path and PR #536.Today the catalog is sent to the server's Thrift
GetSchemas/GetTables/GetColumnsRPC as a pattern, so_/%act as wildcards and over-match. Per the JDBC/ODBC contract (catalog is an exact name; only schema/table/column are patterns) and for Thrift↔SEA parity, catalog should match literally.Change
DatabricksStatementnormalizes the catalog after SPARK handling and before the base RPC call (NormalizeCatalogForThrift):%or*, like null) →null("all catalogs"), mirroring the SEA path (StatementExecutionStatement.EffectiveCatalog) / fix(csharp): fix */% wildcard semantics and TABLE_CAT identifier echo diverge between Thrift and SEA (#525) #536._/%so the server matches it literally. The Thrift server honors the\_/\%escape.escape_pattern_wildcardsis already enabled, the baseHiveServer2Statementescapes catalog itself — so we return it unescaped here to avoid double-escaping.The
AdbcDrivers.HiveServer2submodule is not modified; the change lives entirely in the Databricks-ownedDatabricksStatementoverrides.Evidence (live Thrift warehouse)
Verified against a live Thrift SQL warehouse via
GetSchemas:None/%/*comparator_tests(unescaped_)comparator\_tests(escaped)So escaping produces the correct literal match, and
%/*still return all catalogs.Test plan
DatabricksStatementUnitTests.NormalizeCatalogForThrift_NormalizesAndEscapesCatalog— 11 cases:%/*/null→null, plain name unchanged,_/%escaped (flag off), no double-escape (flag on).Notes
%/*→null). Suggest merging after fix(csharp): fix */% wildcard semantics and TABLE_CAT identifier echo diverge between Thrift and SEA (#525) #536 so the pair reads together.catalog="%"now returns the session default catalog's schemas (was empty), intentionally matching SEA/fix(csharp): fix */% wildcard semantics and TABLE_CAT identifier echo diverge between Thrift and SEA (#525) #536.This pull request and its description were written by Isaac.