Skip to content

[PECOBLR-1575] cross catalog metadata operations in comparator#1229

Open
sreekanth-db wants to merge 1 commit intodatabricks:jdbc-comparator-updatedfrom
sreekanth-db:comparator-cross-catalog-operations
Open

[PECOBLR-1575] cross catalog metadata operations in comparator#1229
sreekanth-db wants to merge 1 commit intodatabricks:jdbc-comparator-updatedfrom
sreekanth-db:comparator-cross-catalog-operations

Conversation

@sreekanth-db
Copy link
Collaborator

Description

Added cross-catalog metadata tests to the JDBC comparator and fixed bug where all old(2.7.6) driver comparisons were failing due to JUnit 5's autoCloseArguments default.

Changes:

  • Added null-catalog test args for all DatabaseMetaData methods that accept a catalog parameter (e.g., getTables(null, ...), getSchemas(null, ...), getColumns(null, ...)), adding 42 new parameterized test invocations to validate cross-catalog metadata behavior.
  • Fixed autoCloseArguments issue: JUnit 5.8+ defaults to calling .close() on any AutoCloseable argument (Connection, ResultSet) after each parameterized test invocation. Since the comparator reuses the same connection objects across 4220+ test invocations, JUnit was silently closing them after the first invocation — causing 1917 test failures. Set autoCloseArguments = false on all 5 @ParameterizedTest annotations.
  • Wrapped ResultSet comparison in ResultSetComparator with try-catch to handle lazy ResultSet errors (e.g., old driver's getPrimaryKeys(null, ...) returns a ResultSet that throws server errors during iteration).

Testing

  • Ran the full comparator test suite: 4220 tests, 0 failures, 0 errors (previously 1917 failures).
  • Verified the comparison report has zero "closed" errors (previously 1716).

NO_CHANGELOG=true

Signed-off-by: Sreekanth Vadigi <sreekanth.vadigi@databricks.com>
@sreekanth-db sreekanth-db changed the title cross catalog metadata operations in comparator [PECOBLR-1575] cross catalog metadata operations in comparator Feb 24, 2026
new String[] {"main", "tpcds_sf100_delta", "%", "%"});

// Cross-catalog tests: null catalog (match all catalogs)
putInMapForKey(
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lets also add some more combinations, like tableType being TABLE, VIEW apart from null

Also, schema can be pattern, schema can be null

also, table can be null

putInMapForKey(functionToArgsMap, Map.entry("getSchemas", 2), new String[] {null, "tpcds_%"});
putInMapForKey(
functionToArgsMap,
Map.entry("getColumns", 4),
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

similarly more combination here as well

new String[] {null, "tpcds_sf100_delta", "catalog_sales"});
putInMapForKey(
functionToArgsMap,
Map.entry("getFunctions", 3),
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

here also

new String[] {null, "tpcds_sf100_delta", "aggregate"});
putInMapForKey(
functionToArgsMap,
Map.entry("getFunctionColumns", 4),
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

do we support this?

new String[] {null, "tpcds_sf100_delta", "aggregate", "%"});
putInMapForKey(
functionToArgsMap,
Map.entry("getProcedures", 3),
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

here also more combination on schema being null, pattern

new String[] {null, "tpcds_sf100_delta", "%"});
putInMapForKey(
functionToArgsMap,
Map.entry("getProcedureColumns", 4),
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

here also

new String[] {null, "tpcds_sf100_delta", "%", "%"});
putInMapForKey(
functionToArgsMap,
Map.entry("getPrimaryKeys", 3),
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

do we support null catalog here? Or just to compare results for null

new String[] {null, "oss_jdbc_tests", "test_result_set_types"});
putInMapForKey(
functionToArgsMap,
Map.entry("getImportedKeys", 3),
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

same comment

Map.entry("getImportedKeys", 3),
new String[] {null, "tpcds_sf100_delta", "catalog_sales"});
putInMapForKey(
functionToArgsMap,
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I believe this is empty and null anyways is not supported, what is goal, you can add a comment on what you are testing here

new String[] {null, "tpcds_sf100_delta", "catalog_sales"});
putInMapForKey(
functionToArgsMap,
Map.entry("getCrossReference", 6),
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

same

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.

2 participants