Skip to content

Commit f2f57ec

Browse files
authored
Fix GEOMETRY/GEOGRAPHY DATA_TYPE mapping in Thrift getColumns() (#1387)
## Summary - Thrift `getThriftRows()` was missing the `isGeospatialType()` check for the `DATA_TYPE` column in `getColumns()`, causing GEOMETRY/GEOGRAPHY columns to return `DATA_TYPE=0` (NULL) instead of the correct value - Added GEOMETRY/GEOGRAPHY to `getCode()` mapping as `Types.OTHER` (1111), consistent with VARIANT handling - When geospatial is disabled: `DATA_TYPE=12` (VARCHAR); when enabled: `DATA_TYPE=1111` (OTHER) ## Test plan - [x] Unit tests added for `getCode()` with GEOMETRY/GEOGRAPHY - [x] Unit tests added for `getThriftRows()` with geospatial enabled/disabled - [x] Verified end-to-end with live workspace (Thrift and SEA now match) - [x] Existing `MetadataResultSetBuilderTest` passes (134 tests) --------- Signed-off-by: Sreekanth Vadigi <sreekanth.vadigi@databricks.com>
1 parent c9a7beb commit f2f57ec

3 files changed

Lines changed: 51 additions & 2 deletions

File tree

NEXT_CHANGELOG.md

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,7 @@
1111
- Fixed `PARSE_SYNTAX_ERROR` for column names containing special characters (e.g., dots) when `EnableBatchedInserts` is enabled, by re-quoting column names with backticks in reconstructed multi-row INSERT statements.
1212
- Fixed Volume ingestion for SEA mode, which was broken due to statement being closed prematurely.
1313
- Fixed escaped pattern characters in catalogName for `getSchemas`, as returned catalogName should be unescaped.
14+
- Fixed `getColumns()` returning `DATA_TYPE=0` (NULL) for GEOMETRY/GEOGRAPHY columns in Thrift mode. Now returns `Types.VARCHAR` (12) when geospatial is disabled and `Types.OTHER` (1111) when enabled, consistent with SEA mode.
1415

1516
---
1617
*Note: When making changes, please add your change under the appropriate section

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

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1131,6 +1131,8 @@ int getCode(String s) {
11311131
case "CHARACTER":
11321132
return 1;
11331133
case "VARIANT":
1134+
case "GEOMETRY":
1135+
case "GEOGRAPHY":
11341136
return 1111;
11351137
}
11361138
if (s.startsWith(INTERVAL)) {
@@ -1626,8 +1628,10 @@ List<List<Object>> getThriftRows(List<List<Object>> rows, List<ResultColumn> col
16261628
}
16271629
}
16281630
if (column.getColumnName().equals(DATA_TYPE_COLUMN.getColumnName())) {
1629-
// Check if complex datatype support is disabled and this is a complex type
1630-
if (!ctx.isComplexDatatypeSupportEnabled() && isComplexType(typeVal)) {
1631+
// Check if geospatial support is disabled and this is a geospatial type
1632+
if (!ctx.isGeoSpatialSupportEnabled() && isGeospatialType(typeVal)) {
1633+
object = Types.VARCHAR;
1634+
} else if (!ctx.isComplexDatatypeSupportEnabled() && isComplexType(typeVal)) {
16311635
object = Types.VARCHAR;
16321636
} else {
16331637
object = getCode(stripBaseTypeName(typeVal));

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

Lines changed: 44 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -71,6 +71,8 @@ void testGetCode() {
7171
assert metadataResultSetBuilder.getCode("SMALLINT") == 5;
7272
assert metadataResultSetBuilder.getCode("INTEGER") == 4;
7373
assert metadataResultSetBuilder.getCode("VARIANT") == 1111;
74+
assert metadataResultSetBuilder.getCode("GEOMETRY") == 1111;
75+
assert metadataResultSetBuilder.getCode("GEOGRAPHY") == 1111;
7476
assert metadataResultSetBuilder.getCode("INTERVAL") == 12;
7577
assert metadataResultSetBuilder.getCode("INTERVAL YEAR") == 12;
7678
}
@@ -818,6 +820,48 @@ void testColumnDefIsPreservedInGetThriftRowsWhenDefaultExists() {
818820
"'42'", updatedRows.get(0).get(12), "COLUMN_DEF should return the actual default value");
819821
}
820822

823+
@Test
824+
void testGetThriftRowsGeospatialDataTypeWhenDisabled() {
825+
// When geospatial support is disabled, GEOMETRY/GEOGRAPHY DATA_TYPE should be VARCHAR (12)
826+
when(connectionContext.isGeoSpatialSupportEnabled()).thenReturn(false);
827+
lenient().when(connectionContext.isComplexDatatypeSupportEnabled()).thenReturn(true);
828+
829+
for (String typeName : List.of("GEOMETRY", "GEOGRAPHY")) {
830+
List<Object> row =
831+
Arrays.asList(
832+
"cat", "schema", "tbl", "col", 0, typeName, 10, null, 0, 10, 1, "", null, null, null,
833+
null, 0, "YES", null, null, null, null, "NO", "NO");
834+
List<List<Object>> updatedRows =
835+
metadataResultSetBuilder.getThriftRows(List.of(row), COLUMN_COLUMNS);
836+
837+
assertEquals(
838+
Types.VARCHAR,
839+
updatedRows.get(0).get(4),
840+
typeName + " DATA_TYPE should be VARCHAR (12) when geospatial is disabled");
841+
}
842+
}
843+
844+
@Test
845+
void testGetThriftRowsGeospatialDataTypeWhenEnabled() {
846+
// When geospatial support is enabled, GEOMETRY/GEOGRAPHY DATA_TYPE should be OTHER (1111)
847+
when(connectionContext.isGeoSpatialSupportEnabled()).thenReturn(true);
848+
when(connectionContext.isComplexDatatypeSupportEnabled()).thenReturn(true);
849+
850+
for (String typeName : List.of("GEOMETRY", "GEOGRAPHY")) {
851+
List<Object> row =
852+
Arrays.asList(
853+
"cat", "schema", "tbl", "col", 0, typeName, 10, null, 0, 10, 1, "", null, null, null,
854+
null, 0, "YES", null, null, null, null, "NO", "NO");
855+
List<List<Object>> updatedRows =
856+
metadataResultSetBuilder.getThriftRows(List.of(row), COLUMN_COLUMNS);
857+
858+
assertEquals(
859+
Types.OTHER,
860+
updatedRows.get(0).get(4),
861+
typeName + " DATA_TYPE should be OTHER (1111) when geospatial is enabled");
862+
}
863+
}
864+
821865
private static Stream<Arguments> provideColumnDefTypeNames() {
822866
return Stream.of(
823867
Arguments.of("INT"),

0 commit comments

Comments
 (0)