abstractdog commented on code in PR #6581:
URL: https://github.com/apache/hive/pull/6581#discussion_r3613494315
##########
ql/src/test/results/clientpositive/llap/geospatial_udfs.q.out:
##########
@@ -94,30 +94,30 @@ order by id
POSTHOOK: type: QUERY
POSTHOOK: Input: default@geom_binary
#### A masked pattern was here ####
-1 MULTIPOLYGON (((0 0, 1 0, 0 1, 0 0)), ((2 2, 3 2, 2 3, 2 2)))
ST_MULTIPOLYGON 3.0 3.0 NULL 0.0 0.0 NULL 2 POINT
(1.3333333333333333 1.3333333333333333) 2 false false true
-2 MULTILINESTRING ((2 4, 10 10), (20 20, 7 8)) ST_MULTILINESTRING
20.0 20.0 NULL 2.0 4.0 NULL 2 POINT
(10.791617601072488 11.472176427667655) 1 false false true
+1 MULTIPOLYGON (((0 0, 1 0, 0 1, 0 0)), ((2 2, 3 2, 2 3, 2 2)))
ST_MULTIPOLYGON 3.0 3.0 NULL 0.0 0.0 NULL 2 POINT
(1.333333333333 1.333333333333) 2 false false true
Review Comment:
let's make a comment about this for clarity: seems like a change in decimal
precision, which I guess was expected by the library change
double-checked: `10.791617601072488` -> `10.791617601072` means a difference
less then a micrometer, far below GPS accuracy, so looks okay to me :)
UPDATE: I just realized the `qt:replace`, feel free to ignore this comment
##########
serde/src/java/org/apache/hadoop/hive/serde2/esriJson/deserializer/GeometryTypeJsonDeserializer.java:
##########
@@ -27,32 +26,26 @@
/**
*
- * Deserializes a JSON geometry type enumeration into a Geometry.Type.*
enumeration
+ * Deserializes a JSON geometry type string (e.g. "esriGeometryPolygon") into
a String.
+ * The raw Esri geometry type string is preserved as-is for downstream use.
*/
-public class GeometryTypeJsonDeserializer extends
JsonDeserializer<Geometry.Type> {
+public class GeometryTypeJsonDeserializer extends JsonDeserializer<String> {
public GeometryTypeJsonDeserializer() {
}
@Override
- public Geometry.Type deserialize(JsonParser parser, DeserializationContext
arg1)
+ public String deserialize(JsonParser parser, DeserializationContext arg1)
throws IOException, JsonProcessingException {
-
String type_text = parser.getText();
-
- // geometry type enumerations coming from the JSON are prepended with
"esriGeometry" (i.e. esriGeometryPolygon)
- // while the geometry-java-api uses the form Geometry.Type.Polygon
- if (type_text.startsWith("esriGeometry")) {
- // cut out esriGeometry to match Geometry.Type enumeration values
- type_text = type_text.substring(12);
-
- try {
- return Enum.valueOf(Geometry.Type.class, type_text);
- } catch (Exception e) {
- // parsing failed, fall through to unknown geometry type
- }
+ if (type_text == null) {
+ return "esriGeometryUnknown";
}
-
- return Geometry.Type.Unknown;
+ // Preserve the raw esriGeometry* string (e.g. "esriGeometryPolygon").
+ // If the value is not already prefixed, normalise it.
+ if (!type_text.startsWith("esriGeometry")) {
Review Comment:
`"esriGeometry"` is used in serializer and deserializer, I think it's a good
candidate for a public constant
maybe the same applies to "esriGeometryUnknown", because it has a special
meaning I guess
##########
serde/src/java/org/apache/hadoop/hive/serde2/esriJson/deserializer/GeometryJsonDeserializer.java:
##########
@@ -17,27 +17,51 @@
*/
package org.apache.hadoop.hive.serde2.esriJson.deserializer;
-import com.esri.core.geometry.Geometry;
import com.esri.core.geometry.GeometryEngine;
+import com.esri.core.geometry.MapGeometry;
+import com.esri.core.geometry.ogc.OGCGeometry;
import com.fasterxml.jackson.core.JsonParser;
import com.fasterxml.jackson.core.JsonProcessingException;
import com.fasterxml.jackson.databind.DeserializationContext;
import com.fasterxml.jackson.databind.JsonDeserializer;
+import org.locationtech.jts.geom.Geometry;
+import org.locationtech.jts.geom.GeometryFactory;
+import org.locationtech.jts.io.WKBReader;
import java.io.IOException;
/**
*
- * Deserializes a JSON geometry definition into a Geometry instance
+ * Deserializes a JSON geometry definition into a JTS Geometry instance.
+ * Uses the ESRI API to parse Esri JSON, then converts to JTS via WKB
round-trip.
*/
public class GeometryJsonDeserializer extends JsonDeserializer<Geometry> {
+ private static final GeometryFactory GEOMETRY_FACTORY = new
GeometryFactory();
+
public GeometryJsonDeserializer() {
}
@Override
public Geometry deserialize(JsonParser arg0, DeserializationContext arg1)
throws IOException, JsonProcessingException {
- return GeometryEngine.jsonToGeometry(arg0).getGeometry();
+ try {
+ MapGeometry mapGeom = GeometryEngine.jsonToGeometry(arg0);
+ if (mapGeom == null || mapGeom.getGeometry() == null) {
+ return null;
+ }
+ OGCGeometry ogcGeom = OGCGeometry.createFromEsriGeometry(
+ mapGeom.getGeometry(), mapGeom.getSpatialReference());
+ java.nio.ByteBuffer wkbBuf = ogcGeom.asBinary();
+ byte[] wkbBytes = new byte[wkbBuf.remaining()];
+ wkbBuf.get(wkbBytes);
+ Geometry jtsGeom = new WKBReader(GEOMETRY_FACTORY).read(wkbBytes);
+ if (mapGeom.getSpatialReference() != null) {
+ jtsGeom.setSRID(mapGeom.getSpatialReference().getID());
+ }
+ return jtsGeom;
+ } catch (Exception e) {
+ return null;
+ }
Review Comment:
hm, how much is this expected? is it worth a `LOG.debug` at least?
##########
serde/src/java/org/apache/hadoop/hive/serde2/esriJson/serializer/GeometryJsonSerializer.java:
##########
@@ -17,19 +17,32 @@
*/
package org.apache.hadoop.hive.serde2.esriJson.serializer;
-import com.esri.core.geometry.Geometry;
import com.esri.core.geometry.GeometryEngine;
+import com.esri.core.geometry.ogc.OGCGeometry;
import com.fasterxml.jackson.core.JsonGenerator;
import com.fasterxml.jackson.databind.JsonSerializer;
import com.fasterxml.jackson.databind.SerializerProvider;
+import org.locationtech.jts.geom.Geometry;
+import org.locationtech.jts.io.WKBWriter;
import java.io.IOException;
+/**
+ * Serializes a JTS Geometry to Esri JSON format.
+ * Converts JTS -> ESRI via WKB round-trip, then uses
GeometryEngine.geometryToJson().
+ */
public class GeometryJsonSerializer extends JsonSerializer<Geometry> {
@Override
public void serialize(Geometry geometry, JsonGenerator jsonGenerator,
SerializerProvider arg2) throws IOException {
-
- jsonGenerator.writeRawValue(GeometryEngine.geometryToJson(null, geometry));
+ try {
+ byte[] wkb = new WKBWriter().write(geometry);
+ OGCGeometry ogcGeom =
OGCGeometry.fromBinary(java.nio.ByteBuffer.wrap(wkb));
+ com.esri.core.geometry.Geometry esriGeom = ogcGeom.getEsriGeometry();
+ int wkid = geometry.getSRID();
Review Comment:
do we need the extra integer declaration?
##########
serde/src/java/org/apache/hadoop/hive/serde2/esriJson/serializer/GeometryJsonSerializer.java:
##########
@@ -17,19 +17,32 @@
*/
package org.apache.hadoop.hive.serde2.esriJson.serializer;
-import com.esri.core.geometry.Geometry;
import com.esri.core.geometry.GeometryEngine;
+import com.esri.core.geometry.ogc.OGCGeometry;
import com.fasterxml.jackson.core.JsonGenerator;
import com.fasterxml.jackson.databind.JsonSerializer;
import com.fasterxml.jackson.databind.SerializerProvider;
+import org.locationtech.jts.geom.Geometry;
+import org.locationtech.jts.io.WKBWriter;
import java.io.IOException;
+/**
+ * Serializes a JTS Geometry to Esri JSON format.
+ * Converts JTS -> ESRI via WKB round-trip, then uses
GeometryEngine.geometryToJson().
+ */
public class GeometryJsonSerializer extends JsonSerializer<Geometry> {
@Override
public void serialize(Geometry geometry, JsonGenerator jsonGenerator,
SerializerProvider arg2) throws IOException {
-
- jsonGenerator.writeRawValue(GeometryEngine.geometryToJson(null, geometry));
+ try {
+ byte[] wkb = new WKBWriter().write(geometry);
Review Comment:
How hot is this codepath? My first impression by looking at `new
WKBWriter()` was something like, "this is going to be expensive", even if it's
a lightweight object...I know it might bring thread safety issues depending on
the callpath, but in general, I would consider avoiding any extra object
creation on a per-row basis, let me know what do you think
UPDATE: I just realized there are the corresponding ThreadLocal stuff in
`GeometryUtils`, which looks reasonable here too
##########
ql/src/java/org/apache/hadoop/hive/ql/udf/esri/serde/EsriJsonSerDe.java:
##########
@@ -17,147 +17,35 @@
*/
package org.apache.hadoop.hive.ql.udf.esri.serde;
-import com.esri.core.geometry.Geometry;
-import com.esri.core.geometry.JsonGeometryException;
-import com.esri.core.geometry.JsonReader;
-import com.esri.core.geometry.MapGeometry;
-import com.esri.core.geometry.OperatorImportFromJson;
-import com.esri.core.geometry.ogc.OGCGeometry;
-import com.fasterxml.jackson.core.JsonFactory;
import com.fasterxml.jackson.core.JsonParser;
-import com.fasterxml.jackson.core.JsonToken;
+import com.fasterxml.jackson.databind.JsonNode;
+import com.fasterxml.jackson.databind.ObjectMapper;
+import org.apache.hadoop.hive.ql.udf.esri.EsriJsonConverter;
+import org.locationtech.jts.geom.Geometry;
public class EsriJsonSerDe extends BaseJsonSerDe {
@Override
- protected String outGeom(OGCGeometry geom) {
- return geom.asJson();
- }
-
- @Override
- protected OGCGeometry parseGeom(JsonParser parser) {
- MapGeometry mapGeom =
- OperatorImportFromJson.local().execute(Geometry.Type.Unknown, new
HiveJsonParserReader(parser));
- return OGCGeometry.createFromEsriGeometry(mapGeom.getGeometry(),
mapGeom.getSpatialReference());
- }
-}
-
-class HiveJsonParserReader implements JsonReader {
- private JsonParser m_jsonParser;
-
- public HiveJsonParserReader(JsonParser jsonParser) {
- this.m_jsonParser = jsonParser;
- }
-
- public static JsonReader createFromString(String str) {
+ protected String outGeom(Geometry geom) {
try {
- JsonFactory factory = new JsonFactory();
- JsonParser jsonParser = factory.createParser(str);
- jsonParser.nextToken();
- return new com.esri.core.geometry.JsonParserReader(jsonParser);
- } catch (Exception var3) {
- throw new JsonGeometryException(var3.getMessage());
+ int wkid = geom.getSRID();
Review Comment:
do we need the integer declaration here?
##########
ql/src/java/org/apache/hadoop/hive/ql/udf/esri/serde/EsriJsonSerDe.java:
##########
@@ -17,147 +17,35 @@
*/
package org.apache.hadoop.hive.ql.udf.esri.serde;
-import com.esri.core.geometry.Geometry;
-import com.esri.core.geometry.JsonGeometryException;
-import com.esri.core.geometry.JsonReader;
-import com.esri.core.geometry.MapGeometry;
-import com.esri.core.geometry.OperatorImportFromJson;
-import com.esri.core.geometry.ogc.OGCGeometry;
-import com.fasterxml.jackson.core.JsonFactory;
import com.fasterxml.jackson.core.JsonParser;
-import com.fasterxml.jackson.core.JsonToken;
+import com.fasterxml.jackson.databind.JsonNode;
+import com.fasterxml.jackson.databind.ObjectMapper;
+import org.apache.hadoop.hive.ql.udf.esri.EsriJsonConverter;
+import org.locationtech.jts.geom.Geometry;
public class EsriJsonSerDe extends BaseJsonSerDe {
@Override
- protected String outGeom(OGCGeometry geom) {
- return geom.asJson();
- }
-
- @Override
- protected OGCGeometry parseGeom(JsonParser parser) {
- MapGeometry mapGeom =
- OperatorImportFromJson.local().execute(Geometry.Type.Unknown, new
HiveJsonParserReader(parser));
- return OGCGeometry.createFromEsriGeometry(mapGeom.getGeometry(),
mapGeom.getSpatialReference());
- }
-}
-
-class HiveJsonParserReader implements JsonReader {
- private JsonParser m_jsonParser;
-
- public HiveJsonParserReader(JsonParser jsonParser) {
- this.m_jsonParser = jsonParser;
- }
-
- public static JsonReader createFromString(String str) {
+ protected String outGeom(Geometry geom) {
try {
- JsonFactory factory = new JsonFactory();
- JsonParser jsonParser = factory.createParser(str);
- jsonParser.nextToken();
- return new com.esri.core.geometry.JsonParserReader(jsonParser);
- } catch (Exception var3) {
- throw new JsonGeometryException(var3.getMessage());
+ int wkid = geom.getSRID();
+ return EsriJsonConverter.geometryToEsriJson(geom, wkid);
+ } catch (Exception e) {
+ return "null";
Review Comment:
maybe refactor to a constant having a descriptive name about the scenario
where "null" is expected
also, original method threw `JsonGeometryException`, current one returns
"null" string, so it's worth a javadoc
##########
ql/src/java/org/apache/hadoop/hive/ql/udf/esri/serde/GeoJsonSerDe.java:
##########
Review Comment:
I know it's not related to the patch, but can we improve this? this direct
assignment looks strange to me, I would either a) make `attrLabel` final and
take care of the assignment in the parent constructor as well, or b) introduce
a protected setter
##########
ql/src/java/org/apache/hadoop/hive/ql/udf/esri/serde/GeoJsonSerDe.java:
##########
@@ -36,20 +38,18 @@ public GeoJsonSerDe() {
}
@Override
- protected String outGeom(OGCGeometry geom) {
- return geom.asGeoJson();
+ protected String outGeom(Geometry geom) {
+ return GeometryUtils.geoJsonWriter().write(geom);
}
@Override
- protected OGCGeometry parseGeom(JsonParser parser) {
+ protected Geometry parseGeom(JsonParser parser) {
try {
ObjectNode node = mapper.readTree(parser);
- return OGCGeometry.fromGeoJson(node.toString());
- } catch (JsonProcessingException e1) {
- e1.printStackTrace(); // TODO Auto-generated catch block
- } catch (IOException e1) {
- e1.printStackTrace(); // TODO Auto-generated catch block
+ return GeometryUtils.geoJsonReader().read(node.toString());
+ } catch (ParseException | IOException e1) {
Review Comment:
could this be `e` instead of `e1`?
##########
ql/src/java/org/apache/hadoop/hive/ql/udf/esri/ST_Aggr_ConvexHull.java:
##########
@@ -92,13 +85,22 @@ public boolean merge(BytesWritable other) throws
HiveException {
}
public BytesWritable terminatePartial() throws HiveException {
- maybeAggregateBuffer(true);
- if (geometries.size() == 1) {
- OGCGeometry rslt =
OGCGeometry.createFromEsriGeometry(geometries.get(0), spatialRef);
- return GeometryUtils.geometryToEsriShapeBytesWritable(rslt);
- } else {
+ if (geometries.isEmpty()) {
return null;
}
+ try {
+ GeometryCollection collection = GeometryUtils.GEOMETRY_FACTORY
+ .createGeometryCollection(geometries.toArray(new Geometry[0]));
+ Geometry result = collection.convexHull();
+ int wkid = (firstWKID == -2) ? GeometryUtils.WKID_UNKNOWN : firstWKID;
+ return GeometryUtils.geometryToEsriShapeBytesWritable(result, wkid);
+ } catch (Exception e) {
+ LOG.error("ST_Aggr_ConvexHull failed", e);
Review Comment:
some of the udf classes use LogUtils, like `LogUtils.Log_InternalError(LOG,
"ST_IsEmpty" + e);`, should it be adopted here too?
##########
ql/src/java/org/apache/hadoop/hive/ql/udf/esri/EsriShapeConverter.java:
##########
@@ -0,0 +1,588 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.hadoop.hive.ql.udf.esri;
+
+import org.locationtech.jts.algorithm.Orientation;
+import org.locationtech.jts.geom.Coordinate;
+import org.locationtech.jts.geom.Geometry;
+import org.locationtech.jts.geom.GeometryCollection;
+import org.locationtech.jts.geom.LinearRing;
+import org.locationtech.jts.geom.LineString;
+import org.locationtech.jts.geom.MultiLineString;
+import org.locationtech.jts.geom.MultiPoint;
+import org.locationtech.jts.geom.MultiPolygon;
+import org.locationtech.jts.geom.Point;
+import org.locationtech.jts.geom.Polygon;
+
+import java.nio.ByteBuffer;
+import java.nio.ByteOrder;
+import java.util.ArrayList;
+import java.util.List;
+
+/**
+ * Reads and writes raw ESRI shape binary format directly from/to JTS
geometries,
+ * without any dependency on the ESRI Geometry library.
+ *
+ * <p>All binary values are little-endian as per the ESRI shape format
specification.
+ *
+ * <p>Supported shape types:
+ * <ul>
+ * <li>0 - Null/Unknown</li>
+ * <li>1 - Point</li>
+ * <li>3 - Polyline (paths)</li>
+ * <li>5 - Polygon (rings)</li>
+ * <li>8 - MultiPoint</li>
+ * <li>11 - PointZ</li>
+ * <li>13 - PolylineZ</li>
+ * <li>15 - PolygonZ</li>
+ * <li>18 - MultiPointZ</li>
+ * </ul>
+ */
+public final class EsriShapeConverter {
Review Comment:
we need unit test coverage for this class too I believe
##########
ql/src/java/org/apache/hadoop/hive/ql/udf/esri/ST_AsText.java:
##########
@@ -60,40 +57,21 @@ public Text evaluate(BytesWritable geomref) {
return null;
}
- OGCGeometry ogcGeometry = GeometryUtils.geometryFromEsriShape(geomref);
- if (ogcGeometry == null) {
+ Geometry geom = GeometryUtils.geometryFromEsriShape(geomref);
+ if (geom == null) {
LogUtils.Log_ArgumentsNull(LOG);
return null;
}
- int wktExportFlag = getWktExportFlag(GeometryUtils.getType(geomref));
-
try {
- // mind: GeometryType with ST_AsText(ST_GeomFromText('MultiLineString((0
80, 0.03 80.04))'))
- // return new Text(ogcGeometry.asText());
- return new
Text(GeometryEngine.geometryToWkt(ogcGeometry.getEsriGeometry(),
wktExportFlag));
+ String wkt = GeometryUtils.wktWriterFor(geom).write(geom);
+ // JTS 1.20 WKTWriter omits the space between dimension qualifier and
'(' (e.g. "POINT Z(").
+ // ISO WKT requires "POINT Z (" with a space. Fix by inserting a space.
+ wkt = wkt.replace(" ZM(", " ZM (").replace(" Z(", " Z (").replace(" M(",
" M (");
+ return new Text(wkt);
} catch (Exception e) {
LOG.error(e.getMessage());
Review Comment:
maybe utilize `LogUtils.Log_InternalError` here too?
##########
ql/src/java/org/apache/hadoop/hive/ql/udf/esri/EsriJsonConverter.java:
##########
@@ -0,0 +1,482 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.hadoop.hive.ql.udf.esri;
+
+import com.fasterxml.jackson.databind.JsonNode;
+import com.fasterxml.jackson.databind.ObjectMapper;
+import org.locationtech.jts.algorithm.Orientation;
+import org.locationtech.jts.geom.Coordinate;
+import org.locationtech.jts.geom.Geometry;
+import org.locationtech.jts.geom.LineString;
+import org.locationtech.jts.geom.LinearRing;
+import org.locationtech.jts.geom.MultiLineString;
+import org.locationtech.jts.geom.MultiPoint;
+import org.locationtech.jts.geom.MultiPolygon;
+import org.locationtech.jts.geom.Point;
+import org.locationtech.jts.geom.Polygon;
+
+import java.io.IOException;
+import java.util.ArrayList;
+import java.util.Arrays;
+import java.util.List;
+
+/**
+ * Converts between JTS geometries and Esri JSON format without any dependency
+ * on the ESRI geometry library.
+ *
+ * <p>Esri JSON format reference:
+ * <ul>
+ * <li>Point: {@code
{"x":N,"y":N,"spatialReference":{"wkid":N}}}</li>
+ * <li>MultiPoint: {@code
{"points":[[x,y],...],"spatialReference":{"wkid":N}}}</li>
+ * <li>LineString: {@code
{"paths":[[[x,y],...]],"spatialReference":{"wkid":N}}}</li>
+ * <li>MultiLineString: {@code
{"paths":[[[x,y],...],...],"spatialReference":{"wkid":N}}}</li>
+ * <li>Polygon: {@code
{"rings":[[[x,y],...,[x,y]]],"spatialReference":{"wkid":N}}}</li>
+ * <li>MultiPolygon: {@code
{"rings":[[[x,y],...],...],"spatialReference":{"wkid":N}}}</li>
+ * </ul>
+ *
+ * <p>Winding-order convention: Esri requires exterior rings to be clockwise
(CW) and
+ * interior rings (holes) to be counter-clockwise (CCW) when viewed in screen
coordinates
+ * (y-axis pointing down). JTS follows the OGC convention, which is the
opposite.
+ * This converter checks and reverses rings as necessary using
+ * {@link Orientation#isCCW(Coordinate[])}.
+ */
+public final class EsriJsonConverter {
+
+ private static final ObjectMapper OBJECT_MAPPER = new ObjectMapper();
+
+ private EsriJsonConverter() {
+ }
+
+ /**
+ * Converts a JTS {@link Geometry} to its Esri JSON string representation.
+ *
+ * @param geom the JTS geometry to convert; must not be {@code null}
+ * @param wkid the Well-Known ID of the spatial reference (0 = omit
spatialReference)
+ * @return Esri JSON string
+ * @throws IllegalArgumentException if the geometry type is not supported
+ */
+ public static String geometryToEsriJson(Geometry geom, int wkid) {
Review Comment:
we need unit test coverage for this class I believe
##########
ql/src/java/org/apache/hadoop/hive/ql/udf/esri/ST_Is3D.java:
##########
@@ -61,13 +62,13 @@ public BooleanWritable evaluate(BytesWritable geomref) {
return null;
}
- OGCGeometry ogcGeometry = GeometryUtils.geometryFromEsriShape(geomref);
- if (ogcGeometry == null) {
+ Geometry geom = GeometryUtils.geometryFromEsriShape(geomref);
+ if (geom == null) {
LogUtils.Log_ArgumentsNull(LOG);
return null;
}
- resultBoolean.set(ogcGeometry.is3D());
+ resultBoolean.set(GeometryUtils.getOrdinates(geom).contains(Ordinate.Z));
Review Comment:
what about introducing `GeometryUtils.is3D(geom)`
##########
ql/src/java/org/apache/hadoop/hive/ql/udf/esri/ST_GeomFromJson.java:
##########
@@ -55,10 +58,13 @@ public Object evaluate(DeferredObject[] arguments) throws
HiveException {
}
try {
- OGCGeometry ogcGeom = OGCGeometry.fromJson(json);
- return GeometryUtils.geometryToEsriShapeBytesWritable(ogcGeom);
+ Geometry jtsGeom = EsriJsonConverter.esriJsonToGeometry(json);
+ if (jtsGeom == null) {
+ return null;
+ }
+ return GeometryUtils.geometryToEsriShapeBytesWritable(jtsGeom);
} catch (Exception e) {
-
+ LOG.error("Failed to parse Esri JSON", e);
Review Comment:
`LogUtils.Log_InternalError`
##########
ql/src/java/org/apache/hadoop/hive/ql/udf/esri/ST_GeomFromWKB.java:
##########
@@ -76,16 +74,12 @@ public BytesWritable evaluate(BytesWritable wkb) throws
UDFArgumentException {
public BytesWritable evaluate(BytesWritable wkb, int wkid) throws
UDFArgumentException {
try {
- SpatialReference spatialReference = null;
+ byte[] byteArr = Arrays.copyOf(wkb.getBytes(), wkb.getLength());
Review Comment:
I think we can assume that `wkbReader` doesn't alter the incoming byte array
(please double-check if the contract is as I expect) so copying looks like an
overhead, instead we can do `GeometryUtils.wkbReader().read(wkb.getBytes())`
if so, this copy can be removed from several udfs
##########
ql/src/java/org/apache/hadoop/hive/ql/udf/esri/ST_GeomFromWKB.java:
##########
@@ -76,16 +74,12 @@ public BytesWritable evaluate(BytesWritable wkb) throws
UDFArgumentException {
public BytesWritable evaluate(BytesWritable wkb, int wkid) throws
UDFArgumentException {
try {
- SpatialReference spatialReference = null;
+ byte[] byteArr = Arrays.copyOf(wkb.getBytes(), wkb.getLength());
+ Geometry geom = GeometryUtils.wkbReader().read(byteArr);
if (wkid != GeometryUtils.WKID_UNKNOWN) {
- spatialReference = SpatialReference.create(wkid);
+ geom.setSRID(wkid);
}
- byte[] byteArr = wkb.getBytes();
- ByteBuffer byteBuf = ByteBuffer.allocate(byteArr.length);
- byteBuf.put(byteArr);
- OGCGeometry ogcObj = OGCGeometry.fromBinary(byteBuf);
- ogcObj.setSpatialReference(spatialReference);
- return GeometryUtils.geometryToEsriShapeBytesWritable(ogcObj);
+ return GeometryUtils.geometryToEsriShapeBytesWritable(geom);
} catch (Exception e) { // IllegalArgumentException, GeometryException
LOG.error(e.getMessage());
Review Comment:
LogUtils.Log_InternalError
##########
ql/src/java/org/apache/hadoop/hive/ql/udf/esri/GeometryUtils.java:
##########
@@ -17,29 +17,211 @@
*/
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.
+ // Old format stores OGC type tag (0-6) at byte 4; 0xFF is outside that
range.
+ public static final byte NEW_FORMAT_MAGIC = (byte) 0xFF;
+
+ public static final GeometryFactory GEOMETRY_FACTORY = new GeometryFactory();
+
+ // ThreadLocal JTS readers/writers to avoid per-row allocation overhead.
+ // These classes are not thread-safe, so ThreadLocal provides safe reuse
+ // across millions of rows within the same task thread.
+ private static final ThreadLocal<WKBReader> WKB_READER =
+ ThreadLocal.withInitial(() -> new WKBReader(GEOMETRY_FACTORY));
+ private static final ThreadLocal<WKBWriter> WKB_WRITER =
+ ThreadLocal.withInitial(WKBWriter::new);
+ private static final ThreadLocal<WKBWriter> WKB_WRITER_3D =
+ ThreadLocal.withInitial(() -> new WKBWriter(3));
+ private static final ThreadLocal<WKBWriter> WKB_WRITER_XYM =
ThreadLocal.withInitial(() -> {
+ WKBWriter w = new WKBWriter(3);
+ w.setOutputOrdinates(Ordinate.createXYM());
+ return w;
+ });
+ private static final ThreadLocal<WKBWriter> WKB_WRITER_4D =
+ ThreadLocal.withInitial(() -> new WKBWriter(4));
+ private static final ThreadLocal<WKTReader> WKT_READER =
+ ThreadLocal.withInitial(() -> new WKTReader(GEOMETRY_FACTORY));
+ private static final ThreadLocal<WKTWriter> WKT_WRITER =
+ ThreadLocal.withInitial(WKTWriter::new);
+ private static final ThreadLocal<WKTWriter> WKT_WRITER_3D =
+ ThreadLocal.withInitial(() -> new WKTWriter(3));
+ private static final ThreadLocal<WKTWriter> WKT_WRITER_XYM =
ThreadLocal.withInitial(() -> {
+ WKTWriter w = new WKTWriter(3);
+ w.setOutputOrdinates(Ordinate.createXYM());
+ return w;
+ });
+ private static final ThreadLocal<WKTWriter> WKT_WRITER_4D =
+ ThreadLocal.withInitial(() -> new WKTWriter(4));
+ private static final ThreadLocal<GeoJsonReader> GEOJSON_READER =
+ ThreadLocal.withInitial(GeoJsonReader::new);
+ private static final ThreadLocal<GeoJsonWriter> GEOJSON_WRITER =
+ ThreadLocal.withInitial(GeoJsonWriter::new);
+
+ /** Get the thread-local WKBReader instance (reused across rows). */
+ public static WKBReader wkbReader() {
+ return WKB_READER.get();
+ }
+
+ /** Get the thread-local WKBWriter instance (reused across rows). */
+ public static WKBWriter wkbWriter() {
+ return WKB_WRITER.get();
+ }
+
+ /** Get the thread-local WKTReader instance (reused across rows). */
+ public static WKTReader wktReader() {
+ return WKT_READER.get();
+ }
+
+ /** Get the thread-local WKTWriter instance (reused across rows). */
+ public static WKTWriter wktWriter() {
+ return WKT_WRITER.get();
+ }
+
+ /** Get a dimension-aware WKBWriter that matches the geometry's coordinate
type. */
+ public static WKBWriter wkbWriterFor(Geometry geom) {
+ Set<Ordinate> ordinates = getOrdinates(geom);
+ if (ordinates.contains(Ordinate.Z) && ordinates.contains(Ordinate.M)) {
Review Comment:
what about `GeometryUtils.is4D(ordinates)`?
##########
ql/src/java/org/apache/hadoop/hive/ql/udf/esri/ST_MinM.java:
##########
@@ -63,17 +64,26 @@ public DoubleWritable evaluate(BytesWritable geomref) {
return null;
}
- OGCGeometry ogcGeometry = GeometryUtils.geometryFromEsriShape(geomref);
- if (ogcGeometry == null) {
+ Geometry geom = GeometryUtils.geometryFromEsriShape(geomref);
+ if (geom == null) {
LogUtils.Log_ArgumentsNull(LOG);
return null;
}
- if (!ogcGeometry.isMeasured()) {
Review Comment:
is there a corresponding quick `isMeasured` filter in JTS to prevent
iterating through all the points?
##########
ql/src/java/org/apache/hadoop/hive/ql/udf/esri/ST_IsMeasured.java:
##########
@@ -61,13 +62,13 @@ public BooleanWritable evaluate(BytesWritable geomref) {
return null;
}
- OGCGeometry ogcGeometry = GeometryUtils.geometryFromEsriShape(geomref);
- if (ogcGeometry == null) {
+ Geometry geom = GeometryUtils.geometryFromEsriShape(geomref);
+ if (geom == null) {
LogUtils.Log_ArgumentsNull(LOG);
return null;
}
- resultBoolean.set(ogcGeometry.isMeasured());
+ resultBoolean.set(GeometryUtils.getOrdinates(geom).contains(Ordinate.M));
Review Comment:
`GeometryUtils.isMeasured(geom)`
##########
ql/src/java/org/apache/hadoop/hive/ql/udf/esri/ST_MaxZ.java:
##########
@@ -63,17 +64,26 @@ public DoubleWritable evaluate(BytesWritable geomref) {
return null;
}
- OGCGeometry ogcGeometry = GeometryUtils.geometryFromEsriShape(geomref);
- if (ogcGeometry == null) {
+ Geometry geom = GeometryUtils.geometryFromEsriShape(geomref);
+ if (geom == null) {
LogUtils.Log_ArgumentsNull(LOG);
return null;
}
- if (!ogcGeometry.is3D()) {
Review Comment:
is there a corresponding quick `is3D` filter in JTS to prevent iterating
through all the 2D points?
##########
ql/src/java/org/apache/hadoop/hive/ql/udf/esri/GeometryUtils.java:
##########
@@ -17,29 +17,211 @@
*/
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.
+ // Old format stores OGC type tag (0-6) at byte 4; 0xFF is outside that
range.
+ public static final byte NEW_FORMAT_MAGIC = (byte) 0xFF;
+
+ public static final GeometryFactory GEOMETRY_FACTORY = new GeometryFactory();
+
+ // ThreadLocal JTS readers/writers to avoid per-row allocation overhead.
+ // These classes are not thread-safe, so ThreadLocal provides safe reuse
+ // across millions of rows within the same task thread.
+ private static final ThreadLocal<WKBReader> WKB_READER =
+ ThreadLocal.withInitial(() -> new WKBReader(GEOMETRY_FACTORY));
+ private static final ThreadLocal<WKBWriter> WKB_WRITER =
+ ThreadLocal.withInitial(WKBWriter::new);
+ private static final ThreadLocal<WKBWriter> WKB_WRITER_3D =
+ ThreadLocal.withInitial(() -> new WKBWriter(3));
+ private static final ThreadLocal<WKBWriter> WKB_WRITER_XYM =
ThreadLocal.withInitial(() -> {
+ WKBWriter w = new WKBWriter(3);
+ w.setOutputOrdinates(Ordinate.createXYM());
+ return w;
+ });
+ private static final ThreadLocal<WKBWriter> WKB_WRITER_4D =
+ ThreadLocal.withInitial(() -> new WKBWriter(4));
+ private static final ThreadLocal<WKTReader> WKT_READER =
+ ThreadLocal.withInitial(() -> new WKTReader(GEOMETRY_FACTORY));
+ private static final ThreadLocal<WKTWriter> WKT_WRITER =
+ ThreadLocal.withInitial(WKTWriter::new);
+ private static final ThreadLocal<WKTWriter> WKT_WRITER_3D =
+ ThreadLocal.withInitial(() -> new WKTWriter(3));
+ private static final ThreadLocal<WKTWriter> WKT_WRITER_XYM =
ThreadLocal.withInitial(() -> {
+ WKTWriter w = new WKTWriter(3);
+ w.setOutputOrdinates(Ordinate.createXYM());
+ return w;
+ });
+ private static final ThreadLocal<WKTWriter> WKT_WRITER_4D =
+ ThreadLocal.withInitial(() -> new WKTWriter(4));
+ private static final ThreadLocal<GeoJsonReader> GEOJSON_READER =
+ ThreadLocal.withInitial(GeoJsonReader::new);
+ private static final ThreadLocal<GeoJsonWriter> GEOJSON_WRITER =
+ ThreadLocal.withInitial(GeoJsonWriter::new);
+
+ /** Get the thread-local WKBReader instance (reused across rows). */
+ public static WKBReader wkbReader() {
+ return WKB_READER.get();
+ }
+
+ /** Get the thread-local WKBWriter instance (reused across rows). */
+ public static WKBWriter wkbWriter() {
+ return WKB_WRITER.get();
+ }
+
+ /** Get the thread-local WKTReader instance (reused across rows). */
+ public static WKTReader wktReader() {
+ return WKT_READER.get();
+ }
+
+ /** Get the thread-local WKTWriter instance (reused across rows). */
+ public static WKTWriter wktWriter() {
+ return WKT_WRITER.get();
+ }
+
+ /** Get a dimension-aware WKBWriter that matches the geometry's coordinate
type. */
+ public static WKBWriter wkbWriterFor(Geometry geom) {
+ Set<Ordinate> ordinates = getOrdinates(geom);
+ if (ordinates.contains(Ordinate.Z) && ordinates.contains(Ordinate.M)) {
+ return WKB_WRITER_4D.get();
+ } else if (ordinates.contains(Ordinate.Z)) {
Review Comment:
what about `GeometryUtils.is3D(ordinates)`?
##########
ql/src/java/org/apache/hadoop/hive/ql/udf/esri/ST_NumGeometries.java:
##########
@@ -61,18 +58,10 @@ public IntWritable evaluate(BytesWritable geomref) {
case ST_POLYGON:
LogUtils.Log_InvalidType(LOG, GeometryUtils.OGCType.ST_MULTIPOLYGON,
ogcType);
Review Comment:
if these cases remain, shouldn't they simply return `1`?
##########
ql/src/test/org/apache/hadoop/hive/ql/udf/esri/serde/JsonSerDeTestingBase.java:
##########
@@ -100,19 +98,16 @@ protected void addWritable(ArrayList<Object> stuff,
java.sql.Timestamp item) {
}
protected void addWritable(ArrayList<Object> stuff, Geometry geom) {
- addWritable(stuff, geom, null);
- }
-
- protected void addWritable(ArrayList<Object> stuff, MapGeometry geom) {
- addWritable(stuff, geom.getGeometry(), geom.getSpatialReference());
- }
-
- protected void addWritable(ArrayList<Object> stuff, Geometry geom,
SpatialReference sref) {
-
stuff.add(GeometryUtils.geometryToEsriShapeBytesWritable(OGCGeometry.createFromEsriGeometry(geom,
sref)));
+ stuff.add(GeometryUtils.geometryToEsriShapeBytesWritable(geom));
}
protected void ckPoint(Point refPt, BytesWritable fieldData) {
- Assert.assertEquals(refPt,
GeometryUtils.geometryFromEsriShape(fieldData).getEsriGeometry());
+ Geometry geom = GeometryUtils.geometryFromEsriShape(fieldData);
+ Assert.assertNotNull("Geometry must not be null!", geom);
+ Assert.assertTrue("Expected a Point geometry!", geom instanceof Point);
+ Point actualPt = (Point) geom;
+ Assert.assertEquals("X coordinate differs!", refPt.getX(),
actualPt.getX(), 0.0001);
+ Assert.assertEquals("Y coordinate differs!", refPt.getY(),
actualPt.getY(), 0.0001);
Review Comment:
Maybe a silly question: do these X/Y coordinates apply to longitude/latitude
on Earth (or is a `Point` a completely different thing)? if so, could we be
more strict here? when it comes to coordinates, 0.0001 degrees could be a
matter of meters
##########
ql/src/test/org/apache/hadoop/hive/ql/udf/esri/TestStCentroid.java:
##########
@@ -33,25 +32,30 @@ public class TestStCentroid {
/**
* Validates the centroid geometry writable.
*
- * @param point
- * the represented point location.
+ * @param expectedX
+ * the expected X (longitude) of the centroid.
+ * @param expectedY
+ * the expected Y (latitude) of the centroid.
* @param geometryAsWritable
* the geometry represented as {@link BytesWritable}.
*/
- private static void validatePoint(Point point, BytesWritable
geometryAsWritable) {
+ private static void validatePoint(double expectedX, double expectedY,
BytesWritable geometryAsWritable) {
ST_X getX = new ST_X();
ST_Y getY = new ST_Y();
DoubleWritable xAsWritable = getX.evaluate(geometryAsWritable);
DoubleWritable yAsWritable = getY.evaluate(geometryAsWritable);
- if (null == xAsWritable || null == yAsWritable || Math.abs(point.getX() -
xAsWritable.get()) > Epsilon
- || Math.abs(point.getY() - yAsWritable.get()) > Epsilon)
- System.err.println("validateCentroid: " + (new
ST_AsText()).evaluate(geometryAsWritable) + " ~ " + point);
+ if (null == xAsWritable || null == yAsWritable || Math.abs(expectedX -
xAsWritable.get()) > Epsilon
+ || Math.abs(expectedY - yAsWritable.get()) > Epsilon) {
+ System.err.println(
Review Comment:
LOG instead of System
##########
ql/src/test/org/apache/hadoop/hive/ql/udf/esri/TestStCentroid.java:
##########
Review Comment:
not introduced by this patch, but `EPSILON` looks better
also, please consider my another comment about coordinate accuracy
##########
ql/src/test/org/apache/hadoop/hive/ql/udf/esri/TestStCentroid.java:
##########
@@ -92,13 +96,13 @@ public void TestPolygonCentroid() throws Exception {
final ST_Polygon stPoly = new ST_Polygon();
BytesWritable bwGeom = stPoly.evaluate(new Text("polygon ((0 0, 0 8, 8 8,
8 0, 0 0))"));
BytesWritable bwCentroid = stCtr.evaluate(bwGeom);
- validatePoint(new Point(4, 4), bwCentroid);
+ validatePoint(4, 4, bwCentroid);
bwGeom = stPoly.evaluate(new Text("polygon ((1 1, 5 1, 3 4, 1 1))"));
bwCentroid = stCtr.evaluate(bwGeom);
- validatePoint(new Point(3, 2), bwCentroid);
+ validatePoint(3, 2, bwCentroid);
bwGeom = stPoly.evaluate(new Text("polygon ((14 0, -14 0, -2 24, 2 24, 14
0))"));
bwCentroid = stCtr.evaluate(bwGeom); // Cross-checked with ...
- validatePoint(new Point(0, 9), bwCentroid); // ...
omnicalculator.com/math/centroid
+ validatePoint(0, 9, bwCentroid); // ...
omnicalculator.com/math/centroid
Review Comment:
a single-line comment is better maybe
```
// Cross-checked with omnicalculator.com/math/centroid
validatePoint(0, 9, bwCentroid);
```
##########
ql/src/test/org/apache/hadoop/hive/ql/udf/esri/TestStGeomFromShape.java:
##########
Review Comment:
see my earlier comment on naming and accuracy
##########
ql/src/java/org/apache/hadoop/hive/ql/udf/esri/ST_Aggr_Union.java:
##########
@@ -102,14 +90,21 @@ public boolean merge(BytesWritable other) throws
HiveException {
* Return a geometry that is the union of all geometries added up until
this point
*/
public BytesWritable terminatePartial() throws HiveException {
+ if (geomList.isEmpty()) {
+ return null;
+ }
try {
- Geometry rslt = xgc.next();
- lgc = null; // not reusable
- xgc = null; // not reusable
- OGCGeometry ogeom = OGCGeometry.createFromEsriGeometry(rslt,
spatialRef);
- return GeometryUtils.geometryToEsriShapeBytesWritable(ogeom);
+ Geometry result = UnaryUnionOp.union(geomList);
+ if (result == null) {
+ return null;
+ }
+ int wkid = (firstWKID == -2) ? GeometryUtils.WKID_UNKNOWN : firstWKID;
+ return GeometryUtils.geometryToEsriShapeBytesWritable(result, wkid);
} catch (Exception e) {
LogUtils.Log_InternalError(LOG, "ST_Aggr_Union: " + e);
Review Comment:
other udf classes use simply `LOG`, what value does `LogUtils` bring here?
##########
ql/src/java/org/apache/hadoop/hive/ql/udf/esri/ST_MinZ.java:
##########
@@ -63,17 +64,26 @@ public DoubleWritable evaluate(BytesWritable geomref) {
return null;
}
- OGCGeometry ogcGeometry = GeometryUtils.geometryFromEsriShape(geomref);
- if (ogcGeometry == null) {
+ Geometry geom = GeometryUtils.geometryFromEsriShape(geomref);
+ if (geom == null) {
LogUtils.Log_ArgumentsNull(LOG);
return null;
}
- if (!ogcGeometry.is3D()) {
Review Comment:
is there a corresponding quick is3D filter in JTS to prevent iterating
through all the 2D points?
##########
ql/src/java/org/apache/hadoop/hive/ql/udf/esri/ST_NumGeometries.java:
##########
@@ -61,18 +58,10 @@ public IntWritable evaluate(BytesWritable geomref) {
case ST_POLYGON:
LogUtils.Log_InvalidType(LOG, GeometryUtils.OGCType.ST_MULTIPOLYGON,
ogcType);
return null;
Review Comment:
how `case ST_POLYGON` and `GeometryUtils.OGCType.ST_MULTIPOLYGON` goes
together? I guess I don't know the context in depth :)
--
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]
