Skip to content

Commit 922f590

Browse files
authored
geospatial fixes (#1062)
## Description <!-- Provide a brief summary of the changes made and the issue they aim to address.--> This PR addresses comments from Milan's review on PR #988 (Geospatial support implementation). ## Changes ### 1. Cache WKB Value for Performance - Added `wkb` field to `AbstractDatabricksGeospatial` to cache WKB representation - WKB now computed once in constructor instead of on every `getWKB()` call ### 2. Simplify SRID Extraction Logic - Removed metadata-based SRID extraction entirely - Per Milan's clarification: "SRID is always present in EWKT unless it's 0" - Simplified `convertToGeospatial()` to only extract SRID from EWKT data - Removed `extractSRIDFromMetadata()` method and regex patterns - Removed unused `arrowMetadata` parameter from `convertToGeospatial()` ## Testing - All unit tests are passing ## Additional Notes to the Reviewer <!-- Share any additional context or insights that may help the reviewer understand the changes better. This could include challenges faced, limitations, or compromises made during the development process. Also, mention any areas of the code that you would like the reviewer to focus on specifically. --> NO_CHANGELOG=true (changelog already added in previous PR) Signed-off-by: Sreekanth Vadigi <sreekanth.vadigi@databricks.com>
1 parent 4d13570 commit 922f590

2 files changed

Lines changed: 12 additions & 62 deletions

File tree

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

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,7 @@ public abstract class AbstractDatabricksGeospatial implements IDatabricksGeospat
2020

2121
private final String wkt;
2222
private final int srid; // Spatial Reference System Identifier
23+
private final byte[] wkb;
2324

2425
/**
2526
* Constructs an AbstractDatabricksGeospatial with the specified WKT and SRID.
@@ -37,17 +38,17 @@ protected AbstractDatabricksGeospatial(String wkt, int srid)
3738

3839
this.wkt = wkt.trim();
3940
this.srid = srid;
41+
this.wkb = WKTConverter.toWKB(this.wkt);
4042
}
4143

4244
/**
4345
* Returns the Well-Known Binary (WKB) representation of the geospatial object.
4446
*
4547
* @return the WKB representation as a byte array
46-
* @throws DatabricksValidationException if WKT to WKB conversion fails
4748
*/
4849
@Override
49-
public byte[] getWKB() throws DatabricksValidationException {
50-
return WKTConverter.toWKB(wkt);
50+
public byte[] getWKB() {
51+
return wkb;
5152
}
5253

5354
/**

src/main/java/com/databricks/jdbc/api/impl/converters/ArrowToJavaObjectConverter.java

Lines changed: 8 additions & 59 deletions
Original file line numberDiff line numberDiff line change
@@ -10,7 +10,6 @@
1010
import com.databricks.jdbc.log.JdbcLoggerFactory;
1111
import com.databricks.jdbc.model.core.ColumnInfo;
1212
import com.databricks.jdbc.model.core.ColumnInfoTypeName;
13-
import com.databricks.jdbc.model.telemetry.enums.DatabricksDriverErrorCode;
1413
import java.math.BigDecimal;
1514
import java.math.RoundingMode;
1615
import java.sql.Date;
@@ -22,8 +21,6 @@
2221
import java.util.List;
2322
import java.util.Optional;
2423
import java.util.function.Function;
25-
import java.util.regex.Matcher;
26-
import java.util.regex.Pattern;
2724
import org.apache.arrow.vector.TimeStampMicroTZVector;
2825
import org.apache.arrow.vector.ValueVector;
2926
import org.apache.arrow.vector.util.Text;
@@ -32,10 +29,6 @@ public class ArrowToJavaObjectConverter {
3229
private static final JdbcLogger LOGGER =
3330
JdbcLoggerFactory.getLogger(ArrowToJavaObjectConverter.class);
3431

35-
// Pre-compiled patterns for SRID extraction from metadata
36-
private static final Pattern GEOMETRY_SRID_PATTERN = Pattern.compile("GEOMETRY\\((\\d+)\\)");
37-
private static final Pattern GEOGRAPHY_SRID_PATTERN = Pattern.compile("GEOGRAPHY\\((\\d+)\\)");
38-
3932
private static final List<DateTimeFormatter> DATE_FORMATTERS =
4033
Arrays.asList(
4134
DateTimeFormatter.ofPattern("yyyy-MM-dd"),
@@ -148,7 +141,7 @@ public static Object convert(
148141
return ic.toLiteral(object);
149142
case GEOMETRY:
150143
case GEOGRAPHY:
151-
return convertToGeospatial(object, arrowMetadata, requiredType);
144+
return convertToGeospatial(object, requiredType);
152145
case NULL:
153146
return null;
154147
default:
@@ -177,23 +170,18 @@ private static Object convertToStruct(Object object, String arrowMetadata)
177170
}
178171

179172
private static AbstractDatabricksGeospatial convertToGeospatial(
180-
Object object, String arrowMetadata, ColumnInfoTypeName type) throws DatabricksSQLException {
173+
Object object, ColumnInfoTypeName type) throws DatabricksSQLException {
181174
String ewkt = convertToString(object);
182175

183-
// Parse EWKT to extract SRID from data if present
184-
int dataSrid = WKTConverter.extractSRIDFromEWKT(ewkt);
176+
// Parse EWKT to extract SRID from data
177+
// SRID is always present in EWKT unless it's 0, in which case it is handled in
178+
// WKTConverter.extractSRIDFromEWKT()
179+
int srid = WKTConverter.extractSRIDFromEWKT(ewkt);
185180
String cleanWkt = WKTConverter.removeSRIDFromEWKT(ewkt);
186181

187-
// Extract SRID from metadata if not present in data
188-
int finalSrid = dataSrid;
189-
if (dataSrid == 0) {
190-
String typeName = type == ColumnInfoTypeName.GEOMETRY ? GEOMETRY : GEOGRAPHY;
191-
finalSrid = extractSRIDFromMetadata(arrowMetadata, typeName);
192-
}
193-
194182
return type == ColumnInfoTypeName.GEOMETRY
195-
? new DatabricksGeometry(cleanWkt, finalSrid)
196-
: new DatabricksGeography(cleanWkt, finalSrid);
183+
? new DatabricksGeometry(cleanWkt, srid)
184+
: new DatabricksGeography(cleanWkt, srid);
197185
}
198186

199187
private static Object convertToTimestamp(Object object, Optional<String> timeZoneOpt)
@@ -327,43 +315,4 @@ private static <T extends Number> T convertToNumber(
327315
LOGGER.error(errorMessage);
328316
throw new DatabricksValidationException(errorMessage);
329317
}
330-
331-
/**
332-
* Extracts SRID from Arrow metadata string.
333-
*
334-
* @param metadata Arrow metadata like "GEOMETRY(32633)" or "GEOGRAPHY(4326)"
335-
* @param typePrefix The prefix to look for ("GEOMETRY" or "GEOGRAPHY")
336-
* @return SRID value, or 0 if not found
337-
* @throws DatabricksParsingException if metadata format is invalid
338-
*/
339-
private static int extractSRIDFromMetadata(String metadata, String typePrefix)
340-
throws DatabricksParsingException {
341-
if (metadata == null) {
342-
LOGGER.debug("Metadata is null, returning default SRID 0 for {}", typePrefix);
343-
return 0;
344-
}
345-
346-
try {
347-
// Look for pattern like "GEOMETRY(32633)" or "GEOGRAPHY(4326)"
348-
Pattern pattern =
349-
typePrefix.equals(GEOMETRY) ? GEOMETRY_SRID_PATTERN : GEOGRAPHY_SRID_PATTERN;
350-
Matcher m = pattern.matcher(metadata);
351-
352-
if (m.find()) {
353-
return Integer.parseInt(m.group(1));
354-
}
355-
} catch (Exception e) {
356-
String errorMessage =
357-
String.format("Failed to parse SRID from %s metadata: %s", typePrefix, metadata);
358-
LOGGER.error(errorMessage, e);
359-
throw new DatabricksParsingException(
360-
errorMessage, e, DatabricksDriverErrorCode.RESULT_SET_ERROR);
361-
}
362-
363-
LOGGER.debug(
364-
"No SRID found in metadata for {}, returning default SRID 0. Metadata: {}",
365-
typePrefix,
366-
metadata);
367-
return 0;
368-
}
369318
}

0 commit comments

Comments
 (0)