Skip to content

[SEA] Fix GetCatalogsAsync to handle ADBC catalog patterns correctly (PECO-3018)#441

Open
eric-wang-1990 wants to merge 1 commit into
mainfrom
fix/peco-3018-sea-catalog-pattern
Open

[SEA] Fix GetCatalogsAsync to handle ADBC catalog patterns correctly (PECO-3018)#441
eric-wang-1990 wants to merge 1 commit into
mainfrom
fix/peco-3018-sea-catalog-pattern

Conversation

@eric-wang-1990
Copy link
Copy Markdown
Collaborator

Summary

Fixes the D8 bug case in IGetObjectsDataProvider.GetCatalogsAsync on the SEA (Statement Execution API) path.

Root cause: GetCatalogsAsync passed the ADBC catalogPattern directly to ShowCatalogsCommand, which converted it via ConvertPattern() and appended it as a SQL LIKE clause. This was wrong because:

  • "comparator\_tests"SHOW CATALOGS LIKE 'comparator_tests'
    SQL _ is a wildcard, so the escaped literal underscore was lost. The catalog comparator_tests would be found but so would comparatorXtests (too broad).
  • "%"SHOW CATALOGS LIKE '*' — glob * is not valid SQL LIKE syntax.
  • ""SHOW CATALOGS LIKE '' — happened to return nothing, but by accident.

Fix: Mirror the Thrift path (HiveServer2Connection.GetCatalogsAsync) which fetches all catalogs via a bare SHOW CATALOGS request and filters results client-side using a pattern-to-regex conversion:

catalogPattern behavior
null SHOW CATALOGS, no filter — return all (JDBC spec: null = any)
"" Return empty list immediately (no server call needed)
any other SHOW CATALOGS, filter results with CatalogPatternToRegex(pattern)

New helpers (analogous to PatternToRegEx in the Thrift layer):

  • ContainsUnescapedWildcard: detects % or _ not preceded by backslash
  • CatalogPatternToRegex: converts ADBC/SQL pattern to anchored case-insensitive Regex
    • %.*, _., \__, \%%, \\\

Also includes minor E2E test fixes to skip Thrift-specific tests when running against SEA-only endpoints.

Note on hiveserver2: This fix is entirely within the Databricks-specific layer (csharp/src/StatementExecution/). No changes were made to csharp/hiveserver2/ (the upstream submodule).

Test plan

  • 24 new unit tests in GetCatalogsAsyncTests.cs covering all pattern cases
  • ContainsUnescapedWildcard: 10 theory cases
  • CatalogPatternToRegex: wildcard expansion, escaped literals, case-insensitivity, regex-char escaping
  • D8 bug-report scenarios: comparator\_tests, comparator_tests, %, compar%, "", nonexistent
  • All unit tests pass: dotnet test --filter "FullyQualifiedName~StatementExecution" → 174 passed, 29 skipped (E2E only)
  • Build succeeds: dotnet build AdbcDrivers.Databricks.csproj

Fixes #PECO-3018

…(PECO-3018)

Root cause (D8 and related cases): GetCatalogsAsync passed the ADBC
catalogPattern directly to ShowCatalogsCommand which appended it as a
SQL LIKE clause via ConvertPattern(). This was wrong because:

  - "comparator\_tests" → SHOW CATALOGS LIKE 'comparator_tests'
    SQL _ is a wildcard, so the literal underscore was lost.
    The literal catalog "comparator_tests" would technically still be
    found, but so would "comparatorXtests" and any other single-char
    variation (too broad).
  - "%" → SHOW CATALOGS LIKE '*' — glob '*' is not valid SQL LIKE
    syntax in all contexts.
  - "" → SHOW CATALOGS LIKE '' — returns nothing; correct by accident
    but handled explicitly now.

Fix: mirror the Thrift path (HiveServer2Connection.GetCatalogsAsync)
which fetches ALL catalogs and filters client-side using PatternToRegEx.

  - null   → SHOW CATALOGS, no filter (return all)
  - ""     → return empty list immediately (no server call)
  - other  → SHOW CATALOGS, filter results with CatalogPatternToRegex

Two new internal static helpers:
  - ContainsUnescapedWildcard: detects % or _ not preceded by backslash
  - CatalogPatternToRegex: converts ADBC/SQL pattern to a case-insensitive
    anchored Regex: % → .*, _ → ., \_ → _, \% → %, \\ → \

Also skip Thrift-specific E2E tests when running against SEA-only endpoints
(CloseOperationE2ETest and SeaMetadataE2ETests).

Signed-off-by: Eric Wang <e.wang@databricks.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant