csringhofer commented on code in PR #6581:
URL: https://github.com/apache/hive/pull/6581#discussion_r3638435461
##########
ql/src/java/org/apache/hadoop/hive/ql/udf/esri/GeometryUtils.java:
##########
@@ -17,29 +17,236 @@
*/
package org.apache.hadoop.hive.ql.udf.esri;
-import com.esri.core.geometry.Geometry;
-import com.esri.core.geometry.GeometryEngine;
-import com.esri.core.geometry.MapGeometry;
-import com.esri.core.geometry.OperatorImportFromESRIShape;
-import com.esri.core.geometry.Polygon;
-import com.esri.core.geometry.SpatialReference;
-import com.esri.core.geometry.ogc.OGCGeometry;
import com.google.common.cache.Cache;
import com.google.common.cache.CacheBuilder;
import
org.apache.hadoop.hive.serde2.objectinspector.primitive.PrimitiveObjectInspectorFactory;
import
org.apache.hadoop.hive.serde2.objectinspector.primitive.WritableBinaryObjectInspector;
import org.apache.hadoop.io.BytesWritable;
+import org.locationtech.jts.algorithm.Orientation;
+import org.locationtech.jts.geom.CoordinateSequence;
+import org.locationtech.jts.geom.Geometry;
+import org.locationtech.jts.geom.GeometryCollection;
+import org.locationtech.jts.geom.GeometryFactory;
+import org.locationtech.jts.geom.LineString;
+import org.locationtech.jts.geom.LinearRing;
+import org.locationtech.jts.geom.MultiPolygon;
+import org.locationtech.jts.geom.Point;
+import org.locationtech.jts.geom.Polygon;
+import org.locationtech.jts.io.Ordinate;
+import org.locationtech.jts.io.ParseException;
+import org.locationtech.jts.io.WKBReader;
+import org.locationtech.jts.io.WKBWriter;
+import org.locationtech.jts.io.WKTReader;
+import org.locationtech.jts.io.WKTWriter;
+import org.locationtech.jts.io.geojson.GeoJsonReader;
+import org.locationtech.jts.io.geojson.GeoJsonWriter;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
import java.nio.ByteBuffer;
import java.nio.ByteOrder;
+import java.util.Arrays;
+import java.util.Set;
public class GeometryUtils {
+ private static final Logger LOG =
LoggerFactory.getLogger(GeometryUtils.class);
+
private static final int SIZE_WKID = 4;
private static final int SIZE_TYPE = 1;
public static final int WKID_UNKNOWN = 0;
+ // Magic byte at offset 4 to distinguish new WKB format from old ESRI format.
Review Comment:
>and since the old format was already Hive-specific
Impala and Trino can read it.
https://github.com/trinodb/trino/blob/34bd248c7430256f69d9969f06bd03fa8bba852c/plugin/trino-geospatial/src/main/java/io/trino/plugin/geospatial/GeoFunctions.java#L605
Probably others too - anyone who can run the Hive UDFs from geospatial
framework for Hadoop, e.g. Spark. There may be more UDFs that use it.
Adding GEOMETRY support may make sense even without Iceberg integration -
you could read existing BINARY data with st_geomFromShape(), and treat it as
GEOMETRY from that point. It could be a huge perf improvement if jts Geometry
objects could be passed with GEOMETRY instead of always serializing +
deserializing them between functions (I have no clue how hard is this to
Implement in Hive - I haven't implemented it in Impala yet).
For the current change: would it be hard to split it in two and keep the
existing serialization format while switching to JTS? The only missing piece I
see is writing M values to Esri format. It also doesn't seem inherently slower
to parse Esri shape for JTS geometries compared to WKB. Without the
serialization format change the behavior differences would be minor, similarly
to the Trino PR, so 90% of this PR could be merged without much thinking.
--
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]