Skip to content

Commit 4a0d20d

Browse files
authored
Force Thrift mode when metadata params require native RPCs (#1416)
## Summary When a user sets `UseQueryForMetadata=0` or `TreatMetadataCatalogNameAsPattern=1` without explicitly setting `UseThriftClient`, the driver now forces Thrift mode. These params require native Thrift RPCs and don't work with SEA. If the user explicitly sets `UseThriftClient=0` (wants SEA), that takes priority — the metadata params will be silently ineffective, but the user's explicit protocol choice is honoured. ## Decision Matrix | UseThriftClient | UseQueryForMetadata | TreatCatalogAsPattern | Result | Reason | |---|---|---|---|---| | `1` (explicit) | any | any | **Thrift** | User explicit | | `0` (explicit) | `0` | `1` | **SEA** | User explicit SEA, metadata params ignored | | not set | `0` (explicit) | not set | **Thrift** | Needs native RPCs | | not set | not set | `1` (explicit) | **Thrift** | Needs native RPCs | | not set | `0` | `1` | **Thrift** | Needs native RPCs | | not set | `1` (explicit) | not set | SAFE flag decides | SHOW commands work in both | | not set | not set | not set | SAFE flag decides | Default | ## Test plan - [x] 13 new tests covering all combinations in DatabricksConnectionContextTest - [x] Full core suite: 3293 tests, 0 failures NO_CHANGELOG=true This pull request was AI-assisted by Isaac. --------- Signed-off-by: Gopal Lal <gopal.lal@databricks.com>
1 parent 77184e7 commit 4a0d20d

2 files changed

Lines changed: 191 additions & 1 deletion

File tree

src/main/java/com/databricks/jdbc/api/impl/DatabricksConnectionContext.java

Lines changed: 33 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -456,10 +456,42 @@ public DatabricksClientType getClientTypeFromContext() {
456456
if (useThriftClient.equals("1")) {
457457
return DatabricksClientType.THRIFT;
458458
} else if (useThriftClient.equals("0")) {
459+
// Warn if user explicitly chose SEA but also set Thrift-only metadata params
460+
String explicitQueryForMetadata =
461+
getParameterIgnoreDefault(DatabricksJdbcUrlParams.USE_QUERY_FOR_METADATA);
462+
String explicitCatalogAsPattern =
463+
getParameterIgnoreDefault(
464+
DatabricksJdbcUrlParams.TREAT_METADATA_CATALOG_NAME_AS_PATTERN);
465+
if ((explicitQueryForMetadata != null && explicitQueryForMetadata.equals("0"))
466+
|| (explicitCatalogAsPattern != null && explicitCatalogAsPattern.equals("1"))) {
467+
LOGGER.warn(
468+
"UseThriftClient=0 (SEA) is set alongside Thrift-only metadata params "
469+
+ "(UseQueryForMetadata={}, TreatMetadataCatalogNameAsPattern={}). "
470+
+ "Honouring SEA — these metadata params will have no effect.",
471+
explicitQueryForMetadata,
472+
explicitCatalogAsPattern);
473+
}
459474
return DatabricksClientType.SEA;
460475
}
461476
}
462-
// Now, user has not provided a value, we will decide based on our checks
477+
// Now, user has not provided a value for UseThriftClient, we will decide based on our checks.
478+
// If user explicitly requires Thrift-native metadata behavior, stay on Thrift:
479+
// - UseQueryForMetadata=0: user wants native Thrift RPCs for metadata (not SHOW commands)
480+
// - TreatMetadataCatalogNameAsPattern=1: only works with native Thrift RPCs
481+
String explicitUseQueryForMetadata =
482+
getParameterIgnoreDefault(DatabricksJdbcUrlParams.USE_QUERY_FOR_METADATA);
483+
String explicitTreatCatalogAsPattern =
484+
getParameterIgnoreDefault(DatabricksJdbcUrlParams.TREAT_METADATA_CATALOG_NAME_AS_PATTERN);
485+
if ((explicitUseQueryForMetadata != null && explicitUseQueryForMetadata.equals("0"))
486+
|| (explicitTreatCatalogAsPattern != null && explicitTreatCatalogAsPattern.equals("1"))) {
487+
LOGGER.info(
488+
"Forcing Thrift client: user requires Thrift-native metadata behavior "
489+
+ "(UseQueryForMetadata={}, TreatMetadataCatalogNameAsPattern={})",
490+
explicitUseQueryForMetadata,
491+
explicitTreatCatalogAsPattern);
492+
return DatabricksClientType.THRIFT;
493+
}
494+
463495
// Check if circuit breaker is open due to recent 429 rate limit failures
464496
if (SeaCircuitBreakerManager.isCircuitOpen()) {
465497
long remainingMs = SeaCircuitBreakerManager.getTimeRemainingMs();

src/test/java/com/databricks/jdbc/api/impl/DatabricksConnectionContextTest.java

Lines changed: 158 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1487,4 +1487,162 @@ public void testUseQueryForMetadataExplicitFalseOnWarehouse() throws DatabricksS
14871487
TestConstants.VALID_URL_1 + ";UseQueryForMetadata=0", properties);
14881488
assertFalse(ctx.useQueryForMetadata());
14891489
}
1490+
1491+
// ---------------------------------------------------------------------------
1492+
// Client type selection with Thrift-native metadata params
1493+
// ---------------------------------------------------------------------------
1494+
1495+
@Test
1496+
public void testUseQueryForMetadata0_forcesThrift() throws DatabricksSQLException {
1497+
// UseQueryForMetadata=0 without UseThriftClient → forces Thrift
1498+
IDatabricksConnectionContext ctx =
1499+
DatabricksConnectionContext.parse(
1500+
TestConstants.VALID_URL_1 + ";UseQueryForMetadata=0", properties);
1501+
assertEquals(DatabricksClientType.THRIFT, ctx.getClientType());
1502+
}
1503+
1504+
@Test
1505+
public void testTreatCatalogAsPattern1_forcesThrift() throws DatabricksSQLException {
1506+
// TreatMetadataCatalogNameAsPattern=1 without UseThriftClient → forces Thrift
1507+
IDatabricksConnectionContext ctx =
1508+
DatabricksConnectionContext.parse(
1509+
TestConstants.VALID_URL_1 + ";TreatMetadataCatalogNameAsPattern=1", properties);
1510+
assertEquals(DatabricksClientType.THRIFT, ctx.getClientType());
1511+
}
1512+
1513+
@Test
1514+
public void testBothMetadataParams_forcesThrift() throws DatabricksSQLException {
1515+
// Both UseQueryForMetadata=0 and TreatMetadataCatalogNameAsPattern=1 → forces Thrift
1516+
IDatabricksConnectionContext ctx =
1517+
DatabricksConnectionContext.parse(
1518+
TestConstants.VALID_URL_1
1519+
+ ";UseQueryForMetadata=0;TreatMetadataCatalogNameAsPattern=1",
1520+
properties);
1521+
assertEquals(DatabricksClientType.THRIFT, ctx.getClientType());
1522+
}
1523+
1524+
@Test
1525+
public void testUseQueryForMetadata0_withExplicitSEA_honoursSEA() throws DatabricksSQLException {
1526+
// UseThriftClient=0 (explicit SEA) + UseQueryForMetadata=0 → SEA wins
1527+
// User explicitly chose SEA, so we honour that even though metadata param won't work
1528+
IDatabricksConnectionContext ctx =
1529+
DatabricksConnectionContext.parse(
1530+
TestConstants.VALID_URL_1 + ";UseThriftClient=0;UseQueryForMetadata=0", properties);
1531+
assertEquals(DatabricksClientType.SEA, ctx.getClientType());
1532+
}
1533+
1534+
@Test
1535+
public void testTreatCatalogAsPattern1_withExplicitSEA_honoursSEA()
1536+
throws DatabricksSQLException {
1537+
// UseThriftClient=0 (explicit SEA) + TreatMetadataCatalogNameAsPattern=1 → SEA wins
1538+
IDatabricksConnectionContext ctx =
1539+
DatabricksConnectionContext.parse(
1540+
TestConstants.VALID_URL_1 + ";UseThriftClient=0;TreatMetadataCatalogNameAsPattern=1",
1541+
properties);
1542+
assertEquals(DatabricksClientType.SEA, ctx.getClientType());
1543+
}
1544+
1545+
@Test
1546+
public void testUseQueryForMetadata0_withExplicitThrift_staysThrift()
1547+
throws DatabricksSQLException {
1548+
// UseThriftClient=1 (explicit Thrift) + UseQueryForMetadata=0 → Thrift
1549+
IDatabricksConnectionContext ctx =
1550+
DatabricksConnectionContext.parse(
1551+
TestConstants.VALID_URL_1 + ";UseThriftClient=1;UseQueryForMetadata=0", properties);
1552+
assertEquals(DatabricksClientType.THRIFT, ctx.getClientType());
1553+
}
1554+
1555+
@Test
1556+
public void testUseQueryForMetadata1_doesNotForceThrift() throws DatabricksSQLException {
1557+
// UseQueryForMetadata=1 (SHOW commands) with explicit SEA → should remain SEA
1558+
// Proves our new check doesn't trigger for UseQueryForMetadata=1
1559+
// VALID_URL_2 has UseThriftClient=0, so it's SEA
1560+
IDatabricksConnectionContext ctx =
1561+
DatabricksConnectionContext.parse(
1562+
TestConstants.VALID_URL_2 + ";UseQueryForMetadata=1", properties_with_pwd);
1563+
assertEquals(DatabricksClientType.SEA, ctx.getClientType());
1564+
}
1565+
1566+
@Test
1567+
public void testTreatCatalogAsPattern0_doesNotForceThrift() throws DatabricksSQLException {
1568+
// TreatMetadataCatalogNameAsPattern=0 (default, literal match) with explicit SEA → stays SEA
1569+
// Proves our new check doesn't trigger for the default/disabled value
1570+
IDatabricksConnectionContext ctx =
1571+
DatabricksConnectionContext.parse(
1572+
TestConstants.VALID_URL_2 + ";TreatMetadataCatalogNameAsPattern=0",
1573+
properties_with_pwd);
1574+
assertEquals(DatabricksClientType.SEA, ctx.getClientType());
1575+
}
1576+
1577+
@Test
1578+
public void testNoMetadataParams_defaultBehavior() throws DatabricksSQLException {
1579+
// No metadata params, no UseThriftClient → other checks decide (Arrow, CF, SAFE flag).
1580+
// VALID_URL_1 has no UseThriftClient and no SAFE flag in tests, so downstream
1581+
// checks (Arrow disabled, CF disabled, no flag) fall through to Thrift default.
1582+
// This tests that our metadata param check doesn't interfere with the default path.
1583+
IDatabricksConnectionContext ctx =
1584+
DatabricksConnectionContext.parse(TestConstants.VALID_URL_1, properties);
1585+
assertEquals(DatabricksClientType.THRIFT, ctx.getClientType());
1586+
}
1587+
1588+
@Test
1589+
public void testUseQueryForMetadataFalseString_doesNotForceThrift()
1590+
throws DatabricksSQLException {
1591+
// "false" is not "0" — our check uses .equals("0"), so "false" doesn't trigger it.
1592+
// With explicit SEA (VALID_URL_2), should stay SEA.
1593+
IDatabricksConnectionContext ctx =
1594+
DatabricksConnectionContext.parse(
1595+
TestConstants.VALID_URL_2 + ";UseQueryForMetadata=false", properties_with_pwd);
1596+
assertEquals(DatabricksClientType.SEA, ctx.getClientType());
1597+
}
1598+
1599+
@Test
1600+
public void testTreatCatalogAsPatternTrueString_doesNotForceThrift()
1601+
throws DatabricksSQLException {
1602+
// "true" is not "1" — our check uses .equals("1"), so "true" doesn't trigger it.
1603+
// With explicit SEA (VALID_URL_2), should stay SEA.
1604+
IDatabricksConnectionContext ctx =
1605+
DatabricksConnectionContext.parse(
1606+
TestConstants.VALID_URL_2 + ";TreatMetadataCatalogNameAsPattern=true",
1607+
properties_with_pwd);
1608+
assertEquals(DatabricksClientType.SEA, ctx.getClientType());
1609+
}
1610+
1611+
@Test
1612+
public void testCluster_metadataParamsIgnored() throws DatabricksSQLException {
1613+
// All-Purpose Cluster always uses Thrift regardless of metadata params
1614+
IDatabricksConnectionContext ctx =
1615+
DatabricksConnectionContext.parse(
1616+
TestConstants.VALID_CLUSTER_URL + ";UseQueryForMetadata=0", properties);
1617+
assertEquals(DatabricksClientType.THRIFT, ctx.getClientType());
1618+
1619+
ctx =
1620+
DatabricksConnectionContext.parse(
1621+
TestConstants.VALID_CLUSTER_URL + ";TreatMetadataCatalogNameAsPattern=1", properties);
1622+
assertEquals(DatabricksClientType.THRIFT, ctx.getClientType());
1623+
}
1624+
1625+
@Test
1626+
public void testUseQueryForMetadata0_withExplicitSEA_useQueryForMetadataStillFalse()
1627+
throws DatabricksSQLException {
1628+
// Even though SEA is forced, UseQueryForMetadata=0 should still report false
1629+
// (the metadata param value is independent of client type selection)
1630+
IDatabricksConnectionContext ctx =
1631+
DatabricksConnectionContext.parse(
1632+
TestConstants.VALID_URL_1 + ";UseThriftClient=0;UseQueryForMetadata=0", properties);
1633+
assertEquals(DatabricksClientType.SEA, ctx.getClientType());
1634+
assertFalse(ctx.useQueryForMetadata());
1635+
}
1636+
1637+
@Test
1638+
public void testTreatCatalogAsPattern1_withExplicitSEA_treatCatalogStillTrue()
1639+
throws DatabricksSQLException {
1640+
// Even though SEA is forced, TreatMetadataCatalogNameAsPattern=1 should still report true
1641+
IDatabricksConnectionContext ctx =
1642+
DatabricksConnectionContext.parse(
1643+
TestConstants.VALID_URL_1 + ";UseThriftClient=0;TreatMetadataCatalogNameAsPattern=1",
1644+
properties);
1645+
assertEquals(DatabricksClientType.SEA, ctx.getClientType());
1646+
assertTrue(ctx.treatMetadataCatalogNameAsPattern());
1647+
}
14901648
}

0 commit comments

Comments
 (0)