ayushtkn commented on code in PR #6581:
URL: https://github.com/apache/hive/pull/6581#discussion_r3634059910
##########
ql/src/java/org/apache/hadoop/hive/ql/udf/esri/GeometryUtils.java:
##########
@@ -67,258 +274,321 @@ public int getIndex() {
public static final WritableBinaryObjectInspector
geometryTransportObjectInspector =
PrimitiveObjectInspectorFactory.writableBinaryObjectInspector;
- private static final Cache<BytesWritable, OGCGeometry> geometryCache =
CacheBuilder.newBuilder().weakKeys().build();
+ private static final Cache<BytesWritable, Geometry> GEOMETRY_CACHE =
+ CacheBuilder.newBuilder().maximumSize(1024).build();
/**
- * @param geomref1
- * @param geomref2
- * @return return true if both geometries are in the same spatial reference
+ * Detect whether the given bytes use the new WKB format.
+ */
+ public static boolean isNewFormat(BytesWritable geomref) {
+ return geomref != null && geomref.getLength() > 4 && geomref.getBytes()[4]
== NEW_FORMAT_MAGIC;
+ }
+
+ /**
+ * Compare spatial references of two geometry byte arrays.
*/
public static boolean compareSpatialReferences(BytesWritable geomref1,
BytesWritable geomref2) {
return getWKID(geomref1) == getWKID(geomref2);
}
- public static BytesWritable geometryToEsriShapeBytesWritable(MapGeometry
mapGeometry) {
- return serialize(mapGeometry);
+ /**
+ * Serialize a JTS Geometry to the new WKB wire format.
+ * Normalizes polygon ring orientation to OGC standard (exterior CCW,
interior CW).
+ */
+ public static BytesWritable geometryToEsriShapeBytesWritable(Geometry
geometry) {
+ if (geometry == null) {
+ return null;
+ }
+ geometry = normalizeRingOrientation(geometry);
+ return new CachedGeometryBytesWritable(geometry);
}
- public static BytesWritable geometryToEsriShapeBytesWritable(Geometry
geometry, int wkid, OGCType type) {
- return serialize(geometry, wkid, type);
+ /**
+ * Serialize a JTS Geometry with a specific SRID.
+ * Normalizes polygon ring orientation to OGC standard (exterior CCW,
interior CW).
+ */
+ public static BytesWritable geometryToEsriShapeBytesWritable(Geometry
geometry, int wkid) {
+ if (geometry == null) {
+ return null;
+ }
+ geometry = normalizeRingOrientation(geometry);
+ geometry.setSRID(wkid);
+ return new CachedGeometryBytesWritable(geometry);
}
- public static BytesWritable geometryToEsriShapeBytesWritable(OGCGeometry
geometry) {
- return new CachedGeometryBytesWritable(geometry);
+ /**
+ * Normalize polygon ring orientation to OGC standard:
+ * exterior rings counter-clockwise (CCW), interior rings clockwise (CW).
+ * Non-polygon geometries are returned unchanged.
+ */
+ public static Geometry normalizeRingOrientation(Geometry geom) {
+ if (geom instanceof Polygon polygon) {
+ return normalizePolygonRings(polygon);
+ } else if (geom instanceof MultiPolygon mp) {
+ Polygon[] polys = new Polygon[mp.getNumGeometries()];
+ for (int i = 0; i < mp.getNumGeometries(); i++) {
+ polys[i] = normalizePolygonRings((Polygon) mp.getGeometryN(i));
+ }
+ MultiPolygon result = geom.getFactory().createMultiPolygon(polys);
+ result.setSRID(geom.getSRID());
+ return result;
+ }
+ return geom;
}
- public static OGCGeometry geometryFromEsriShape(BytesWritable geomref) {
- // always assume bytes are recycled and can't be cached by using
- // geomref.getBytes() as the key
- return geometryFromEsriShape(geomref, true);
+ private static Polygon normalizePolygonRings(Polygon polygon) {
+ GeometryFactory factory = polygon.getFactory();
+ LinearRing shell = polygon.getExteriorRing();
+
+ // Exterior ring should be CCW
+ boolean shellChanged = false;
+ if (shell.getNumPoints() >= 4 &&
!Orientation.isCCW(shell.getCoordinateSequence())) {
+ shell = shell.reverse();
+ shellChanged = true;
+ }
+
+ // Interior rings should be CW (not CCW)
+ int numHoles = polygon.getNumInteriorRing();
+ LinearRing[] holes = new LinearRing[numHoles];
+ boolean holesChanged = false;
+ for (int i = 0; i < numHoles; i++) {
+ LinearRing hole = polygon.getInteriorRingN(i);
+ if (hole.getNumPoints() >= 4 &&
Orientation.isCCW(hole.getCoordinateSequence())) {
+ holes[i] = hole.reverse();
+ holesChanged = true;
+ } else {
+ holes[i] = hole;
+ }
+ }
+
+ if (!shellChanged && !holesChanged) {
+ return polygon;
+ }
+
+ Polygon result = factory.createPolygon(shell, holes);
+ result.setSRID(polygon.getSRID());
+ return result;
}
- public static OGCGeometry geometryFromEsriShape(BytesWritable geomref,
boolean bytesRecycled) {
+ /**
+ * Deserialize geometry bytes (either old ESRI format or new WKB format) to
a JTS Geometry.
+ */
+ public static Geometry geometryFromEsriShape(BytesWritable geomref) {
+ return geometryFromEsriShape(geomref, true);
+ }
+ /**
+ * Deserialize geometry bytes to a JTS Geometry.
+ * @param geomref the geometry bytes
+ * @param bytesRecycled true if the bytes backing the writable may be reused
(disables caching)
+ */
+ public static Geometry geometryFromEsriShape(BytesWritable geomref, boolean
bytesRecycled) {
if (geomref == null) {
return null;
}
- // this geomref might actually be a LazyGeometryBytesWritable which
- // means we don't need to deserialize from bytes
- if (geomref instanceof CachedGeometryBytesWritable) {
- return ((CachedGeometryBytesWritable) geomref).getGeometry();
+ if (geomref instanceof CachedGeometryBytesWritable cached) {
+ return cached.getGeometry();
}
- // if geomref bytes are recycled, we can't use the cache because every
- // key in the cache will be the same byte array
if (!bytesRecycled) {
- // check for a cache hit to previously created geometries
- OGCGeometry cachedGeom = geometryCache.getIfPresent(geomref);
-
+ Geometry cachedGeom = GEOMETRY_CACHE.getIfPresent(geomref);
if (cachedGeom != null) {
return cachedGeom;
}
}
- // not in cache or instance of CachedGeometryBytesWritable. now
- // need to create the geometry from its bytes
- int wkid = getWKID(geomref);
- ByteBuffer shapeBuffer = getShapeByteBuffer(geomref);
+ int length = geomref.getLength();
+ byte[] bytes = geomref.getBytes();
- //minimum for a shape, even an empty one, is the 4 byte type record
- if (shapeBuffer.limit() < 4) {
+ if (length < 5) {
return null;
+ }
+
+ Geometry geom;
+ int srid;
+
+ if (bytes[4] == NEW_FORMAT_MAGIC) {
+ // New WKB format: bytes 0-3 = SRID (big-endian), byte 4 = 0xFF, bytes
5+ = WKB
+ srid = ((bytes[0] & 0xFF) << 24) | ((bytes[1] & 0xFF) << 16)
+ | ((bytes[2] & 0xFF) << 8) | (bytes[3] & 0xFF);
+ byte[] wkb = Arrays.copyOfRange(bytes, 5, length);
+ try {
+ geom = wkbReader().read(wkb);
+ } catch (ParseException e) {
+ LOG.warn("Failed to parse WKB geometry", e);
+ return null;
+ }
} else {
- if (shapeBuffer.getInt(0) == Geometry.Type.Unknown.value()) { //empty
Geometry, intentional
+ // Old ESRI format: bytes 0-3 = WKID (little-endian), byte 4 = OGC type
(0-6), bytes 5+ = ESRI shape
+ srid = ByteBuffer.wrap(bytes, 0,
4).order(ByteOrder.LITTLE_ENDIAN).getInt();
+ ByteBuffer shapeBuffer = ByteBuffer.wrap(bytes, SIZE_WKID + SIZE_TYPE,
length - SIZE_WKID - SIZE_TYPE)
+ .slice().order(ByteOrder.LITTLE_ENDIAN);
+
+ if (shapeBuffer.limit() < 4) {
return null;
- } else {
- SpatialReference spatialReference = null;
- if (wkid != GeometryUtils.WKID_UNKNOWN) {
- spatialReference = SpatialReference.create(wkid);
- }
+ }
- Geometry esriGeom = OperatorImportFromESRIShape.local().execute(0,
Geometry.Type.Unknown, shapeBuffer);
- OGCGeometry createdGeom = OGCGeometry.createFromEsriGeometry(esriGeom,
spatialReference);
+ // Check if shape type is Unknown (empty geometry)
+ if (shapeBuffer.getInt(0) == 0) {
+ return null;
+ }
- if (!bytesRecycled) {
- // only add bytes to cache if we know they aren't being recycled
- geometryCache.put(geomref, createdGeom);
- }
+ // Use native EsriShapeConverter to read the old ESRI shape format
+ try {
+ geom = EsriShapeConverter.fromEsriShape(shapeBuffer);
+ } catch (Exception e) {
+ LOG.warn("Failed to parse ESRI shape geometry", e);
+ return null;
+ }
+ }
- return createdGeom;
+ if (geom != null) {
+ geom.setSRID(srid);
+ if (!bytesRecycled) {
+ GEOMETRY_CACHE.put(geomref, geom);
}
}
+
+ return geom;
}
/**
- * Gets the geometry type for the given hive geometry bytes
- *
- * @param geomref reference to hive geometry bytes
- * @return OGCType set in the 5th byte of the hive geometry bytes
+ * Gets the geometry type for the given hive geometry bytes.
+ * For new WKB format, reads the type directly from the WKB header
+ * without full deserialization.
*/
public static OGCType getType(BytesWritable geomref) {
- // SIZE_WKID is the offset to the byte that stores the type information
- return OGCTypeLookup[geomref.getBytes()[SIZE_WKID]];
+ if (geomref == null || geomref.getLength() < 5) {
+ return OGCType.UNKNOWN;
+ }
+ byte[] bytes = geomref.getBytes();
+ if (bytes[4] == NEW_FORMAT_MAGIC) {
+ // New format: bytes 5+ are WKB. WKB layout: byte 5 = byte order,
+ // bytes 6-9 = geometry type int (in the indicated byte order).
+ if (geomref.getLength() < 10) {
+ return OGCType.UNKNOWN;
+ }
+ ByteOrder order = (bytes[5] == 1) ? ByteOrder.LITTLE_ENDIAN :
ByteOrder.BIG_ENDIAN;
+ int wkbType = ByteBuffer.wrap(bytes, 6, 4).order(order).getInt();
+ // JTS uses extended WKB: bit 31 (0x80000000) = Z, bit 30 (0x40000000) =
M,
+ // bit 29 (0x20000000) = SRID. Strip these flag bits to get the base
type.
+ // Also handle ISO WKB encoding (type + 1000/2000/3000) for
compatibility.
+ int baseType = wkbType & 0x1FFFFFFF; // strip Z/M/SRID flag bits
+ if (baseType >= 1000) {
+ baseType = baseType % 1000; // ISO WKB encoding
+ }
+ return switch (baseType) {
+ case 1 -> OGCType.ST_POINT;
+ case 2 -> OGCType.ST_LINESTRING;
+ case 3 -> OGCType.ST_POLYGON;
+ case 4 -> OGCType.ST_MULTIPOINT;
+ case 5 -> OGCType.ST_MULTILINESTRING;
+ case 6 -> OGCType.ST_MULTIPOLYGON;
+ default -> OGCType.UNKNOWN;
+ };
+ }
+ int typeIdx = bytes[SIZE_WKID] & 0xFF;
+ if (typeIdx >= OGCTypeLookup.length) {
+ return OGCType.UNKNOWN;
+ }
+ return OGCTypeLookup[typeIdx];
}
/**
- * Sets the geometry type (in place) for the given hive geometry bytes
- * @param geomref reference to hive geometry bytes
- * @param type OGC geometry type
+ * Infer OGCType from a JTS Geometry.
*/
- public static void setType(BytesWritable geomref, OGCType type) {
- geomref.getBytes()[SIZE_WKID] = (byte) type.getIndex();
+ public static OGCType inferOGCType(Geometry geom) {
+ if (geom == null) {
+ return OGCType.UNKNOWN;
+ }
+ return switch (geom.getGeometryType()) {
+ case "Point" -> OGCType.ST_POINT;
+ case "LineString", "LinearRing" -> OGCType.ST_LINESTRING;
+ case "Polygon" -> OGCType.ST_POLYGON;
+ case "MultiPoint" -> OGCType.ST_MULTIPOINT;
+ case "MultiLineString" -> OGCType.ST_MULTILINESTRING;
+ case "MultiPolygon" -> OGCType.ST_MULTIPOLYGON;
+ default -> OGCType.UNKNOWN;
+ };
}
/**
- * Gets the WKID for the given hive geometry bytes
- *
- * @param geomref reference to hive geometry bytes
- * @return WKID set in the first 4 bytes of the hive geometry bytes
+ * Gets the WKID/SRID for the given hive geometry bytes.
*/
public static int getWKID(BytesWritable geomref) {
- ByteBuffer bb = ByteBuffer.wrap(geomref.getBytes());
- return bb.getInt(0);
+ if (geomref == null || geomref.getLength() < 4) {
+ return WKID_UNKNOWN;
+ }
+ byte[] bytes = geomref.getBytes();
+ if (geomref.getLength() > 4 && bytes[4] == NEW_FORMAT_MAGIC) {
+ // New format: big-endian SRID
+ return ((bytes[0] & 0xFF) << 24) | ((bytes[1] & 0xFF) << 16)
Review Comment:
Good catch. I've unified it: the new format now stores the SRID
little-endian, matching the old format's WKID byte order. Since the SRID sits
at bytes 0–3 in both layouts and is now encoded the same way, getWKID/setWKID
no longer branch on the format
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
