[SEA] Fix list_schemas to return empty rowset instead of throwing on non-matching catalog patterns (PECO-3017)#1439
Closed
eric-wang-1990 wants to merge 1 commit into
Closed
Conversation
…non-matching catalog patterns (PECO-3017) Previously, calling getSchemas() with a wildcard catalog pattern (e.g., "%", "my_%") in SEA mode generated invalid SQL (SHOW SCHEMAS IN `%`) which caused the server to throw a DatabricksException. Similarly, literal nonexistent catalogs could throw instead of returning an empty result set per the JDBC spec. Fix: - Treat "%" catalog as null (matches all) → generates SHOW SCHEMAS IN ALL CATALOGS - Detect JDBC catalog patterns (unescaped % or _) and expand client-side by listing catalogs, filtering by the pattern, then fetching schemas per match - Catch object-not-found errors for literal nonexistent catalogs and return empty New helpers added to WildcardUtil: - isJdbcPattern(): detects unescaped % or _ wildcards (respects \ escaping) - isMatchAllCatalogPattern(): true for null or "%" (matches any catalog) New static method DatabricksMetadataQueryClient.jdbcPatternMatches() for case-insensitive JDBC wildcard matching (%, _, \-escape support). 29 new unit tests added across WildcardUtilTest and DatabricksMetadataQueryClientTest. Signed-off-by: Eric Wang <e.wang@databricks.com>
Contributor
There was a problem hiding this comment.
Pull request overview
This PR fixes DatabaseMetaData.getSchemas() behavior in SEA mode when the catalog argument is a JDBC search pattern (e.g., %, main%, m_in) by avoiding invalid identifier SQL and returning empty rowsets (vs throwing) for non-matching / nonexistent catalogs, per JDBC expectations.
Changes:
- Treat catalog pattern
%as “match all catalogs” and avoid generatingSHOW SCHEMAS IN \%``. - Add JDBC-pattern detection and client-side expansion for partial wildcard catalog patterns by listing catalogs, filtering via JDBC pattern matching, then fetching schemas per matched catalog.
- Add unit tests for JDBC pattern detection/matching and
listSchemas()wildcard/nonexistent-catalog behaviors; document the behavior inNEXT_CHANGELOG.md.
Reviewed changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
src/main/java/com/databricks/jdbc/dbclient/impl/sqlexec/DatabricksMetadataQueryClient.java |
Adds match-all handling for %, client-side catalog-pattern expansion, and JDBC pattern matching helper. |
src/main/java/com/databricks/jdbc/common/util/WildcardUtil.java |
Adds helpers to detect JDBC patterns and match-all catalog patterns. |
src/test/java/com/databricks/jdbc/dbclient/impl/sqlexec/DatabricksMetadataQueryClientTest.java |
Adds tests for JDBC pattern matching and new listSchemas() wildcard/nonexistent behaviors. |
src/test/java/com/databricks/jdbc/common/util/WildcardUtilTest.java |
Adds tests for isJdbcPattern and isMatchAllCatalogPattern. |
NEXT_CHANGELOG.md |
Notes the SEA-mode getSchemas() wildcard/nonexistent-catalog fix. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
+681
to
+685
| c -> { | ||
| List<List<Object>> rows = new ArrayList<>(); | ||
| try (ResultSet catalogSchemas = | ||
| session.getDatabricksMetadataClient().listSchemas(session, c, schemaNamePattern)) { | ||
| while (catalogSchemas.next()) { |
Comment on lines
+120
to
+129
| /** | ||
| * Returns true if the JDBC catalog pattern matches all catalogs — that is, it is {@code null}, | ||
| * {@code %}, or a pattern that would match any string. | ||
| * | ||
| * @param catalog the catalog pattern to check | ||
| * @return true if the pattern matches all catalogs | ||
| */ | ||
| public static boolean isMatchAllCatalogPattern(String catalog) { | ||
| return catalog == null || "%".equals(catalog); | ||
| } |
Comment on lines
+1695
to
+1706
| IDatabricksSession mockSessionLocal = mock(IDatabricksSession.class); | ||
| IDatabricksMetadataClient mockMetadataClient = mock(IDatabricksMetadataClient.class); | ||
| when(mockSessionLocal.getComputeResource()).thenReturn(mockedComputeResource); | ||
| when(mockSessionLocal.getDatabricksMetadataClient()).thenReturn(mockMetadataClient); | ||
| when(mockSessionLocal.getConnectionContext()).thenReturn(mockContext); | ||
|
|
||
| // The catalog list: "main" matches "main%", "other" does not | ||
| DatabricksResultSet catalogResultSet = mock(DatabricksResultSet.class); | ||
| when(catalogResultSet.next()).thenReturn(true, true, false); | ||
| when(catalogResultSet.getString(1)).thenReturn("main", "other"); | ||
| when(mockMetadataClient.listCatalogs(mockSessionLocal)).thenReturn(catalogResultSet); | ||
|
|
Contributor
Author
|
Closing — this was created in the wrong repo (JDBC instead of ADBC). The fix belongs in the ADBC C# driver. |
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
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
Problem (PECO-3017): In SEA mode, calling
getSchemas()with a wildcard catalog pattern (e.g.,%,my_%,%catalog%) causes the driver to generate invalid SQL (SHOW SCHEMAS IN \%`) which throws aDatabricksException`. Similarly, passing a literal nonexistent catalog name should return an empty result set per the JDBC spec, but could throw in some edge cases.Root cause: The
listSchemasmethod inDatabricksMetadataQueryClientpassed the raw catalog parameter (after stripping JDBC escapes) directly into a backtick-quoted SQL identifier. When the catalog is a JDBC pattern (e.g.,%), this generates syntactically invalid SQL or a catalog-not-found error from the server.Fix approach
%/ match-all patterns: Treat catalog%the same asnull— generatesSHOW SCHEMAS IN ALL CATALOGSinstead ofSHOW SCHEMAS IN \%`(per JDBC spec,%` means "match any catalog").main%,%log%,m_in): Detect JDBC patterns client-side using newWildcardUtil.isJdbcPattern(), then expand by listing all catalogs, filtering by the pattern withjdbcPatternMatches(), and fetching schemas per matching catalog.isObjectNotFoundException()— the server throws 42704 or aNO_SUCH_CATALOG_EXCEPTIONmessage, and the driver returns an empty result set.New helpers
WildcardUtil.isJdbcPattern(String): detects unescaped%or_wildcards, respecting\-escape sequences.WildcardUtil.isMatchAllCatalogPattern(String): true fornullor%.DatabricksMetadataQueryClient.jdbcPatternMatches(String, String): case-insensitive JDBC wildcard matching with%,_, and\-escape support (package-visible for testing).Test plan
WildcardUtilTest: 12 cases forisJdbcPattern, 6 cases forisMatchAllCatalogPatternDatabricksMetadataQueryClientTest:jdbcPatternMatchestestListSchemasWithPercentCatalog_treatsAsMatchAll: verifies%→SHOW SCHEMAS IN ALL CATALOGStestListSchemasWithPartialCatalogPattern_expandsClientSide: verifiesmain%expands client-sidetestListSchemasWithUnderscoreCatalogPattern_expandsClientSide: verifiesm_inunderscore handlingtestListSchemasWithNonexistentLiteralCatalog_returnsEmpty: verifies 42704 → empty resulttestListSchemasNonexistentCatalog_nullSqlState_returnsEmpty: verifies null SQL state handledtestListSchemasWithPatternMatchingNoCatalogs_returnsEmpty: verifies no-match → empty resultlistSchemastests continue to pass (no regression)Reference