Skip to content

Commit b9b93af

Browse files
committed
Address review feedback: AIX default, log level, stale tests
- AIX/PowerPC: default to Arrow disabled (was enabled). Honour explicit EnableArrow=1 if user sets it. Added isAixOrPowerPc() helper. - Restore client-type guard for AIX — prevents routing to SEA when Arrow is disabled on AIX. - Log deprecation at INFO (not WARN) — logged once at connection init, not per statement. - Updated changelog to mention PowerPC architectures explicitly. - Renamed stale/contradictory tests: testClientTypeWhenArrowDisabled → clarified as non-AIX behavior testJsonInlineChunkedResults_withoutArrow → deprecated flag test - Added 2 new tests: default Arrow=true, explicit disable ignored. Co-authored-by: Isaac Signed-off-by: Gopal Lal <gopal.lal@databricks.com>
1 parent da555af commit b9b93af

4 files changed

Lines changed: 46 additions & 12 deletions

File tree

NEXT_CHANGELOG.md

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -12,7 +12,7 @@
1212

1313
4. **Native geospatial type support (`GEOMETRY` and `GEOGRAPHY`) is now enabled by default.** `getObject()` now returns `IGeometry`/`IGeography` instances instead of EWKT strings. Set `EnableGeoSpatialSupport=0` to restore the previous behavior.
1414

15-
5. **`EnableArrow` connection property is deprecated and ignored.** Arrow serialization is now always enabled. Setting `EnableArrow=0` previously disabled Arrow and forced columnar/JSON inline results; this value is now ignored and a deprecation warning is logged. For JSON inline results with SEA, disable CloudFetch via `EnableQueryResultDownload=0`. The only exception is AIX platforms, where `EnableArrow` is still honoured due to known Arrow native library compatibility issues.
15+
5. **`EnableArrow` connection property is deprecated and ignored.** Arrow serialization is now always enabled. Setting `EnableArrow=0` previously disabled Arrow and forced columnar/JSON inline results; this value is now ignored and a deprecation warning is logged. For JSON inline results with SEA, disable CloudFetch via `EnableQueryResultDownload=0`. Exception: on AIX platforms and PowerPC architectures (`os.arch` contains `ppc`), `EnableArrow` is still honoured and defaults to disabled due to known Arrow native library compatibility issues.
1616

1717
### Added
1818

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

Lines changed: 21 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -505,6 +505,10 @@ public DatabricksClientType getClientTypeFromContext() {
505505
remainingMs);
506506
return DatabricksClientType.THRIFT;
507507
}
508+
// On AIX/PowerPC, Arrow may be disabled — check before routing to SEA
509+
if (isAixOrPowerPc() && !shouldEnableArrow()) {
510+
return DatabricksClientType.THRIFT;
511+
}
508512
// Check if CloudFetch is disabled - Thrift is required for inline mode
509513
if (!isCloudFetchEnabled()) {
510514
return DatabricksClientType.THRIFT;
@@ -668,17 +672,20 @@ public Boolean shouldEnableArrow() {
668672
// Arrow is always enabled unless running on AIX or IBM Power (which have known
669673
// issues with the Arrow native library). The EnableArrow connection property is
670674
// deprecated and its value is ignored on non-AIX/IBM platforms.
671-
String osName = System.getProperty("os.name", "").toLowerCase();
672-
String osArch = System.getProperty("os.arch", "").toLowerCase();
673-
if (osName.contains("aix") || osArch.contains("ppc")) {
674-
// On AIX/IBM Power, honour the user's explicit setting (default is "1" = enabled)
675-
return Objects.equals(getParameter(DatabricksJdbcUrlParams.ENABLE_ARROW), "1");
675+
if (isAixOrPowerPc()) {
676+
// On AIX/PowerPC, Arrow native library has known issues — default to disabled.
677+
// Honour explicit EnableArrow=1 if user sets it.
678+
String explicitValue = getParameterIgnoreDefault(DatabricksJdbcUrlParams.ENABLE_ARROW);
679+
if (explicitValue != null) {
680+
return explicitValue.equals("1");
681+
}
682+
return false; // default disabled on AIX/PowerPC
676683
}
677684

678-
// Log deprecation warning if user explicitly set EnableArrow=0 (ignored)
685+
// Log deprecation warning once if user explicitly set EnableArrow=0 (ignored)
679686
String explicitValue = getParameterIgnoreDefault(DatabricksJdbcUrlParams.ENABLE_ARROW);
680687
if (explicitValue != null && explicitValue.equals("0")) {
681-
LOGGER.warn(
688+
LOGGER.info(
682689
"EnableArrow=0 is deprecated and ignored. Arrow serialization is always enabled. "
683690
+ "To use JSON inline results with SEA, disable CloudFetch via "
684691
+ "EnableQueryResultDownload=0.");
@@ -1213,6 +1220,13 @@ private String getParameterIgnoreDefault(DatabricksJdbcUrlParams key) {
12131220
return this.parameters.getOrDefault(key.getParamName().toLowerCase(), null);
12141221
}
12151222

1223+
/** Returns true if running on AIX or IBM PowerPC architecture. */
1224+
private static boolean isAixOrPowerPc() {
1225+
String osName = System.getProperty("os.name", "").toLowerCase();
1226+
String osArch = System.getProperty("os.arch", "").toLowerCase();
1227+
return osName.contains("aix") || osArch.contains("ppc");
1228+
}
1229+
12161230
/**
12171231
* Resolves a boolean feature flag with client-side priority over server-side.
12181232
*

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

Lines changed: 21 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -358,6 +358,21 @@ public void testEnableCloudFetch() throws DatabricksSQLException {
358358
assertTrue(connectionContext.shouldEnableArrow());
359359
}
360360

361+
@Test
362+
public void testShouldEnableArrow_defaultIsTrue() throws DatabricksSQLException {
363+
// On non-AIX, Arrow is always enabled regardless of EnableArrow setting
364+
IDatabricksConnectionContext ctx =
365+
DatabricksConnectionContext.parse(TestConstants.VALID_URL_1, properties);
366+
assertTrue(ctx.shouldEnableArrow(), "Arrow should be enabled by default");
367+
}
368+
369+
@Test
370+
public void testShouldEnableArrow_explicitDisableIgnoredOnNonAix() throws DatabricksSQLException {
371+
IDatabricksConnectionContext ctx =
372+
DatabricksConnectionContext.parse(TestConstants.VALID_URL_1 + ";EnableArrow=0", properties);
373+
assertTrue(ctx.shouldEnableArrow(), "EnableArrow=0 should be ignored on non-AIX");
374+
}
375+
361376
@Test
362377
public void testAllPurposeClusterParsing() throws DatabricksSQLException {
363378
DatabricksConnectionContext connectionContext =
@@ -947,7 +962,9 @@ public void testClientTypeWithExplicitUseThriftClientDisabled() throws Databrick
947962
}
948963

949964
@Test
950-
public void testClientTypeWhenArrowDisabled() throws DatabricksSQLException {
965+
public void testClientTypeWhenArrowDisabled_nonAix_ignoredDefaultsToThrift()
966+
throws DatabricksSQLException {
967+
// EnableArrow=0 is ignored on non-AIX — but without SEA feature flag, defaults to THRIFT
951968
String urlWithArrowDisabled =
952969
"jdbc:databricks://sample-host.cloud.databricks.com:9999/default;AuthMech=3;"
953970
+ "httpPath=/sql/1.0/warehouses/9999999999999999;EnableArrow=0";
@@ -969,7 +986,9 @@ public void testClientTypeWhenCloudFetchDisabled() throws DatabricksSQLException
969986
}
970987

971988
@Test
972-
public void testClientTypeWhenBothArrowAndCloudFetchDisabled() throws DatabricksSQLException {
989+
public void testClientTypeWhenBothArrowAndCloudFetchDisabled_nonAix()
990+
throws DatabricksSQLException {
991+
// EnableArrow=0 ignored on non-AIX; CloudFetch disabled → THRIFT
973992
String urlWithBothDisabled =
974993
"jdbc:databricks://sample-host.cloud.databricks.com:9999/default;AuthMech=3;"
975994
+ "httpPath=/sql/1.0/warehouses/9999999999999999;EnableArrow=0;EnableQueryResultDownload=0";

src/test/java/com/databricks/jdbc/integration/fakeservice/tests/SqlExecApiIntegrationTests.java

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -14,9 +14,10 @@
1414
public class SqlExecApiIntegrationTests extends AbstractFakeServiceIntegrationTests {
1515

1616
@Test
17-
void testJsonInlineChunkedResults_withoutArrow() throws SQLException {
17+
void testChunkedResults_enableArrowDeprecatedIgnored() throws SQLException {
18+
// EnableArrow=0 is deprecated and ignored on non-AIX — Arrow is always enabled.
19+
// This test verifies the query still succeeds with the deprecated flag set.
1820
final String table = "samples.tpch.lineitem";
19-
// Limit set such that atleast 2 chunks are present for results
2021
final int maxRows = 64000;
2122
final String sql = "SELECT * FROM " + table + " limit " + maxRows;
2223

0 commit comments

Comments
 (0)