Merge branch 'transform-bugfix-23.0' into transform-bugfix-23.1

# Conflicts: # pom.xml
sheinbergon · Mar 10, 2023 · 39ee975 · 39ee975
2 parents bb7ec42 + 4c801c4
commit 39ee975
Show file tree

Hide file tree

Showing 7 changed files with 118 additions and 36 deletions.
diff --git a/.idea/detekt.xml b/.idea/detekt.xml
diff --git a/pom.xml b/pom.xml
@@ -21,7 +21,8 @@
         <carrotsearch.version>0.7.0</carrotsearch.version>
         <arrow-memory-netty.version>9.0.0</arrow-memory-netty.version>
     </properties>
-    <version>0.8.8${artifact.version.suffix}</version>
+    <version>0.8.10</version>
+    <packaging>jar</packaging>
     <name>dremio-udf-gis</name>
     <description>GIS UDF extensions for Dremio</description>
     <url>https://github.com/sheinbergon/dremio-udf-gis</url>
@@ -218,6 +219,7 @@
         </dependency>
     </dependencies>
     <build>
+        <finalName>dremio-udf-gis-${version}${artifact.version.suffix}</finalName>
         <resources>
             <resource>
                 <directory>src/main/java</directory>

diff --git a/src/main/java/org/sheinbergon/dremio/udf/gis/util/GeometryHelpers.java b/src/main/java/org/sheinbergon/dremio/udf/gis/util/GeometryHelpers.java
@@ -56,7 +56,7 @@ public final class GeometryHelpers {
   private static final int GEOMETRY_DIMENSIONS = 2;
   private static final double AZIMUTH_NORTH_RADIANS = Angle.toRadians(90.0);
 
-  private static final Pattern EWKT_REGEX_PATTERN = Pattern.compile("^\\s*SRID\\s*=\\s*(\\d+)\\s*;\\s*(.+)\\s*$");
+  public static final Pattern EWKT_REGEX_PATTERN = Pattern.compile("^\\s*SRID\\s*=\\s*(\\d+)\\s*;\\s*(.+)\\s*$");
 
   private GeometryHelpers() {
   }

diff --git a/src/main/java/org/sheinbergon/dremio/udf/gis/util/GeometryTransformation.java b/src/main/java/org/sheinbergon/dremio/udf/gis/util/GeometryTransformation.java
@@ -25,6 +25,8 @@
 import org.slf4j.LoggerFactory;
 
 import javax.annotation.Nonnull;
+import java.util.Arrays;
+import java.util.stream.IntStream;
 
 public final class GeometryTransformation {
 
@@ -138,13 +140,16 @@ private static ProjCoordinate[] transformCoordinates(
   private static Polygon transform(
       @Nonnull final CoordinateTransform transform,
       @Nonnull final Polygon polygon) {
-    return polygon.getFactory()
-        .createPolygon(
-            transformCoordinates(transform, polygon.getCoordinates()));
+    LinearRing exterior = transform(transform, polygon.getExteriorRing());
+    LinearRing[] interior = IntStream.range(0, polygon.getNumInteriorRing())
+        .mapToObj(polygon::getInteriorRingN)
+        .map(ring -> transform(transform, ring))
+        .toArray(LinearRing[]::new);
+    return polygon.getFactory().createPolygon(exterior, interior);
   }
 
   @Nonnull
-  private static Geometry transform(
+  private static Point transform(
       @Nonnull final CoordinateTransform transform,
       @Nonnull final Point point) {
     return point.getFactory().createPoint(
@@ -153,7 +158,7 @@ private static Geometry transform(
   }
 
   @Nonnull
-  private static Geometry transform(
+  private static LinearRing transform(
       @Nonnull final CoordinateTransform transform,
       @Nonnull final LinearRing linearRing) {
     return linearRing.getFactory()
@@ -162,7 +167,7 @@ private static Geometry transform(
   }
 
   @Nonnull
-  private static Geometry transform(
+  private static LineString transform(
       @Nonnull final CoordinateTransform transform,
       @Nonnull final LineString lineString) {
     return lineString.getFactory().createLineString(
@@ -171,16 +176,14 @@ private static Geometry transform(
   }
 
   @Nonnull
-  private static Geometry transform(
+  private static MultiPolygon transform(
       @Nonnull final org.locationtech.proj4j.CoordinateTransform transform,
       @Nonnull final MultiPolygon multiPolygon) {
-    Polygon[] polygon = new Polygon[multiPolygon.getNumGeometries()];
-    for (int i = 0; i < polygon.length; ++i) {
-      polygon[i] = multiPolygon.getFactory()
-          .createPolygon(transformCoordinates(transform,
-              multiPolygon.getGeometryN(i).getCoordinates()));
-    }
-    return multiPolygon.getFactory().createMultiPolygon(polygon);
+    Polygon[] polygons = new Polygon[multiPolygon.getNumGeometries()];
+    Arrays.setAll(polygons, i -> transform(
+        transform,
+        (Polygon) multiPolygon.getGeometryN(i)));
+    return multiPolygon.getFactory().createMultiPolygon(polygons);
   }
 
   @Nonnull
@@ -193,20 +196,18 @@ private static Geometry transform(
   }
 
   @Nonnull
-  private static Geometry transform(
+  private static MultiLineString transform(
       @Nonnull final CoordinateTransform transform,
       @Nonnull final MultiLineString multiLineString) {
     LineString[] lineString = new LineString[multiLineString.getNumGeometries()];
-    for (int index = 0; index < lineString.length; ++index) {
-      lineString[index] = multiLineString.getFactory()
-          .createLineString(transformCoordinates(transform,
-              multiLineString.getGeometryN(index).getCoordinates()));
-    }
+    Arrays.setAll(lineString, index -> transform(
+        transform,
+        (LineString) multiLineString.getGeometryN(index)));
     return multiLineString.getFactory().createMultiLineString(lineString);
   }
 
   @Nonnull
-  private static Geometry transform(
+  private static GeometryCollection transform(
       @Nonnull final CoordinateTransform transform,
       @Nonnull final GeometryCollection collection) {
     Geometry[] geometry = new Geometry[collection.getNumGeometries()];

diff --git a/src/test/kotlin/org/sheinbergon/dremio/udf/gis/STCollectAggregateTests.kt b/src/test/kotlin/org/sheinbergon/dremio/udf/gis/STCollectAggregateTests.kt
@@ -12,11 +12,10 @@ internal class STCollectAggregateTests : GeometryAggregationFunSpec<STCollectAgg
       "Calling ST_Collect on several POLYGONs returns a MULTIPOLYGON",
       arrayOf(
         "POLYGON((0 0, 4 0, 4 4, 0 4, 0 0), (1 1, 1 2, 2 2, 2 1, 1 1))",
-        "POLYGON((30 10, 40 40, 20 40, 10 20, 30 10))",
         "POLYGON((50 40, 20 30, 10 20, 50 40))",
         "POLYGON EMPTY"
       ),
-      "MULTIPOLYGON (((0 0, 4 0, 4 4, 0 4, 0 0), (1 1, 1 2, 2 2, 2 1, 1 1)), ((30 10, 40 40, 20 40, 10 20, 30 10)), ((50 40, 20 30, 10 20, 50 40)))"
+      "MULTIPOLYGON (((0 0, 4 0, 4 4, 0 4, 0 0), (1 1, 1 2, 2 2, 2 1, 1 1)), ((50 40, 20 30, 10 20, 50 40)), EMPTY)"
     )
 
     testGeometryAggregationNoInput("Calling ST_Collect on no/null input returns nothing")

diff --git a/src/test/kotlin/org/sheinbergon/dremio/udf/gis/STTransformToSridTests.kt b/src/test/kotlin/org/sheinbergon/dremio/udf/gis/STTransformToSridTests.kt
@@ -20,6 +20,52 @@ internal class STTransformToSridTests : GeometryTransformationFunSpec<STTransfor
       sourceSrid = 3857,
       expected = "SRID=4326;POINT (71.0589 42.3601)"
     ) { function.targetSridInput.value = 4326 }
+
+    testGeometryTransformationEWKT(
+      name = "Calling ST_TRANSFORM on a doughnut MULTIPOLYGON",
+      wkt = """
+        MULTIPOLYGON((
+          (
+            3301892.7081 2305424.6332, 
+            3297641.4146 2293854.3149, 
+            3311252.1571 2288743.8019,  
+            3292652.3093 2299850.1394, 
+            3294313.4361 2305713.7012, 
+            3301892.7081 2305424.6332
+          ), 
+          (
+            3270969.4748 2271758.2972,
+            3272874.4151 2267708.8757,
+            3275976.4969 2266056.1655,
+            3278803.0097 2276290.0461, 
+            3273515.1866 2277201.8981, 
+            3270969.4748 2271758.2972
+          )
+        ))
+      """.replace('\n', ' '),
+      sourceSrid = 3035,
+      expected = """
+        SRID=4326;MULTIPOLYGON((
+          (
+            -2.546387000278512 43.08800899493393,
+            -2.575743178434398 42.97881974838686,
+            -2.401407821787952 42.953749749806,
+            -2.647614448413054 43.02455267884065,
+            -2.638786226764408 43.07914976493757,
+            -2.546387000278512 43.08800899493393
+          ),
+          (
+            -2.85513603155533 42.74197265119143,
+            -2.824354352347231 42.70893582158278,
+            -2.783822206487717 42.69900705701716,
+            -2.769511302427943 42.79422494822382,
+            -2.83502342149246 42.79422494807209,
+            -2.85513603155533 42.74197265119143
+            )
+          )
+        ))
+      """.replace('\n', ' ')
+    ) { function.targetSridInput.value = 4326 }
   }
 
   override val function = STTransformToSrid().apply {

diff --git a/src/test/kotlin/org/sheinbergon/dremio/udf/gis/util/ValueHolderTestSupport.kt b/src/test/kotlin/org/sheinbergon/dremio/udf/gis/util/ValueHolderTestSupport.kt
@@ -14,10 +14,12 @@ import org.apache.arrow.vector.holders.NullableVarBinaryHolder
 import org.apache.arrow.vector.holders.NullableVarCharHolder
 import org.apache.arrow.vector.holders.VarCharHolder
 import org.locationtech.jts.geom.Geometry
+import org.locationtech.jts.geom.GeometryFactory
 import org.locationtech.jts.geom.PrecisionModel
+import org.locationtech.jts.io.InputStreamInStream
+import org.locationtech.jts.io.WKBReader
 import org.locationtech.jts.io.WKBWriter
 import org.locationtech.jts.io.WKTReader
-import org.locationtech.jts.precision.GeometryPrecisionReducer
 import java.io.StringReader
 import java.nio.charset.StandardCharsets
 
@@ -65,27 +67,54 @@ internal fun NullableVarBinaryHolder.setBinary(bytes: ByteArray) {
 }
 
 internal fun NullableVarBinaryHolder.valueIsAsDescribedInWKT(text: String) =
-  valueIsAsDescribedIn(text, GeometryHelpers::toGeometry, GeometryHelpers::toBinary)
+  valueIsAsDescribedIn(text, ::toGeometryFromWKT, GeometryHelpers::toBinary)
 
 internal fun NullableVarBinaryHolder.valueIsAsDescribedInEWKT(text: String) =
-  valueIsAsDescribedIn(text, GeometryHelpers::toGeometryFromEWKT, GeometryHelpers::toEWKB)
+  valueIsAsDescribedIn(text, ::toGeometryFromEWKT, GeometryHelpers::toEWKB)
 
 internal fun NullableVarBinaryHolder.valueIsAsDescribedIn(
   text: String,
   adapter: (NullableVarCharHolder) -> Geometry,
   serializer: (Geometry) -> ByteArray,
 ) {
-  val evaluated = GeometryHelpers.toGeometry(this)
-  val reduced = GeometryPrecisionReducer.reducePointwise(evaluated, SCALED_PRECISION_MODEL)
-  reduced.srid = evaluated.srid
+  val evaluated = toGeometry(this)
   val expected = NullableVarCharHolder()
     .apply { setUtf8(text) }
     .let(adapter)
-
-  kotlin.runCatching { serializer(reduced) shouldBe serializer(expected) }
-    .recoverCatching { serializer(evaluated) shouldBe serializer(expected) }
-    .onSuccess { this.isSet shouldBeExactly 1 }
-    .getOrThrow()
+  serializer(evaluated) shouldBe serializer(expected)
+  this.isSet shouldBeExactly 1
+}
+
+private fun toGeometry(holder: NullableVarBinaryHolder) = holder.buffer?.run {
+  val buffer = holder.buffer.nioBuffer(holder.start.toLong(), holder.end - holder.start)
+  ByteBufferInputStream.toInputStream(buffer).use { stream ->
+    val adapter = InputStreamInStream(stream)
+    val reader = WKBReader(GeometryFactory(SCALED_PRECISION_MODEL))
+    reader.read(adapter)
+  }
+} ?: GeometryHelpers.emptyGeometry()
+
+private fun toGeometryFromWKT(holder: NullableVarCharHolder): Geometry {
+  val wkt = GeometryHelpers.toUTF8String(holder)
+  val reader = WKTReader(GeometryFactory(SCALED_PRECISION_MODEL))
+  return reader.read(wkt)
+}
+
+private fun toGeometryFromEWKT(
+  holder: NullableVarCharHolder
+): Geometry {
+  val ewkt = GeometryHelpers.toUTF8String(holder)
+  val reader = WKTReader(GeometryFactory(SCALED_PRECISION_MODEL))
+  val matcher = GeometryHelpers.EWKT_REGEX_PATTERN.matcher(ewkt)
+  return if (matcher.find()) {
+    reader
+      .read(matcher.group(2))
+      .also { geometry ->
+        geometry.srid = matcher.group(1).toInt()
+      }
+  } else {
+    throw IllegalArgumentException("input '$ewkt' is not a valid EWKT")
+  }
 }
 
 internal fun NullableVarBinaryHolder.valueIsNotSet() {