Skip to content

Commit 7174d62

Browse files
authored
fix getTables result ordering in SEA (#1127)
## Description <!-- Provide a brief summary of the changes made and the issue they aim to address.--> Add sorting to getTables() result in SEA mode to match Thrift mode behavior. Results are now sorted by TABLE_TYPE, TABLE_CAT, TABLE_SCHEM, TABLE_NAME (per JDBC spec) Why sorting will work in SEA mode: - Metadata queries use JSON_ARRAY format which eagerly loads ALL data upfront before the ResultSet is returned - The getRows() method extracts all rows into a List<List<Object>> before filtering - Sorting can be applied to this in-memory list just like Thrift mode ## Testing Unit tests Manual verification Compared results for `getTables("main", "tpcds_sf100_delta", "%", null);` in SEA/Thrift mode. ## Additional Notes to the Reviewer <!-- Share any additional context or insights that may help the reviewer understand the changes better. This could include challenges faced, limitations, or compromises made during the development process. Also, mention any areas of the code that you would like the reviewer to focus on specifically. --> NO_CHANGELOG=true
1 parent dee4266 commit 7174d62

2 files changed

Lines changed: 92 additions & 0 deletions

File tree

src/main/java/com/databricks/jdbc/dbclient/impl/common/MetadataResultSetBuilder.java

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -90,6 +90,14 @@ public DatabricksResultSet getTablesResult(DatabricksResultSet resultSet, String
9090
getRows(resultSet, TABLE_COLUMNS, defaultAdapter).stream()
9191
.filter(row -> allowedTableTypes.contains(row.get(3))) // Filtering based on table type
9292
.collect(Collectors.toList());
93+
94+
// Sort in order TABLE_TYPE, TABLE_CAT, TABLE_SCHEM, TABLE_NAME (matching Thrift mode)
95+
rows.sort(
96+
Comparator.comparing((List<Object> r) -> (String) r.get(3)) // TABLE_TYPE
97+
.thenComparing(r -> (String) r.get(0)) // TABLE_CAT
98+
.thenComparing(r -> (String) r.get(1)) // TABLE_SCHEM
99+
.thenComparing(r -> (String) r.get(2))); // TABLE_NAME
100+
93101
return buildResultSet(
94102
TABLE_COLUMNS,
95103
rows,

src/test/java/com/databricks/jdbc/dbclient/impl/common/MetadataResultSetBuilderTest.java

Lines changed: 84 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@
44
import static org.junit.jupiter.api.Assertions.*;
55
import static org.mockito.Mockito.mock;
66
import static org.mockito.Mockito.when;
7+
import static org.mockito.Mockito.withSettings;
78

89
import com.databricks.jdbc.api.impl.DatabricksResultSet;
910
import com.databricks.jdbc.api.internal.IDatabricksConnectionContext;
@@ -480,6 +481,89 @@ void testGetTablesResultWithNullTableType() throws SQLException {
480481
assertEquals(2, rowCount);
481482
}
482483

484+
@Test
485+
void testGetTablesResultSortingInSeaMode() throws SQLException {
486+
// Create mock DatabricksResultSet with rows in unsorted order
487+
DatabricksResultSet mockResultSet = mock(DatabricksResultSet.class, withSettings().lenient());
488+
java.sql.ResultSetMetaData mockMetaData = mock(java.sql.ResultSetMetaData.class);
489+
when(mockMetaData.getColumnCount()).thenReturn(TABLE_COLUMNS.size());
490+
when(mockResultSet.getMetaData()).thenReturn(mockMetaData);
491+
492+
// Return 4 rows in unsorted order, then false
493+
when(mockResultSet.next())
494+
.thenReturn(true)
495+
.thenReturn(true)
496+
.thenReturn(true)
497+
.thenReturn(true)
498+
.thenReturn(false);
499+
500+
// Stub all TABLE_COLUMNS to return null by default (some columns aren't in resultset)
501+
for (ResultColumn resultColumn : TABLE_COLUMNS) {
502+
when(mockResultSet.getObject(resultColumn.getResultSetColumnName())).thenReturn(null);
503+
}
504+
505+
// Set up rows in deliberately unsorted order to verify sorting works
506+
// Expected sort order: TABLE_TYPE, TABLE_CAT, TABLE_SCHEM, TABLE_NAME
507+
// Row data uses ResultColumn.getResultSetColumnName() values
508+
when(mockResultSet.getObject(CATALOG_COLUMN.getResultSetColumnName())) // TABLE_CAT
509+
.thenReturn("catalog_b")
510+
.thenReturn("catalog_a")
511+
.thenReturn("catalog_a")
512+
.thenReturn("catalog_a");
513+
when(mockResultSet.getObject(SCHEMA_COLUMN.getResultSetColumnName())) // TABLE_SCHEM
514+
.thenReturn("schema_z")
515+
.thenReturn("schema_a")
516+
.thenReturn("schema_b")
517+
.thenReturn("schema_a");
518+
when(mockResultSet.getObject(TABLE_NAME_COLUMN.getResultSetColumnName())) // TABLE_NAME
519+
.thenReturn("table_x")
520+
.thenReturn("table_b")
521+
.thenReturn("table_a")
522+
.thenReturn("table_a");
523+
when(mockResultSet.getObject(TABLE_TYPE_COLUMN.getResultSetColumnName())) // TABLE_TYPE
524+
.thenReturn("VIEW")
525+
.thenReturn("TABLE")
526+
.thenReturn("TABLE")
527+
.thenReturn("VIEW");
528+
529+
// Call SEA mode method with both TABLE and VIEW types
530+
String[] tableTypes = new String[] {"TABLE", "VIEW"};
531+
ResultSet resultSet = metadataResultSetBuilder.getTablesResult(mockResultSet, tableTypes);
532+
533+
// Verify sorting: TABLE_TYPE first, then TABLE_CAT, TABLE_SCHEM, TABLE_NAME
534+
// Expected order after sorting:
535+
// 1. TABLE, catalog_a, schema_a, table_b
536+
// 2. TABLE, catalog_a, schema_b, table_a
537+
// 3. VIEW, catalog_a, schema_a, table_a
538+
// 4. VIEW, catalog_b, schema_z, table_x
539+
540+
assertTrue(resultSet.next());
541+
assertEquals("TABLE", resultSet.getString("TABLE_TYPE"));
542+
assertEquals("catalog_a", resultSet.getString("TABLE_CAT"));
543+
assertEquals("schema_a", resultSet.getString("TABLE_SCHEM"));
544+
assertEquals("table_b", resultSet.getString("TABLE_NAME"));
545+
546+
assertTrue(resultSet.next());
547+
assertEquals("TABLE", resultSet.getString("TABLE_TYPE"));
548+
assertEquals("catalog_a", resultSet.getString("TABLE_CAT"));
549+
assertEquals("schema_b", resultSet.getString("TABLE_SCHEM"));
550+
assertEquals("table_a", resultSet.getString("TABLE_NAME"));
551+
552+
assertTrue(resultSet.next());
553+
assertEquals("VIEW", resultSet.getString("TABLE_TYPE"));
554+
assertEquals("catalog_a", resultSet.getString("TABLE_CAT"));
555+
assertEquals("schema_a", resultSet.getString("TABLE_SCHEM"));
556+
assertEquals("table_a", resultSet.getString("TABLE_NAME"));
557+
558+
assertTrue(resultSet.next());
559+
assertEquals("VIEW", resultSet.getString("TABLE_TYPE"));
560+
assertEquals("catalog_b", resultSet.getString("TABLE_CAT"));
561+
assertEquals("schema_z", resultSet.getString("TABLE_SCHEM"));
562+
assertEquals("table_x", resultSet.getString("TABLE_NAME"));
563+
564+
assertFalse(resultSet.next());
565+
}
566+
483567
@Test
484568
void testComplexTypesReturnVarcharWhenSupportDisabled() throws SQLException {
485569
when(connectionContext.isComplexDatatypeSupportEnabled()).thenReturn(false);

0 commit comments

Comments
 (0)