ayushtkn commented on code in PR #6581:
URL: https://github.com/apache/hive/pull/6581#discussion_r3634054767
##########
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
Review Comment:
You're right that the raw geometry column carries a Hive-internal prefix
(SRID | 0xFF | WKB) and isn't directly portable, but that's pre-existing, not
new here: the previous ESRI-lib format stored `WKID | type | ESRI-shape` in the
same `BINARY` column. If anything this improves the situation, since your
strip-first-5-bytes workaround now yields standard WKB rather than ESRI
shapefile bytes. For portable output the intended paths remain ST_AsBinary
(WKB) and ST_AsGeoJson. On EWKB agreed it's attractive because the SRID
travels inside the WKB, but as you noted it's hard to disambiguate from the old
ESRI bytes on read, which is why I went with an explicit magic-byte prefix that
cleanly separates old vs new.
--
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]
