Skip to content

Commit 727db4c

Browse files
authored
Fix getTables(types=[]) returning rows in Thrift mode (#1451)
## Summary - Per JDBC spec, `DatabaseMetaData.getTables(_, _, _, new String[0])` (empty `types` array) means "no types selected" and should return zero rows. The Thrift path was leaking rows because the post-server filter at `MetadataResultSetBuilder.getTablesResult` is guarded by `tableTypes.length > 0`, which short-circuits the filter for empty arrays. - Symptom: legacy `hive_metastore` objects have `TABLE_TYPE=""` from the Thrift server. The client normalizes `""` → `"TABLE"`, and with the broken filter guard those rows survived even for `types=[]`. - Fix: mirror SEA's existing guard (`DatabricksMetadataQueryClient.java:128-131`) in the Thrift `listTables` — short-circuit to an empty result set at the top of the method when `tableTypes` is non-null and length 0. The server is never queried. ## Test plan - [x] Added `testListTablesWithEmptyTypesReturnsEmptyWithoutServerCall` — asserts zero rows AND that `thriftAccessor.getThriftResponse` is never invoked - [x] Existing `testListTables` (non-empty types) still passes — 46/46 in `DatabricksThriftServiceClientTest` - [x] Verified end-to-end against a real workspace: before fix, `getTables(null, null, null, [])` on Thrift returned 2 `hive_metastore.default.trim_repro*` rows while SEA returned 0; after fix, both return 0 This pull request and its description were written by Isaac. Signed-off-by: Sreekanth Vadigi <sreekanth.vadigi@databricks.com>
1 parent 722d80a commit 727db4c

3 files changed

Lines changed: 22 additions & 0 deletions

File tree

NEXT_CHANGELOG.md

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,7 @@
2020
- When a Statement is re-executed, the previous server-side operation is now explicitly closed before starting the new execution, preventing orphaned server-side operations when Statements are reused.
2121

2222
### Fixed
23+
- Fixed `DatabaseMetaData.getTables()` in Thrift mode returning rows when called with an empty `types` array. Per JDBC spec, empty types means "no types selected" and now correctly returns zero rows (matching SEA mode).
2324
- Fixed `?` characters inside SQL comments, string literals, and quoted identifiers being incorrectly counted as parameter placeholders when `supportManyParameters=1`. `SQLInterpolator` now uses `SqlCommentParser` to locate only real placeholders. Fixes #1331.
2425
- Fixed `MetadataOperationTimeout` not being applied when metadata operations use SHOW commands. Operations like `getTables`, `getSchemas`, and `getColumns` now respect the `MetadataOperationTimeout` connection property instead of hanging indefinitely with no timeout.
2526
- Reclassify transient server errors to standard SQL states (08S01, 40001) across all Thrift error sites. This ensures UC unavailability and concurrent modification errors surface consistently for better retry handling. Note: Dashboards and branching logic keyed on legacy XXUCC or 42000 must be updated.

src/main/java/com/databricks/jdbc/dbclient/impl/thrift/DatabricksThriftServiceClient.java

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -476,6 +476,11 @@ public DatabricksResultSet listTables(
476476
session.toString(), catalog, schemaNamePattern, tableNamePattern);
477477
LOGGER.debug(context);
478478

479+
// Per JDBC spec: null types = return all types; empty array = return nothing
480+
if (tableTypes != null && tableTypes.length == 0) {
481+
return metadataResultSetBuilder.getTablesResult(catalog, tableTypes, new ArrayList<>());
482+
}
483+
479484
if (!metadataResultSetBuilder.shouldAllowCatalogAccess(catalog, null, session)) {
480485
return metadataResultSetBuilder.getTablesResult(catalog, tableTypes, new ArrayList<>());
481486
}

src/test/java/com/databricks/jdbc/dbclient/impl/thrift/DatabricksThriftServiceClientTest.java

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,7 @@
1313
import static org.mockito.ArgumentMatchers.anyLong;
1414
import static org.mockito.ArgumentMatchers.eq;
1515
import static org.mockito.Mockito.lenient;
16+
import static org.mockito.Mockito.never;
1617
import static org.mockito.Mockito.verify;
1718
import static org.mockito.Mockito.when;
1819

@@ -590,6 +591,21 @@ void testListTables() throws SQLException {
590591
assertEquals(resultSet.getStatementStatus().getState(), StatementState.SUCCEEDED);
591592
}
592593

594+
@Test
595+
void testListTablesWithEmptyTypesReturnsEmptyWithoutServerCall() throws SQLException {
596+
// Per JDBC spec: empty types array means "no types selected" → return no rows.
597+
// The driver must short-circuit and NOT send the Thrift request to the server.
598+
DatabricksThriftServiceClient client =
599+
new DatabricksThriftServiceClient(thriftAccessor, connectionContext);
600+
601+
DatabricksResultSet resultSet =
602+
client.listTables(session, TEST_CATALOG, TEST_SCHEMA, TEST_TABLE, new String[0]);
603+
604+
assertEquals(StatementState.SUCCEEDED, resultSet.getStatementStatus().getState());
605+
assertFalse(resultSet.next(), "Empty types array must yield zero rows");
606+
verify(thriftAccessor, never()).getThriftResponse(any());
607+
}
608+
593609
@Test
594610
void testListColumns() throws SQLException {
595611
DatabricksThriftServiceClient client =

0 commit comments

Comments
 (0)