Skip to content

Commit 46bd9e0

Browse files
authored
Decouple geospatial support from complex datatype flag (#1438)
## Summary `EnableGeoSpatialSupport` is now independent of `EnableComplexDatatypeSupport`. Previously, `isGeoSpatialSupportEnabled()` required `isComplexDatatypeSupportEnabled()` to be true, which was an unnecessary coupling. ### Before ``` EnableComplexDatatypeSupport=0 + EnableGeoSpatialSupport=1 → geospatial DISABLED ``` ### After ``` EnableComplexDatatypeSupport=0 + EnableGeoSpatialSupport=1 → geospatial ENABLED ``` Geospatial types (GEOMETRY, GEOGRAPHY) have their own conversion path (`GeospatialConverter`, `WKTConverter`) and don't depend on complex type handling (ARRAY, MAP, STRUCT). ### Changes - `DatabricksConnectionContext.isGeoSpatialSupportEnabled()` — removed `isComplexDatatypeSupportEnabled()` gate - `IDatabricksConnectionContext` — updated Javadoc to remove dependency claim - `ArrowStreamResultTest` — updated test to reflect independence ## Test plan - [x] `DatabricksConnectionContextTest` — 110 tests pass - [x] `ArrowStreamResultTest` — 15 tests pass (updated coupling test) - [x] `DatabricksResultSetMetaDataTest` — 28 tests pass - [x] `MetadataResultSetBuilderTest` — 134 tests pass NO_CHANGELOG=true This pull request was AI-assisted by Isaac. --------- Signed-off-by: Gopal Lal <gopal.lal@databricks.com>
1 parent 780e0e1 commit 46bd9e0

13 files changed

Lines changed: 136 additions & 57 deletions

NEXT_CHANGELOG.md

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@
55
### Added
66

77
### Updated
8+
- `EnableGeoSpatialSupport` no longer requires `EnableComplexDatatypeSupport=1`. Geospatial types (GEOMETRY, GEOGRAPHY) can now be enabled independently of complex type support (ARRAY, MAP, STRUCT).
89

910
### Fixed
1011

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

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -995,9 +995,7 @@ public boolean isComplexDatatypeSupportEnabled() {
995995

996996
@Override
997997
public boolean isGeoSpatialSupportEnabled() {
998-
// Geospatial support requires complex datatype support to be enabled
999-
return isComplexDatatypeSupportEnabled()
1000-
&& getParameter(DatabricksJdbcUrlParams.ENABLE_GEOSPATIAL_SUPPORT).equals("1");
998+
return getParameter(DatabricksJdbcUrlParams.ENABLE_GEOSPATIAL_SUPPORT).equals("1");
1001999
}
10021000

10031001
@Override

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

Lines changed: 23 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -526,6 +526,10 @@ public Object getObject(int columnIndex) throws SQLException {
526526
}
527527
int columnType = resultSetMetaData.getColumnType(columnIndex);
528528
String columnTypeName = resultSetMetaData.getColumnTypeName(columnIndex);
529+
// Geospatial types: handle independently of complex datatype flag
530+
if (isGeospatialType(columnTypeName)) {
531+
return handleGeospatialType(obj, columnTypeName);
532+
}
529533
// separate handling for complex data types
530534
if (isComplexType(columnTypeName)) {
531535
return handleComplexDataTypes(obj, columnTypeName);
@@ -541,6 +545,25 @@ public Object getObject(int columnIndex) throws SQLException {
541545
return ConverterHelper.convertSqlTypeToJavaType(columnType, obj);
542546
}
543547

548+
private Object handleGeospatialType(Object obj, String columnName) throws DatabricksSQLException {
549+
if (resultSetType == ResultSetType.SEA_INLINE) {
550+
obj = convertGeospatialForSEAInline(obj, columnName);
551+
}
552+
return obj;
553+
}
554+
555+
private Object convertGeospatialForSEAInline(Object obj, String columnName)
556+
throws DatabricksSQLException {
557+
if (columnName.startsWith(GEOMETRY)) {
558+
return ConverterHelper.getConverterForColumnType(Types.OTHER, GEOMETRY)
559+
.toDatabricksGeometry(obj);
560+
} else if (columnName.startsWith(GEOGRAPHY)) {
561+
return ConverterHelper.getConverterForColumnType(Types.OTHER, GEOGRAPHY)
562+
.toDatabricksGeography(obj);
563+
}
564+
return obj;
565+
}
566+
544567
private Object handleComplexDataTypes(Object obj, String columnName)
545568
throws DatabricksSQLException {
546569
if (resultSetType == ResultSetType.SEA_INLINE) {
@@ -558,12 +581,6 @@ private Object convertToComplexDataTypesForSEAInline(Object obj, String columnNa
558581
return parser.parseJsonStringToDbMap(obj.toString(), columnName);
559582
} else if (columnName.startsWith(STRUCT)) {
560583
return parser.parseJsonStringToDbStruct(obj.toString(), columnName);
561-
} else if (columnName.startsWith(GEOMETRY)) {
562-
return ConverterHelper.getConverterForColumnType(Types.OTHER, GEOMETRY)
563-
.toDatabricksGeometry(obj);
564-
} else if (columnName.startsWith(GEOGRAPHY)) {
565-
return ConverterHelper.getConverterForColumnType(Types.OTHER, GEOGRAPHY)
566-
.toDatabricksGeography(obj);
567584
}
568585
throw new DatabricksParsingException(
569586
"Unexpected metadata format. Type is not a COMPLEX: " + columnName,

src/main/java/com/databricks/jdbc/api/impl/arrow/ArrowStreamResult.java

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -253,9 +253,7 @@ public Object getObject(int columnIndex) throws DatabricksSQLException {
253253
public static boolean isComplexType(ColumnInfoTypeName type) {
254254
return type == ColumnInfoTypeName.ARRAY
255255
|| type == ColumnInfoTypeName.MAP
256-
|| type == ColumnInfoTypeName.STRUCT
257-
|| type == ColumnInfoTypeName.GEOMETRY
258-
|| type == ColumnInfoTypeName.GEOGRAPHY;
256+
|| type == ColumnInfoTypeName.STRUCT;
259257
}
260258

261259
/**

src/main/java/com/databricks/jdbc/api/internal/IDatabricksConnectionContext.java

Lines changed: 1 addition & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -308,10 +308,7 @@ public interface IDatabricksConnectionContext {
308308
/** Returns true if driver return complex data type java objects natively as opposed to string */
309309
boolean isComplexDatatypeSupportEnabled();
310310

311-
/**
312-
* Returns true if driver returns GEOMETRY and GEOGRAPHY types natively. Requires
313-
* isComplexDatatypeSupportEnabled() to be true
314-
*/
311+
/** Returns true if driver returns GEOMETRY and GEOGRAPHY types natively. */
315312
boolean isGeoSpatialSupportEnabled();
316313

317314
/** Returns the size for HTTP connection pool */

src/main/java/com/databricks/jdbc/common/DatabricksJdbcUrlParams.java

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -130,7 +130,7 @@ public enum DatabricksJdbcUrlParams {
130130
"0"),
131131
ENABLE_GEOSPATIAL_SUPPORT(
132132
"EnableGeoSpatialSupport",
133-
"flag to enable native support of GEOMETRY and GEOGRAPHY data types. Requires EnableComplexDatatypeSupport=1",
133+
"flag to enable native support of GEOMETRY and GEOGRAPHY data types",
134134
"0"),
135135
ROWS_FETCHED_PER_BLOCK(
136136
"RowsFetchedPerBlock",

src/main/java/com/databricks/jdbc/common/util/DatabricksThriftUtil.java

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -233,7 +233,8 @@ public static ColumnInfo getColumnInfoFromTColumnDesc(
233233

234234
String typeText = getTypeTextFromTypeDesc(columnDesc.getTypeDesc());
235235

236-
if (arrowMetadata != null && isComplexType(arrowMetadata)) {
236+
if (arrowMetadata != null
237+
&& (isComplexType(arrowMetadata) || isGeospatialType(arrowMetadata))) {
237238
typeText = arrowMetadata;
238239
if (arrowMetadata.startsWith(GEOMETRY)) {
239240
columnInfoTypeName = ColumnInfoTypeName.GEOMETRY;

src/main/java/com/databricks/jdbc/common/util/DatabricksTypeUtil.java

Lines changed: 13 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -561,18 +561,22 @@ public static String getDecimalTypeString(BigDecimal bd) {
561561
}
562562

563563
/**
564-
* Checks if the given type name represents a complex type (ARRAY, MAP, STRUCT, GEOMETRY, or
565-
* GEOGRAPHY).
564+
* Checks if the given type name represents a complex type (ARRAY, MAP, STRUCT).
566565
*
567566
* @param typeName The type name to check
568-
* @return true if the type name starts with ARRAY, MAP, STRUCT, GEOMETRY, or GEOGRAPHY, false
569-
* otherwise
567+
* @return true if the type name starts with ARRAY, MAP, or STRUCT
570568
*/
571569
public static boolean isComplexType(String typeName) {
572-
return typeName.startsWith(ARRAY)
573-
|| typeName.startsWith(MAP)
574-
|| typeName.startsWith(STRUCT)
575-
|| typeName.startsWith(GEOMETRY)
576-
|| typeName.startsWith(GEOGRAPHY);
570+
return typeName.startsWith(ARRAY) || typeName.startsWith(MAP) || typeName.startsWith(STRUCT);
571+
}
572+
573+
/**
574+
* Checks if the given type name represents a geospatial type (GEOMETRY, GEOGRAPHY).
575+
*
576+
* @param typeName The type name to check
577+
* @return true if the type name starts with GEOMETRY or GEOGRAPHY
578+
*/
579+
public static boolean isGeospatialType(String typeName) {
580+
return typeName.startsWith(GEOMETRY) || typeName.startsWith(GEOGRAPHY);
577581
}
578582
}

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

Lines changed: 53 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1488,6 +1488,59 @@ public void testUseQueryForMetadataExplicitFalseOnWarehouse() throws DatabricksS
14881488
assertFalse(ctx.useQueryForMetadata());
14891489
}
14901490

1491+
// ---------------------------------------------------------------------------
1492+
// Geospatial flag independence from complex datatype flag
1493+
// ---------------------------------------------------------------------------
1494+
1495+
@Test
1496+
public void testGeospatialEnabled_complexDisabled() throws DatabricksSQLException {
1497+
IDatabricksConnectionContext ctx =
1498+
DatabricksConnectionContext.parse(
1499+
TestConstants.VALID_URL_1 + ";EnableGeoSpatialSupport=1;EnableComplexDatatypeSupport=0",
1500+
properties);
1501+
assertTrue(ctx.isGeoSpatialSupportEnabled());
1502+
assertFalse(ctx.isComplexDatatypeSupportEnabled());
1503+
}
1504+
1505+
@Test
1506+
public void testGeospatialDisabled_complexEnabled() throws DatabricksSQLException {
1507+
IDatabricksConnectionContext ctx =
1508+
DatabricksConnectionContext.parse(
1509+
TestConstants.VALID_URL_1 + ";EnableGeoSpatialSupport=0;EnableComplexDatatypeSupport=1",
1510+
properties);
1511+
assertFalse(ctx.isGeoSpatialSupportEnabled());
1512+
assertTrue(ctx.isComplexDatatypeSupportEnabled());
1513+
}
1514+
1515+
@Test
1516+
public void testGeospatialAndComplexBothEnabled() throws DatabricksSQLException {
1517+
IDatabricksConnectionContext ctx =
1518+
DatabricksConnectionContext.parse(
1519+
TestConstants.VALID_URL_1 + ";EnableGeoSpatialSupport=1;EnableComplexDatatypeSupport=1",
1520+
properties);
1521+
assertTrue(ctx.isGeoSpatialSupportEnabled());
1522+
assertTrue(ctx.isComplexDatatypeSupportEnabled());
1523+
}
1524+
1525+
@Test
1526+
public void testGeospatialAndComplexBothDisabled() throws DatabricksSQLException {
1527+
IDatabricksConnectionContext ctx =
1528+
DatabricksConnectionContext.parse(
1529+
TestConstants.VALID_URL_1 + ";EnableGeoSpatialSupport=0;EnableComplexDatatypeSupport=0",
1530+
properties);
1531+
assertFalse(ctx.isGeoSpatialSupportEnabled());
1532+
assertFalse(ctx.isComplexDatatypeSupportEnabled());
1533+
}
1534+
1535+
@Test
1536+
public void testGeospatialDefaultDisabled() throws DatabricksSQLException {
1537+
// Neither flag set — both default to disabled
1538+
IDatabricksConnectionContext ctx =
1539+
DatabricksConnectionContext.parse(TestConstants.VALID_URL_1, properties);
1540+
assertFalse(ctx.isGeoSpatialSupportEnabled());
1541+
assertFalse(ctx.isComplexDatatypeSupportEnabled());
1542+
}
1543+
14911544
// ---------------------------------------------------------------------------
14921545
// Client type selection with Thrift-native metadata params
14931546
// ---------------------------------------------------------------------------

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

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -765,9 +765,9 @@ public void testJsonArrayFormatMetadataWithGeospatialFlagDisabled() throws SQLEx
765765

766766
@Test
767767
public void testJsonArrayWithComplexTypesEnabledButGeospatialDisabled() throws SQLException {
768-
// This test validates the important scenario where EnableComplexDatatypeSupport=1
769-
// but EnableGeoSpatialSupport=0 (disabled). This simulates real-world usage where
770-
// users want complex types (ARRAY, MAP, STRUCT) but want geospatial data as strings.
768+
// This test validates that with EnableGeoSpatialSupport=0, geospatial columns
769+
// report as STRING in metadata regardless of the EnableComplexDatatypeSupport setting.
770+
// The two flags are independent.
771771
//
772772
// Expected behavior:
773773
// - GEOMETRY/GEOGRAPHY column types should report as STRING in metadata

0 commit comments

Comments
 (0)