From 02494ddcd0032dc429c4169d1917c100caeb555b Mon Sep 17 00:00:00 2001 From: zhangfengcdt Date: Wed, 17 Jul 2024 08:35:43 -0700 Subject: [PATCH 01/10] [SEDONA-607] New function ST_SafeGeom to return geometry with exceptions --- .../sedona_sql/expressions/Functions.scala | 50 +++++++++++++++++++ 1 file changed, 50 insertions(+) diff --git a/spark/common/src/main/scala/org/apache/spark/sql/sedona_sql/expressions/Functions.scala b/spark/common/src/main/scala/org/apache/spark/sql/sedona_sql/expressions/Functions.scala index 3caadda1bc..894d54a7b0 100644 --- a/spark/common/src/main/scala/org/apache/spark/sql/sedona_sql/expressions/Functions.scala +++ b/spark/common/src/main/scala/org/apache/spark/sql/sedona_sql/expressions/Functions.scala @@ -33,6 +33,7 @@ import org.locationtech.jts.algorithm.MinimumBoundingCircle import org.locationtech.jts.geom._ import org.apache.spark.sql.sedona_sql.expressions.InferrableFunctionConverter._ import org.apache.spark.unsafe.types.UTF8String +import org.apache.spark.sql.types.{AbstractDataType, BooleanType, DataType, DoubleType, IntegerType, StructField, StructType} /** * Return the distance between two geometries. @@ -1604,3 +1605,52 @@ case class ST_IsValidReason(inputExpressions: Seq[Expression]) protected def withNewChildrenInternal(newChildren: IndexedSeq[Expression]) = copy(inputExpressions = newChildren) } + +/** + * Returns a geometry that represents the point set intersection of the Geometries. + * + * @param geom + * The geometry to validate. + * + * @return + * A struct of the following schema: + * - geometry: Geometry + * - error_message: String + */ +case class ST_SafeGeom(inputExpressions: Seq[Expression]) + extends Expression + with CodegenFallback + with ExpectsInputTypes { + + override def nullable: Boolean = true + + // Define the schema for the result + override def dataType: DataType = StructType( + Seq( + StructField("geometry", new GeometryUDT, nullable = true), + StructField("error_message", StringType, nullable = true))) + + override def eval(input: InternalRow): Any = { + // Evaluate the input expressions + val geom = inputExpressions(0).toGeometry(input) + + // Check if the raster geometry is null + if (geom == null) { + null + } else { + // Get the geometry with potential error message + val safeGemo = Functions.isValidReason(geom) + + // Create an InternalRow with the metadata + InternalRow.fromSeq(safeGemo.map(_.asInstanceOf[Any])) + } + } + + override def children: Seq[Expression] = inputExpressions + + protected def withNewChildrenInternal(newChildren: IndexedSeq[Expression]): ST_SafeGeom = { + copy(inputExpressions = newChildren) + } + + override def inputTypes: Seq[AbstractDataType] = Seq(GeometryUDT) +} From 05db1ac7e9cff273c4c6ee76dea45e1daa6fad9c Mon Sep 17 00:00:00 2001 From: zhangfengcdt Date: Wed, 17 Jul 2024 10:08:43 -0700 Subject: [PATCH 02/10] [SEDONA-607] Include Geometry in ST Function Exceptions --- .../org/apache/sedona/common/Functions.java | 4 +- .../geometryObjects/FaultyGeometry.java | 145 ++++++++++++++++++ .../geometrySerde/GeometrySerializer.java | 3 + .../apache/sedona/common/utils/GeomUtils.java | 20 +++ .../sedona_sql/expressions/Functions.scala | 50 ------ .../sedona/sql/dataFrameAPITestScala.scala | 13 ++ 6 files changed, 184 insertions(+), 51 deletions(-) create mode 100644 common/src/main/java/org/apache/sedona/common/geometryObjects/FaultyGeometry.java diff --git a/common/src/main/java/org/apache/sedona/common/Functions.java b/common/src/main/java/org/apache/sedona/common/Functions.java index a43519db19..403daede84 100644 --- a/common/src/main/java/org/apache/sedona/common/Functions.java +++ b/common/src/main/java/org/apache/sedona/common/Functions.java @@ -28,6 +28,7 @@ import java.util.stream.Collectors; import org.apache.commons.lang3.tuple.Pair; import org.apache.sedona.common.geometryObjects.Circle; +import org.apache.sedona.common.geometryObjects.FaultyGeometry; import org.apache.sedona.common.sphere.Spheroid; import org.apache.sedona.common.subDivide.GeometrySubDivider; import org.apache.sedona.common.utils.*; @@ -2157,7 +2158,8 @@ public static Geometry rotate(Geometry geometry, double angle, Geometry pointOri return geometry; } if (pointOrigin == null || pointOrigin.isEmpty() || !(pointOrigin instanceof Point)) { - throw new IllegalArgumentException("The origin must be a non-empty Point geometry."); + return new FaultyGeometry(pointOrigin, "The origin must be a non-empty Point geometry."); + // throw new IllegalArgumentException("The origin must be a non-empty Point geometry."); } Point origin = (Point) pointOrigin; double originX = origin.getX(); diff --git a/common/src/main/java/org/apache/sedona/common/geometryObjects/FaultyGeometry.java b/common/src/main/java/org/apache/sedona/common/geometryObjects/FaultyGeometry.java new file mode 100644 index 0000000000..cf974ee08f --- /dev/null +++ b/common/src/main/java/org/apache/sedona/common/geometryObjects/FaultyGeometry.java @@ -0,0 +1,145 @@ +/* + * 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.sedona.common.geometryObjects; + +import org.locationtech.jts.geom.*; + +public class FaultyGeometry extends Geometry { + private final Geometry geometry; + private final String errorMessage; + + public FaultyGeometry(Geometry geometry, String errorMessage) { + super(geometry.getFactory()); + this.geometry = geometry; + this.errorMessage = errorMessage; + } + + public Geometry getGeometry() { + return geometry; + } + + public String getErrorMessage() { + return errorMessage; + } + + @Override + public String getGeometryType() { + return geometry.getGeometryType(); + } + + @Override + public Coordinate getCoordinate() { + return geometry.getCoordinate(); + } + + @Override + public Coordinate[] getCoordinates() { + return geometry.getCoordinates(); + } + + @Override + public int getNumPoints() { + return geometry.getNumPoints(); + } + + @Override + public boolean isEmpty() { + return geometry.isEmpty(); + } + + @Override + public int getDimension() { + return geometry.getDimension(); + } + + @Override + public Geometry getBoundary() { + return geometry.getBoundary(); + } + + @Override + public int getBoundaryDimension() { + return geometry.getBoundaryDimension(); + } + + @Override + protected Geometry reverseInternal() { + return geometry.reverse(); + } + + @Override + public boolean equalsExact(Geometry other, double tolerance) { + return geometry.equalsExact(other, tolerance); + } + + @Override + public void apply(CoordinateFilter filter) { + geometry.apply(filter); + } + + @Override + public void apply(CoordinateSequenceFilter filter) { + geometry.apply(filter); + } + + @Override + public void apply(GeometryFilter filter) { + geometry.apply(filter); + } + + @Override + public void apply(GeometryComponentFilter filter) { + geometry.apply(filter); + } + + @Override + protected Geometry copyInternal() { + return geometry.copy(); + } + + @Override + public void normalize() { + geometry.normalize(); + } + + @Override + protected Envelope computeEnvelopeInternal() { + return geometry.getEnvelopeInternal(); + } + + @Override + protected int compareToSameClass(Object o) { + return geometry.compareTo(o); + } + + @Override + protected int compareToSameClass(Object o, CoordinateSequenceComparator comp) { + return geometry.compareTo(o, comp); + } + + @Override + protected int getTypeCode() { + return 0; + } + + @Override + public String toString() { + return geometry.toString() + (errorMessage != null ? " - " + errorMessage : ""); + } +} diff --git a/common/src/main/java/org/apache/sedona/common/geometrySerde/GeometrySerializer.java b/common/src/main/java/org/apache/sedona/common/geometrySerde/GeometrySerializer.java index 508a62901d..56297a3090 100644 --- a/common/src/main/java/org/apache/sedona/common/geometrySerde/GeometrySerializer.java +++ b/common/src/main/java/org/apache/sedona/common/geometrySerde/GeometrySerializer.java @@ -18,6 +18,7 @@ */ package org.apache.sedona.common.geometrySerde; +import org.apache.sedona.common.geometryObjects.FaultyGeometry; import org.locationtech.jts.geom.Coordinate; import org.locationtech.jts.geom.CoordinateSequence; import org.locationtech.jts.geom.Geometry; @@ -53,6 +54,8 @@ public static byte[] serialize(Geometry geometry) { buffer = serializeMultiPolygon((MultiPolygon) geometry); } else if (geometry instanceof GeometryCollection) { buffer = serializeGeometryCollection((GeometryCollection) geometry); + } else if (geometry instanceof FaultyGeometry) { + return serialize(((FaultyGeometry) geometry).getGeometry()); } else { throw new UnsupportedOperationException( "Geometry type is not supported: " + geometry.getClass().getSimpleName()); diff --git a/common/src/main/java/org/apache/sedona/common/utils/GeomUtils.java b/common/src/main/java/org/apache/sedona/common/utils/GeomUtils.java index 9ec7d47398..3d66c7acbf 100644 --- a/common/src/main/java/org/apache/sedona/common/utils/GeomUtils.java +++ b/common/src/main/java/org/apache/sedona/common/utils/GeomUtils.java @@ -23,6 +23,7 @@ import java.nio.ByteOrder; import java.util.*; import org.apache.sedona.common.Functions; +import org.apache.sedona.common.geometryObjects.FaultyGeometry; import org.locationtech.jts.algorithm.Angle; import org.locationtech.jts.algorithm.distance.DiscreteFrechetDistance; import org.locationtech.jts.algorithm.distance.DiscreteHausdorffDistance; @@ -175,6 +176,13 @@ public static String getEWKT(Geometry geometry) { if (srid != 0) { sridString = "SRID=" + String.valueOf(srid) + ";"; } + if (geometry instanceof FaultyGeometry) { + return "[ERROR]" + + ((FaultyGeometry) geometry).getErrorMessage() + + " " + + sridString + + new WKTWriter(4).write(((FaultyGeometry) geometry).getGeometry()); + } return sridString + new WKTWriter(4).write(geometry); } @@ -182,12 +190,24 @@ public static String getWKT(Geometry geometry) { if (geometry == null) { return null; } + if (geometry instanceof FaultyGeometry) { + return "[ERROR]" + + ((FaultyGeometry) geometry).getErrorMessage() + + " " + + new WKTWriter(4).write(((FaultyGeometry) geometry).getGeometry()); + } return new WKTWriter(4).write(geometry); } public static String getHexEWKB(Geometry geometry, int endian) { WKBWriter writer = new WKBWriter(GeomUtils.getDimension(geometry), endian, geometry.getSRID() != 0); + if (geometry instanceof FaultyGeometry) { + return "[ERROR]" + + ((FaultyGeometry) geometry).getErrorMessage() + + " " + + WKBWriter.toHex(writer.write(((FaultyGeometry) geometry).getGeometry())); + } return WKBWriter.toHex(writer.write(geometry)); } diff --git a/spark/common/src/main/scala/org/apache/spark/sql/sedona_sql/expressions/Functions.scala b/spark/common/src/main/scala/org/apache/spark/sql/sedona_sql/expressions/Functions.scala index 630b645469..47711716a3 100644 --- a/spark/common/src/main/scala/org/apache/spark/sql/sedona_sql/expressions/Functions.scala +++ b/spark/common/src/main/scala/org/apache/spark/sql/sedona_sql/expressions/Functions.scala @@ -33,7 +33,6 @@ import org.locationtech.jts.algorithm.MinimumBoundingCircle import org.locationtech.jts.geom._ import org.apache.spark.sql.sedona_sql.expressions.InferrableFunctionConverter._ import org.apache.spark.unsafe.types.UTF8String -import org.apache.spark.sql.types.{AbstractDataType, BooleanType, DataType, DoubleType, IntegerType, StructField, StructType} /** * Return the distance between two geometries. @@ -1613,52 +1612,3 @@ case class ST_Rotate(inputExpressions: Seq[Expression]) protected def withNewChildrenInternal(newChildren: IndexedSeq[Expression]) = copy(inputExpressions = newChildren) } - -/** - * Returns a geometry that represents the point set intersection of the Geometries. - * - * @param geom - * The geometry to validate. - * - * @return - * A struct of the following schema: - * - geometry: Geometry - * - error_message: String - */ -case class ST_SafeGeom(inputExpressions: Seq[Expression]) - extends Expression - with CodegenFallback - with ExpectsInputTypes { - - override def nullable: Boolean = true - - // Define the schema for the result - override def dataType: DataType = StructType( - Seq( - StructField("geometry", new GeometryUDT, nullable = true), - StructField("error_message", StringType, nullable = true))) - - override def eval(input: InternalRow): Any = { - // Evaluate the input expressions - val geom = inputExpressions(0).toGeometry(input) - - // Check if the raster geometry is null - if (geom == null) { - null - } else { - // Get the geometry with potential error message - val safeGemo = Functions.isValidReason(geom) - - // Create an InternalRow with the metadata - InternalRow.fromSeq(safeGemo.map(_.asInstanceOf[Any])) - } - } - - override def children: Seq[Expression] = inputExpressions - - protected def withNewChildrenInternal(newChildren: IndexedSeq[Expression]): ST_SafeGeom = { - copy(inputExpressions = newChildren) - } - - override def inputTypes: Seq[AbstractDataType] = Seq(GeometryUDT) -} diff --git a/spark/common/src/test/scala/org/apache/sedona/sql/dataFrameAPITestScala.scala b/spark/common/src/test/scala/org/apache/sedona/sql/dataFrameAPITestScala.scala index 81b849592f..e77a100388 100644 --- a/spark/common/src/test/scala/org/apache/sedona/sql/dataFrameAPITestScala.scala +++ b/spark/common/src/test/scala/org/apache/sedona/sql/dataFrameAPITestScala.scala @@ -2193,5 +2193,18 @@ class dataFrameAPITestScala extends TestBaseScala { "SRID=4326;POLYGON ((-0.4546817643920842 0.5948176504236309, 1.4752502925921425 0.0700679430157733, 2 2, 0.0700679430157733 2.5247497074078575, 0.7726591178039579 1.2974088252118154, -0.4546817643920842 0.5948176504236309))" assert(expected.equals(actual)) } + + it("Returning geometry with errors on ST_Rotate") { + val baseDf = sparkSession.sql( + "SELECT ST_GeomFromEWKT('SRID=4326;POLYGON ((0 0, 2 0, 2 2, 0 2, 1 1, 0 0))') AS geom1, ST_GeomFromEWKT('SRID=4326;POLYGON ((0 0, 1 0, 1 1, 0 1, 1 1, 0 0))') AS geom2") + val row = baseDf.select(ST_Rotate("geom1", 50, "geom2")).take(1).mkString("") + val rowWithError = + baseDf.select(ST_AsEWKT(ST_Rotate("geom1", 50, "geom2"))).take(1).mkString("") + + assertEquals("[POLYGON ((0 0, 1 0, 1 1, 0 1, 1 1, 0 0))]", row) + assertEquals( + "[[ERROR]The origin must be a non-empty Point geometry. SRID=4326;POLYGON ((0 0, 1 0, 1 1, 0 1, 1 1, 0 0))]", + rowWithError) + } } } From b9d3b3db6870ea7b880b74ad30bce60a05c5fae4 Mon Sep 17 00:00:00 2001 From: zhangfengcdt Date: Wed, 17 Jul 2024 10:20:09 -0700 Subject: [PATCH 03/10] throw exception on serialization of geometry --- .../common/geometrySerde/GeometrySerializer.java | 6 +++++- .../apache/sedona/sql/dataFrameAPITestScala.scala | 12 +++++++++--- 2 files changed, 14 insertions(+), 4 deletions(-) diff --git a/common/src/main/java/org/apache/sedona/common/geometrySerde/GeometrySerializer.java b/common/src/main/java/org/apache/sedona/common/geometrySerde/GeometrySerializer.java index 56297a3090..2e9cbc6308 100644 --- a/common/src/main/java/org/apache/sedona/common/geometrySerde/GeometrySerializer.java +++ b/common/src/main/java/org/apache/sedona/common/geometrySerde/GeometrySerializer.java @@ -55,7 +55,11 @@ public static byte[] serialize(Geometry geometry) { } else if (geometry instanceof GeometryCollection) { buffer = serializeGeometryCollection((GeometryCollection) geometry); } else if (geometry instanceof FaultyGeometry) { - return serialize(((FaultyGeometry) geometry).getGeometry()); + throw new UnsupportedOperationException( + "FaultyGeometry returned with error message: " + + ((FaultyGeometry) geometry).getErrorMessage() + + " for geometry: " + + geometry); } else { throw new UnsupportedOperationException( "Geometry type is not supported: " + geometry.getClass().getSimpleName()); diff --git a/spark/common/src/test/scala/org/apache/sedona/sql/dataFrameAPITestScala.scala b/spark/common/src/test/scala/org/apache/sedona/sql/dataFrameAPITestScala.scala index e77a100388..394ccd98ba 100644 --- a/spark/common/src/test/scala/org/apache/sedona/sql/dataFrameAPITestScala.scala +++ b/spark/common/src/test/scala/org/apache/sedona/sql/dataFrameAPITestScala.scala @@ -2197,11 +2197,17 @@ class dataFrameAPITestScala extends TestBaseScala { it("Returning geometry with errors on ST_Rotate") { val baseDf = sparkSession.sql( "SELECT ST_GeomFromEWKT('SRID=4326;POLYGON ((0 0, 2 0, 2 2, 0 2, 1 1, 0 0))') AS geom1, ST_GeomFromEWKT('SRID=4326;POLYGON ((0 0, 1 0, 1 1, 0 1, 1 1, 0 0))') AS geom2") - val row = baseDf.select(ST_Rotate("geom1", 50, "geom2")).take(1).mkString("") + + // Use intercept to assert that an exception is thrown + val exception = intercept[Exception] { + baseDf.select(ST_Rotate("geom1", 50, "geom2")).take(1) + } + // Check the exception message + assert(exception.getMessage.contains( + "FaultyGeometry returned with error message: The origin must be a non-empty Point geometry. for geometry: POLYGON ((0 0, 1 0, 1 1, 0 1, 1 1, 0 0)) - The origin must be a non-empty Point geometry.")) + val rowWithError = baseDf.select(ST_AsEWKT(ST_Rotate("geom1", 50, "geom2"))).take(1).mkString("") - - assertEquals("[POLYGON ((0 0, 1 0, 1 1, 0 1, 1 1, 0 0))]", row) assertEquals( "[[ERROR]The origin must be a non-empty Point geometry. SRID=4326;POLYGON ((0 0, 1 0, 1 1, 0 1, 1 1, 0 0))]", rowWithError) From 2e5b91aee59094df3e47681b4e32871de958cfc3 Mon Sep 17 00:00:00 2001 From: zhangfengcdt Date: Wed, 17 Jul 2024 16:02:51 -0700 Subject: [PATCH 04/10] refactor the code --- .../org/apache/sedona/common/Functions.java | 51 +++--- .../exception/IllegalGeometryException.java | 41 +++++ .../geometryObjects/FaultyGeometry.java | 145 ------------------ .../geometrySerde/GeometrySerializer.java | 7 - .../apache/sedona/common/utils/GeomUtils.java | 32 ++-- .../sedona/sql/dataFrameAPITestScala.scala | 15 +- 6 files changed, 87 insertions(+), 204 deletions(-) create mode 100644 common/src/main/java/org/apache/sedona/common/exception/IllegalGeometryException.java delete mode 100644 common/src/main/java/org/apache/sedona/common/geometryObjects/FaultyGeometry.java diff --git a/common/src/main/java/org/apache/sedona/common/Functions.java b/common/src/main/java/org/apache/sedona/common/Functions.java index 403daede84..013c296b17 100644 --- a/common/src/main/java/org/apache/sedona/common/Functions.java +++ b/common/src/main/java/org/apache/sedona/common/Functions.java @@ -27,8 +27,8 @@ import java.util.*; import java.util.stream.Collectors; import org.apache.commons.lang3.tuple.Pair; +import org.apache.sedona.common.exception.IllegalGeometryException; import org.apache.sedona.common.geometryObjects.Circle; -import org.apache.sedona.common.geometryObjects.FaultyGeometry; import org.apache.sedona.common.sphere.Spheroid; import org.apache.sedona.common.subDivide.GeometrySubDivider; import org.apache.sedona.common.utils.*; @@ -253,8 +253,8 @@ public static int bestSRID(Geometry geometry) throws IllegalArgumentException { Double xwidth = Spheroid.angularWidth(envelope); Double ywidth = Spheroid.angularHeight(envelope); if (xwidth.isNaN() | ywidth.isNaN()) { - throw new IllegalArgumentException( - "Only lon/lat coordinate systems are supported by ST_BestSRID"); + throw new IllegalGeometryException( + "Only lon/lat coordinate systems are supported by ST_BestSRID", geometry); } // Prioritize polar regions for Lambert Azimuthal Equal Area projection @@ -598,8 +598,9 @@ public static String asHexEWKB(Geometry geom, String endian) { } else if (endian.equalsIgnoreCase("XDR")) { return GeomUtils.getHexEWKB(geom, ByteOrderValues.BIG_ENDIAN); } - throw new IllegalArgumentException( - "You must select either NDR (little-endian) or XDR (big-endian) as the endian format."); + throw new IllegalGeometryException( + "You must select either NDR (little-endian) or XDR (big-endian) as the endian format.", + geom); } public static String asHexEWKB(Geometry geom) { @@ -822,7 +823,8 @@ public static Geometry closestPoint(Geometry left, Geometry right) { Coordinate[] closestPoints = distanceOp.nearestPoints(); return left.getFactory().createPoint(closestPoints[0]); } catch (Exception e) { - throw new IllegalArgumentException("ST_ClosestPoint doesn't support empty geometry object."); + throw new IllegalGeometryException( + "ST_ClosestPoint doesn't support empty geometry object.", left); } } @@ -1537,8 +1539,8 @@ public static Geometry makeLine(Geometry[] geoms) { coordinates.add(coord); } } else { - throw new IllegalArgumentException( - "ST_MakeLine only supports Point, MultiPoint and LineString geometries"); + throw new IllegalGeometryException( + "ST_MakeLine only supports Point, MultiPoint and LineString geometries", geom); } } @@ -1623,7 +1625,7 @@ public static Geometry collectionExtract(Geometry geometry, Integer geomType) { emptyResult = factory.createMultiPolygon(); break; default: - throw new IllegalArgumentException("Invalid geometry type"); + throw new IllegalGeometryException("Invalid geometry type: " + geomType, geometry); } List geometries = GeomUtils.extractGeometryCollection(geometry, geomClass); if (geometries.isEmpty()) { @@ -1769,10 +1771,9 @@ private static Coordinate[] extractCoordinates(Geometry geometry) { public static int numPoints(Geometry geometry) throws Exception { String geometryType = geometry.getGeometryType(); if (!(Geometry.TYPENAME_LINESTRING.equalsIgnoreCase(geometryType))) { - throw new IllegalArgumentException( - "Unsupported geometry type: " - + geometryType - + ", only LineString geometry is supported."); + throw new IllegalGeometryException( + "Unsupported geometry type: " + geometryType + ", only LineString geometry is supported.", + geometry); } return geometry.getNumPoints(); } @@ -1816,10 +1817,11 @@ private static Geometry[] convertGeometryToArray(Geometry geom) { public static Integer nRings(Geometry geometry) throws Exception { String geometryType = geometry.getGeometryType(); if (!(geometry instanceof Polygon || geometry instanceof MultiPolygon)) { - throw new IllegalArgumentException( + throw new IllegalGeometryException( "Unsupported geometry type: " + geometryType - + ", only Polygon or MultiPolygon geometries are supported."); + + ", only Polygon or MultiPolygon geometries are supported.", + geometry); } int numRings = 0; if (geometry instanceof Polygon) { @@ -1885,7 +1887,7 @@ public static Geometry geometricMedian( String geometryType = geometry.getGeometryType(); if (!(Geometry.TYPENAME_POINT.equals(geometryType) || Geometry.TYPENAME_MULTIPOINT.equals(geometryType))) { - throw new Exception("Unsupported geometry type: " + geometryType); + throw new IllegalGeometryException("Unsupported geometry type: " + geometryType, geometry); } Coordinate[] coordinates = extractCoordinates(geometry); if (coordinates.length == 0) return new Point(null, factory); @@ -1896,9 +1898,10 @@ public static Geometry geometricMedian( for (int i = 0; i < maxIter && delta > tolerance; i++) delta = iteratePoints(median, coordinates, distances); if (failIfNotConverged && delta > tolerance) - throw new Exception( + throw new IllegalGeometryException( String.format( - "Median failed to converge within %.1E after %d iterations.", tolerance, maxIter)); + "Median failed to converge within %.1E after %d iterations.", tolerance, maxIter), + geometry); boolean is3d = !Double.isNaN(geometry.getCoordinate().z); if (!is3d) median.z = Double.NaN; return factory.createPoint(median); @@ -1966,7 +1969,9 @@ public static double angle(Geometry point1, Geometry point2, Geometry point3, Ge if (point3 == null && point4 == null) return Functions.angle(point1, point2); else if (point4 == null) return Functions.angle(point1, point2, point3); if (GeomUtils.isAnyGeomEmpty(point1, point2, point3, point4)) - throw new IllegalArgumentException("ST_Angle cannot support empty geometries."); + throw new IllegalArgumentException( + "ST_Angle cannot support empty geometries, empty index = " + + Arrays.toString(GeomUtils.emptyGeometries(point1, point2, point3, point4))); if (!(point1 instanceof Point && point2 instanceof Point && point3 instanceof Point @@ -1983,7 +1988,9 @@ public static double angle(Geometry point1, Geometry point2, Geometry point3, Ge public static double angle(Geometry point1, Geometry point2, Geometry point3) throws IllegalArgumentException { if (GeomUtils.isAnyGeomEmpty(point1, point2, point3)) - throw new IllegalArgumentException("ST_Angle cannot support empty geometries."); + throw new IllegalArgumentException( + "ST_Angle cannot support empty geometries, empty index = " + + Arrays.toString(GeomUtils.emptyGeometries(point1, point2, point3))); if (!(point1 instanceof Point && point2 instanceof Point && point3 instanceof Point)) throw new IllegalArgumentException( "ST_Angle supports either only POINT or only LINESTRING geometries."); @@ -2158,8 +2165,8 @@ public static Geometry rotate(Geometry geometry, double angle, Geometry pointOri return geometry; } if (pointOrigin == null || pointOrigin.isEmpty() || !(pointOrigin instanceof Point)) { - return new FaultyGeometry(pointOrigin, "The origin must be a non-empty Point geometry."); - // throw new IllegalArgumentException("The origin must be a non-empty Point geometry."); + throw new IllegalGeometryException( + "The origin must be a non-empty Point geometry.", pointOrigin); } Point origin = (Point) pointOrigin; double originX = origin.getX(); diff --git a/common/src/main/java/org/apache/sedona/common/exception/IllegalGeometryException.java b/common/src/main/java/org/apache/sedona/common/exception/IllegalGeometryException.java new file mode 100644 index 0000000000..d05bdee95d --- /dev/null +++ b/common/src/main/java/org/apache/sedona/common/exception/IllegalGeometryException.java @@ -0,0 +1,41 @@ +/* + * 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.sedona.common.exception; + +import org.locationtech.jts.geom.Geometry; + +/** Exception for illegal geometries. */ +public class IllegalGeometryException extends RuntimeException { + // The geometry that caused the exception + private Geometry geometry; + + public IllegalGeometryException(String message, Geometry geometry) { + super(message + " [GEOM] " + geometry.toString()); + this.geometry = geometry; + } + + public IllegalGeometryException(String message, Geometry geometry, Throwable cause) { + super(message + " " + geometry.toString(), cause); + this.geometry = geometry; + } + + public Geometry getGeometry() { + return geometry; + } +} diff --git a/common/src/main/java/org/apache/sedona/common/geometryObjects/FaultyGeometry.java b/common/src/main/java/org/apache/sedona/common/geometryObjects/FaultyGeometry.java deleted file mode 100644 index cf974ee08f..0000000000 --- a/common/src/main/java/org/apache/sedona/common/geometryObjects/FaultyGeometry.java +++ /dev/null @@ -1,145 +0,0 @@ -/* - * 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.sedona.common.geometryObjects; - -import org.locationtech.jts.geom.*; - -public class FaultyGeometry extends Geometry { - private final Geometry geometry; - private final String errorMessage; - - public FaultyGeometry(Geometry geometry, String errorMessage) { - super(geometry.getFactory()); - this.geometry = geometry; - this.errorMessage = errorMessage; - } - - public Geometry getGeometry() { - return geometry; - } - - public String getErrorMessage() { - return errorMessage; - } - - @Override - public String getGeometryType() { - return geometry.getGeometryType(); - } - - @Override - public Coordinate getCoordinate() { - return geometry.getCoordinate(); - } - - @Override - public Coordinate[] getCoordinates() { - return geometry.getCoordinates(); - } - - @Override - public int getNumPoints() { - return geometry.getNumPoints(); - } - - @Override - public boolean isEmpty() { - return geometry.isEmpty(); - } - - @Override - public int getDimension() { - return geometry.getDimension(); - } - - @Override - public Geometry getBoundary() { - return geometry.getBoundary(); - } - - @Override - public int getBoundaryDimension() { - return geometry.getBoundaryDimension(); - } - - @Override - protected Geometry reverseInternal() { - return geometry.reverse(); - } - - @Override - public boolean equalsExact(Geometry other, double tolerance) { - return geometry.equalsExact(other, tolerance); - } - - @Override - public void apply(CoordinateFilter filter) { - geometry.apply(filter); - } - - @Override - public void apply(CoordinateSequenceFilter filter) { - geometry.apply(filter); - } - - @Override - public void apply(GeometryFilter filter) { - geometry.apply(filter); - } - - @Override - public void apply(GeometryComponentFilter filter) { - geometry.apply(filter); - } - - @Override - protected Geometry copyInternal() { - return geometry.copy(); - } - - @Override - public void normalize() { - geometry.normalize(); - } - - @Override - protected Envelope computeEnvelopeInternal() { - return geometry.getEnvelopeInternal(); - } - - @Override - protected int compareToSameClass(Object o) { - return geometry.compareTo(o); - } - - @Override - protected int compareToSameClass(Object o, CoordinateSequenceComparator comp) { - return geometry.compareTo(o, comp); - } - - @Override - protected int getTypeCode() { - return 0; - } - - @Override - public String toString() { - return geometry.toString() + (errorMessage != null ? " - " + errorMessage : ""); - } -} diff --git a/common/src/main/java/org/apache/sedona/common/geometrySerde/GeometrySerializer.java b/common/src/main/java/org/apache/sedona/common/geometrySerde/GeometrySerializer.java index 2e9cbc6308..508a62901d 100644 --- a/common/src/main/java/org/apache/sedona/common/geometrySerde/GeometrySerializer.java +++ b/common/src/main/java/org/apache/sedona/common/geometrySerde/GeometrySerializer.java @@ -18,7 +18,6 @@ */ package org.apache.sedona.common.geometrySerde; -import org.apache.sedona.common.geometryObjects.FaultyGeometry; import org.locationtech.jts.geom.Coordinate; import org.locationtech.jts.geom.CoordinateSequence; import org.locationtech.jts.geom.Geometry; @@ -54,12 +53,6 @@ public static byte[] serialize(Geometry geometry) { buffer = serializeMultiPolygon((MultiPolygon) geometry); } else if (geometry instanceof GeometryCollection) { buffer = serializeGeometryCollection((GeometryCollection) geometry); - } else if (geometry instanceof FaultyGeometry) { - throw new UnsupportedOperationException( - "FaultyGeometry returned with error message: " - + ((FaultyGeometry) geometry).getErrorMessage() - + " for geometry: " - + geometry); } else { throw new UnsupportedOperationException( "Geometry type is not supported: " + geometry.getClass().getSimpleName()); diff --git a/common/src/main/java/org/apache/sedona/common/utils/GeomUtils.java b/common/src/main/java/org/apache/sedona/common/utils/GeomUtils.java index 3d66c7acbf..89a3968fef 100644 --- a/common/src/main/java/org/apache/sedona/common/utils/GeomUtils.java +++ b/common/src/main/java/org/apache/sedona/common/utils/GeomUtils.java @@ -23,7 +23,7 @@ import java.nio.ByteOrder; import java.util.*; import org.apache.sedona.common.Functions; -import org.apache.sedona.common.geometryObjects.FaultyGeometry; +import org.apache.sedona.common.exception.IllegalGeometryException; import org.locationtech.jts.algorithm.Angle; import org.locationtech.jts.algorithm.distance.DiscreteFrechetDistance; import org.locationtech.jts.algorithm.distance.DiscreteHausdorffDistance; @@ -176,13 +176,6 @@ public static String getEWKT(Geometry geometry) { if (srid != 0) { sridString = "SRID=" + String.valueOf(srid) + ";"; } - if (geometry instanceof FaultyGeometry) { - return "[ERROR]" - + ((FaultyGeometry) geometry).getErrorMessage() - + " " - + sridString - + new WKTWriter(4).write(((FaultyGeometry) geometry).getGeometry()); - } return sridString + new WKTWriter(4).write(geometry); } @@ -190,24 +183,12 @@ public static String getWKT(Geometry geometry) { if (geometry == null) { return null; } - if (geometry instanceof FaultyGeometry) { - return "[ERROR]" - + ((FaultyGeometry) geometry).getErrorMessage() - + " " - + new WKTWriter(4).write(((FaultyGeometry) geometry).getGeometry()); - } return new WKTWriter(4).write(geometry); } public static String getHexEWKB(Geometry geometry, int endian) { WKBWriter writer = new WKBWriter(GeomUtils.getDimension(geometry), endian, geometry.getSRID() != 0); - if (geometry instanceof FaultyGeometry) { - return "[ERROR]" - + ((FaultyGeometry) geometry).getErrorMessage() - + " " - + WKBWriter.toHex(writer.write(((FaultyGeometry) geometry).getGeometry())); - } return WKBWriter.toHex(writer.write(geometry)); } @@ -499,6 +480,15 @@ public static boolean isAnyGeomEmpty(Geometry... geometries) { return false; } + public static int[] emptyGeometries(Geometry... geometries) { + List emptyGeometries = new ArrayList<>(); + int i = 0; + for (Geometry geometry : geometries) { + if (geometry != null) if (geometry.isEmpty()) emptyGeometries.add(i); + } + return emptyGeometries.stream().mapToInt(Integer::intValue).toArray(); + } + public static Coordinate[] getStartEndCoordinates(Geometry line) { if (line.getNumPoints() < 2) return null; Coordinate[] coordinates = line.getCoordinates(); @@ -574,7 +564,7 @@ public static Double getHausdorffDistance(Geometry g1, Geometry g2, double densi public static Geometry addMeasure(Geometry geom, double measure_start, double measure_end) { if (!(geom instanceof LineString) && !(geom instanceof MultiLineString)) { - throw new IllegalArgumentException("Geometry must be a LineString or MultiLineString."); + throw new IllegalGeometryException("Geometry must be a LineString or MultiLineString.", geom); } if (geom instanceof LineString) { diff --git a/spark/common/src/test/scala/org/apache/sedona/sql/dataFrameAPITestScala.scala b/spark/common/src/test/scala/org/apache/sedona/sql/dataFrameAPITestScala.scala index 394ccd98ba..11465afa93 100644 --- a/spark/common/src/test/scala/org/apache/sedona/sql/dataFrameAPITestScala.scala +++ b/spark/common/src/test/scala/org/apache/sedona/sql/dataFrameAPITestScala.scala @@ -19,6 +19,7 @@ package org.apache.sedona.sql import org.apache.commons.codec.binary.Hex +import org.apache.sedona.common.exception.IllegalGeometryException import org.apache.spark.sql.Row import org.apache.spark.sql.functions.{array, col, element_at, lit} import org.apache.spark.sql.sedona_sql.expressions.st_aggregates._ @@ -2194,23 +2195,19 @@ class dataFrameAPITestScala extends TestBaseScala { assert(expected.equals(actual)) } - it("Returning geometry with errors on ST_Rotate") { + it("Returning exception with geometry when exception is thrown by ST_Rotate") { val baseDf = sparkSession.sql( "SELECT ST_GeomFromEWKT('SRID=4326;POLYGON ((0 0, 2 0, 2 2, 0 2, 1 1, 0 0))') AS geom1, ST_GeomFromEWKT('SRID=4326;POLYGON ((0 0, 1 0, 1 1, 0 1, 1 1, 0 0))') AS geom2") // Use intercept to assert that an exception is thrown - val exception = intercept[Exception] { + val exception = intercept[IllegalGeometryException] { baseDf.select(ST_Rotate("geom1", 50, "geom2")).take(1) } + // Check the exception geometry + assert(exception.getGeometry != null) // Check the exception message assert(exception.getMessage.contains( - "FaultyGeometry returned with error message: The origin must be a non-empty Point geometry. for geometry: POLYGON ((0 0, 1 0, 1 1, 0 1, 1 1, 0 0)) - The origin must be a non-empty Point geometry.")) - - val rowWithError = - baseDf.select(ST_AsEWKT(ST_Rotate("geom1", 50, "geom2"))).take(1).mkString("") - assertEquals( - "[[ERROR]The origin must be a non-empty Point geometry. SRID=4326;POLYGON ((0 0, 1 0, 1 1, 0 1, 1 1, 0 0))]", - rowWithError) + "The origin must be a non-empty Point geometry. [GEOM] POLYGON ((0 0, 1 0, 1 1, 0 1, 1 1, 0 0))")) } } } From 15872ce4db70fa025338e0eb4e35c0eb184c3a65 Mon Sep 17 00:00:00 2001 From: zhangfengcdt Date: Thu, 18 Jul 2024 08:44:30 -0700 Subject: [PATCH 05/10] fix unit tests --- .../org/apache/sedona/common/Functions.java | 84 ++++++++++++------- .../sedona/common/FunctionsGeoTools.java | 19 ++++- .../exception/IllegalGeometryException.java | 21 ++--- .../apache/sedona/common/utils/GeomUtils.java | 3 +- .../apache/sedona/common/FunctionsTest.java | 59 ++++++++----- .../sedona/sql/dataFrameAPITestScala.scala | 2 +- 6 files changed, 121 insertions(+), 67 deletions(-) diff --git a/common/src/main/java/org/apache/sedona/common/Functions.java b/common/src/main/java/org/apache/sedona/common/Functions.java index 013c296b17..e85e9d45a3 100644 --- a/common/src/main/java/org/apache/sedona/common/Functions.java +++ b/common/src/main/java/org/apache/sedona/common/Functions.java @@ -101,7 +101,7 @@ public static Geometry buffer(Geometry geometry, double radius, boolean useSpher public static Geometry buffer( Geometry geometry, double radius, boolean useSpheroid, String params) - throws IllegalArgumentException { + throws IllegalGeometryException { BufferParameters bufferParameters = new BufferParameters(); // Processing parameters @@ -123,7 +123,8 @@ public static Geometry buffer( try { return bufferSpheroid(geometry, radius, bufferParameters); } catch (RuntimeException e) { - throw new RuntimeException("Error processing spheroidal buffer", e); + throw new IllegalGeometryException( + "Error processing spheroidal buffer" + e.getMessage(), new Geometry[] {geometry}); } } else { // Existing planar buffer logic with params handling @@ -254,7 +255,8 @@ public static int bestSRID(Geometry geometry) throws IllegalArgumentException { Double ywidth = Spheroid.angularHeight(envelope); if (xwidth.isNaN() | ywidth.isNaN()) { throw new IllegalGeometryException( - "Only lon/lat coordinate systems are supported by ST_BestSRID", geometry); + "Only lon/lat coordinate systems are supported by ST_BestSRID", + new Geometry[] {geometry}); } // Prioritize polar regions for Lambert Azimuthal Equal Area projection @@ -600,7 +602,7 @@ public static String asHexEWKB(Geometry geom, String endian) { } throw new IllegalGeometryException( "You must select either NDR (little-endian) or XDR (big-endian) as the endian format.", - geom); + new Geometry[] {geom}); } public static String asHexEWKB(Geometry geom) { @@ -824,7 +826,7 @@ public static Geometry closestPoint(Geometry left, Geometry right) { return left.getFactory().createPoint(closestPoints[0]); } catch (Exception e) { throw new IllegalGeometryException( - "ST_ClosestPoint doesn't support empty geometry object.", left); + "ST_ClosestPoint doesn't support empty geometry object.", new Geometry[] {left, right}); } } @@ -1109,7 +1111,12 @@ public static double lineLocatePoint(Geometry geom, Geometry point) { } public static Geometry locateAlong(Geometry linear, double measure, double offset) { - return GeometryLocateAlongProcessor.processGeometry(linear, measure, offset); + try { + return GeometryLocateAlongProcessor.processGeometry(linear, measure, offset); + } catch (Exception e) { + throw new IllegalGeometryException( + "ST_LocateAlong failed to evaluate. " + e.getMessage(), new Geometry[] {linear}); + } } public static Geometry locateAlong(Geometry linear, double measure) { @@ -1540,7 +1547,8 @@ public static Geometry makeLine(Geometry[] geoms) { } } else { throw new IllegalGeometryException( - "ST_MakeLine only supports Point, MultiPoint and LineString geometries", geom); + "ST_MakeLine only supports Point, MultiPoint and LineString geometries", + new Geometry[] {geom}); } } @@ -1625,7 +1633,8 @@ public static Geometry collectionExtract(Geometry geometry, Integer geomType) { emptyResult = factory.createMultiPolygon(); break; default: - throw new IllegalGeometryException("Invalid geometry type: " + geomType, geometry); + throw new IllegalGeometryException( + "Invalid geometry type: " + geomType, new Geometry[] {geometry}); } List geometries = GeomUtils.extractGeometryCollection(geometry, geomClass); if (geometries.isEmpty()) { @@ -1773,7 +1782,7 @@ public static int numPoints(Geometry geometry) throws Exception { if (!(Geometry.TYPENAME_LINESTRING.equalsIgnoreCase(geometryType))) { throw new IllegalGeometryException( "Unsupported geometry type: " + geometryType + ", only LineString geometry is supported.", - geometry); + new Geometry[] {geometry}); } return geometry.getNumPoints(); } @@ -1821,7 +1830,7 @@ public static Integer nRings(Geometry geometry) throws Exception { "Unsupported geometry type: " + geometryType + ", only Polygon or MultiPolygon geometries are supported.", - geometry); + new Geometry[] {geometry}); } int numRings = 0; if (geometry instanceof Polygon) { @@ -1887,7 +1896,8 @@ public static Geometry geometricMedian( String geometryType = geometry.getGeometryType(); if (!(Geometry.TYPENAME_POINT.equals(geometryType) || Geometry.TYPENAME_MULTIPOINT.equals(geometryType))) { - throw new IllegalGeometryException("Unsupported geometry type: " + geometryType, geometry); + throw new IllegalGeometryException( + "Unsupported geometry type: " + geometryType, new Geometry[] {geometry}); } Coordinate[] coordinates = extractCoordinates(geometry); if (coordinates.length == 0) return new Point(null, factory); @@ -1901,7 +1911,7 @@ public static Geometry geometricMedian( throw new IllegalGeometryException( String.format( "Median failed to converge within %.1E after %d iterations.", tolerance, maxIter), - geometry); + new Geometry[] {geometry}); boolean is3d = !Double.isNaN(geometry.getCoordinate().z); if (!is3d) median.z = Double.NaN; return factory.createPoint(median); @@ -1965,19 +1975,21 @@ public static Geometry boundingDiagonal(Geometry geometry) { } public static double angle(Geometry point1, Geometry point2, Geometry point3, Geometry point4) - throws IllegalArgumentException { + throws IllegalGeometryException { if (point3 == null && point4 == null) return Functions.angle(point1, point2); else if (point4 == null) return Functions.angle(point1, point2, point3); if (GeomUtils.isAnyGeomEmpty(point1, point2, point3, point4)) - throw new IllegalArgumentException( + throw new IllegalGeometryException( "ST_Angle cannot support empty geometries, empty index = " - + Arrays.toString(GeomUtils.emptyGeometries(point1, point2, point3, point4))); + + Arrays.toString(GeomUtils.emptyGeometries(point1, point2, point3, point4)), + new Geometry[] {point1, point2, point3, point4}); if (!(point1 instanceof Point && point2 instanceof Point && point3 instanceof Point && point4 instanceof Point)) - throw new IllegalArgumentException( - "ST_Angle supports either only POINT or only LINESTRING geometries."); + throw new IllegalGeometryException( + "ST_Angle supports either only POINT or only LINESTRING geometries.", + new Geometry[] {point1, point2, point3, point4}); return GeomUtils.calcAngle( point1.getCoordinate(), point2.getCoordinate(), @@ -1986,14 +1998,16 @@ public static double angle(Geometry point1, Geometry point2, Geometry point3, Ge } public static double angle(Geometry point1, Geometry point2, Geometry point3) - throws IllegalArgumentException { + throws IllegalGeometryException { if (GeomUtils.isAnyGeomEmpty(point1, point2, point3)) - throw new IllegalArgumentException( + throw new IllegalGeometryException( "ST_Angle cannot support empty geometries, empty index = " - + Arrays.toString(GeomUtils.emptyGeometries(point1, point2, point3))); + + Arrays.toString(GeomUtils.emptyGeometries(point1, point2, point3)), + new Geometry[] {point1, point2, point3}); if (!(point1 instanceof Point && point2 instanceof Point && point3 instanceof Point)) - throw new IllegalArgumentException( - "ST_Angle supports either only POINT or only LINESTRING geometries."); + throw new IllegalGeometryException( + "ST_Angle supports either only POINT or only LINESTRING geometries.", + new Geometry[] {point1, point2, point3}); return GeomUtils.calcAngle( point2.getCoordinate(), point1.getCoordinate(), @@ -2001,12 +2015,14 @@ public static double angle(Geometry point1, Geometry point2, Geometry point3) point3.getCoordinate()); } - public static double angle(Geometry line1, Geometry line2) throws IllegalArgumentException { + public static double angle(Geometry line1, Geometry line2) throws IllegalGeometryException { if (GeomUtils.isAnyGeomEmpty(line1, line2)) - throw new IllegalArgumentException("ST_Angle cannot support empty geometries."); + throw new IllegalGeometryException( + "ST_Angle cannot support empty geometries.", new Geometry[] {line1, line2}); if (!(line1 instanceof LineString && line2 instanceof LineString)) - throw new IllegalArgumentException( - "ST_Angle supports either only POINT or only LINESTRING geometries."); + throw new IllegalGeometryException( + "ST_Angle supports either only POINT or only LINESTRING geometries.", + new Geometry[] {line1, line2}); Coordinate[] startEndLine1 = GeomUtils.getStartEndCoordinates(line1); Coordinate[] startEndLine2 = GeomUtils.getStartEndCoordinates(line2); assert startEndLine1 != null; @@ -2020,11 +2036,19 @@ public static double degrees(double angleInRadian) { } public static Double hausdorffDistance(Geometry g1, Geometry g2, double densityFrac) { - return GeomUtils.getHausdorffDistance(g1, g2, densityFrac); + try { + return GeomUtils.getHausdorffDistance(g1, g2, densityFrac); + } catch (IllegalArgumentException e) { + throw new IllegalGeometryException(e.getMessage(), new Geometry[] {g1, g2}); + } } public static Double hausdorffDistance(Geometry g1, Geometry g2) { - return GeomUtils.getHausdorffDistance(g1, g2, -1); + try { + return GeomUtils.getHausdorffDistance(g1, g2, -1); + } catch (IllegalArgumentException e) { + throw new IllegalGeometryException(e.getMessage(), new Geometry[] {g1, g2}); + } } private static IsValidOp getIsValidOpObject(Geometry geom, int flag) { @@ -2158,7 +2182,7 @@ public static Geometry rotate(Geometry geometry, double angle, double originX, d * @param angle The angle in radians to rotate the geometry. * @param pointOrigin The origin point around which to rotate. * @return The rotated geometry. - * @throws IllegalArgumentException if the pointOrigin is not a Point geometry. + * @throws IllegalGeometryException if the pointOrigin is not a Point geometry. */ public static Geometry rotate(Geometry geometry, double angle, Geometry pointOrigin) { if (geometry == null || geometry.isEmpty()) { @@ -2166,7 +2190,7 @@ public static Geometry rotate(Geometry geometry, double angle, Geometry pointOri } if (pointOrigin == null || pointOrigin.isEmpty() || !(pointOrigin instanceof Point)) { throw new IllegalGeometryException( - "The origin must be a non-empty Point geometry.", pointOrigin); + "The origin must be a non-empty Point geometry.", new Geometry[] {geometry, pointOrigin}); } Point origin = (Point) pointOrigin; double originX = origin.getX(); diff --git a/common/src/main/java/org/apache/sedona/common/FunctionsGeoTools.java b/common/src/main/java/org/apache/sedona/common/FunctionsGeoTools.java index 28bf6b9045..995c580aa8 100644 --- a/common/src/main/java/org/apache/sedona/common/FunctionsGeoTools.java +++ b/common/src/main/java/org/apache/sedona/common/FunctionsGeoTools.java @@ -19,6 +19,7 @@ package org.apache.sedona.common; import java.util.Set; +import org.apache.sedona.common.exception.IllegalGeometryException; import org.geotools.geometry.jts.JTS; import org.geotools.referencing.CRS; import org.geotools.referencing.ReferencingFactoryFinder; @@ -39,19 +40,31 @@ public class FunctionsGeoTools { public static Geometry transform(Geometry geometry, String targetCRS) throws FactoryException, TransformException { - return transform(geometry, null, targetCRS, true); + try { + return transform(geometry, null, targetCRS, true); + } catch (FactoryException e) { + throw new IllegalGeometryException(e.getMessage(), new Geometry[] {geometry}); + } } public static Geometry transform(Geometry geometry, String sourceCRS, String targetCRS) throws FactoryException, TransformException { - return transform(geometry, sourceCRS, targetCRS, true); + try { + return transform(geometry, sourceCRS, targetCRS, true); + } catch (FactoryException e) { + throw new IllegalGeometryException(e.getMessage(), new Geometry[] {geometry}); + } } public static Geometry transform( Geometry geometry, String sourceCRScode, String targetCRScode, boolean lenient) throws FactoryException, TransformException { CoordinateReferenceSystem targetCRS = parseCRSString(targetCRScode); - return transformToGivenTarget(geometry, sourceCRScode, targetCRS, lenient); + try { + return transformToGivenTarget(geometry, sourceCRScode, targetCRS, lenient); + } catch (FactoryException e) { + throw new IllegalGeometryException(e.getMessage(), new Geometry[] {geometry}); + } } /** diff --git a/common/src/main/java/org/apache/sedona/common/exception/IllegalGeometryException.java b/common/src/main/java/org/apache/sedona/common/exception/IllegalGeometryException.java index d05bdee95d..299a2cddba 100644 --- a/common/src/main/java/org/apache/sedona/common/exception/IllegalGeometryException.java +++ b/common/src/main/java/org/apache/sedona/common/exception/IllegalGeometryException.java @@ -18,24 +18,25 @@ */ package org.apache.sedona.common.exception; +import java.util.Arrays; import org.locationtech.jts.geom.Geometry; /** Exception for illegal geometries. */ public class IllegalGeometryException extends RuntimeException { - // The geometry that caused the exception - private Geometry geometry; + // The geometries that caused the exception + private Geometry[] geometries; - public IllegalGeometryException(String message, Geometry geometry) { - super(message + " [GEOM] " + geometry.toString()); - this.geometry = geometry; + public IllegalGeometryException(String message, Geometry[] geometries) { + super(message + " [GEOM] " + Arrays.toString(geometries)); + this.geometries = geometries; } - public IllegalGeometryException(String message, Geometry geometry, Throwable cause) { - super(message + " " + geometry.toString(), cause); - this.geometry = geometry; + public IllegalGeometryException(String message, Geometry[] geometries, Throwable cause) { + super(message + " " + Arrays.toString(geometries), cause); + this.geometries = geometries; } - public Geometry getGeometry() { - return geometry; + public Geometry[] getGeometries() { + return geometries; } } diff --git a/common/src/main/java/org/apache/sedona/common/utils/GeomUtils.java b/common/src/main/java/org/apache/sedona/common/utils/GeomUtils.java index 89a3968fef..234c43a830 100644 --- a/common/src/main/java/org/apache/sedona/common/utils/GeomUtils.java +++ b/common/src/main/java/org/apache/sedona/common/utils/GeomUtils.java @@ -564,7 +564,8 @@ public static Double getHausdorffDistance(Geometry g1, Geometry g2, double densi public static Geometry addMeasure(Geometry geom, double measure_start, double measure_end) { if (!(geom instanceof LineString) && !(geom instanceof MultiLineString)) { - throw new IllegalGeometryException("Geometry must be a LineString or MultiLineString.", geom); + throw new IllegalGeometryException( + "Geometry must be a LineString or MultiLineString.", new Geometry[] {geom}); } if (geom instanceof LineString) { diff --git a/common/src/test/java/org/apache/sedona/common/FunctionsTest.java b/common/src/test/java/org/apache/sedona/common/FunctionsTest.java index 3868b45395..d3c0e35ce3 100644 --- a/common/src/test/java/org/apache/sedona/common/FunctionsTest.java +++ b/common/src/test/java/org/apache/sedona/common/FunctionsTest.java @@ -26,6 +26,7 @@ import com.google.common.math.DoubleMath; import java.util.*; import java.util.stream.Collectors; +import org.apache.sedona.common.exception.IllegalGeometryException; import org.apache.sedona.common.sphere.Haversine; import org.apache.sedona.common.sphere.Spheroid; import org.apache.sedona.common.utils.*; @@ -1216,7 +1217,9 @@ public void geometricMedianTolerance() throws Exception { public void geometricMedianUnsupported() { LineString lineString = GEOMETRY_FACTORY.createLineString(coordArray(1480, 0, 620, 0)); Exception e = assertThrows(Exception.class, () -> Functions.geometricMedian(lineString)); - assertEquals("Unsupported geometry type: LineString", e.getMessage()); + assertEquals( + "Unsupported geometry type: LineString [GEOM] [LINESTRING (1480 0, 620 0)]", + e.getMessage()); } @Test @@ -1226,7 +1229,9 @@ public void geometricMedianFailConverge() { coordArray(12, 5, 62, 7, 100, -1, 100, -5, 10, 20, 105, -5)); Exception e = assertThrows(Exception.class, () -> Functions.geometricMedian(multiPoint, 1e-6, 5, true)); - assertEquals("Median failed to converge within 1.0E-06 after 5 iterations.", e.getMessage()); + assertEquals( + "Median failed to converge within 1.0E-06 after 5 iterations. [GEOM] [MULTIPOINT ((12 5), (62 7), (100 -1), (100 -5), (10 20), (105 -5))]", + e.getMessage()); } @Test @@ -1580,8 +1585,10 @@ public void numPoints() throws Exception { public void numPointsUnsupported() throws Exception { Polygon polygon = GEOMETRY_FACTORY.createPolygon(coordArray(0, 0, 0, 90, 0, 0)); String expected = - "Unsupported geometry type: " + "Polygon" + ", only LineString geometry is supported."; - Exception e = assertThrows(IllegalArgumentException.class, () -> Functions.numPoints(polygon)); + "Unsupported geometry type: " + + "Polygon" + + ", only LineString geometry is supported. [GEOM] [POLYGON ((0 0, 0 90, 0 0))]"; + Exception e = assertThrows(IllegalGeometryException.class, () -> Functions.numPoints(polygon)); assertEquals(expected, e.getMessage()); } @@ -2018,9 +2025,10 @@ public void makeLineWithWrongType() { Polygon polygon2 = GEOMETRY_FACTORY.createPolygon(coordArray(0, 0, 0, 10, 10, 10, 10, 0, 0, 0)); Exception e = - assertThrows(IllegalArgumentException.class, () -> Functions.makeLine(polygon1, polygon2)); + assertThrows(IllegalGeometryException.class, () -> Functions.makeLine(polygon1, polygon2)); assertEquals( - "ST_MakeLine only supports Point, MultiPoint and LineString geometries", e.getMessage()); + "ST_MakeLine only supports Point, MultiPoint and LineString geometries [GEOM] [POLYGON ((0 0, 0 90, 0 0))]", + e.getMessage()); } @Test @@ -2530,8 +2538,8 @@ public void nRingsUnsupported() { String expected = "Unsupported geometry type: " + "LineString" - + ", only Polygon or MultiPolygon geometries are supported."; - Exception e = assertThrows(IllegalArgumentException.class, () -> Functions.nRings(lineString)); + + ", only Polygon or MultiPolygon geometries are supported. [GEOM] [LINESTRING (0 1, 1 2, 1 2)]"; + Exception e = assertThrows(IllegalGeometryException.class, () -> Functions.nRings(lineString)); assertEquals(expected, e.getMessage()); } @@ -2864,8 +2872,10 @@ public void angleInvalidEmptyGeom() { Point point3 = GEOMETRY_FACTORY.createPoint(new Coordinate(1, 1)); Exception e = - assertThrows(IllegalArgumentException.class, () -> Functions.angle(point1, point2, point3)); - assertEquals("ST_Angle cannot support empty geometries.", e.getMessage()); + assertThrows(IllegalGeometryException.class, () -> Functions.angle(point1, point2, point3)); + assertEquals( + "ST_Angle cannot support empty geometries, empty index = [0] [GEOM] [POINT (3 5), POINT EMPTY, POINT (1 1)]", + e.getMessage()); } @Test @@ -2876,9 +2886,10 @@ public void angleInvalidUnsupportedGeom() { Exception e = assertThrows( - IllegalArgumentException.class, () -> Functions.angle(point1, polygon, point3)); + IllegalGeometryException.class, () -> Functions.angle(point1, polygon, point3)); assertEquals( - "ST_Angle supports either only POINT or only LINESTRING geometries.", e.getMessage()); + "ST_Angle supports either only POINT or only LINESTRING geometries. [GEOM] [POINT (3 5), POLYGON ((1 0, 1 1, 2 1, 2 0, 1 0)), POINT (1 1)]", + e.getMessage()); } @Test @@ -3215,19 +3226,22 @@ public void closestPointEmpty() { // One of the object is empty Point point = GEOMETRY_FACTORY.createPoint(new Coordinate(1, 1)); LineString emptyLineString = GEOMETRY_FACTORY.createLineString(); - String expected = "ST_ClosestPoint doesn't support empty geometry object."; + String expected1 = + "ST_ClosestPoint doesn't support empty geometry object. [GEOM] [POINT (1 1), LINESTRING EMPTY]"; Exception e1 = assertThrows( - IllegalArgumentException.class, () -> Functions.closestPoint(point, emptyLineString)); - assertEquals(expected, e1.getMessage()); + IllegalGeometryException.class, () -> Functions.closestPoint(point, emptyLineString)); + assertEquals(expected1, e1.getMessage()); // Both objects are empty Polygon emptyPolygon = GEOMETRY_FACTORY.createPolygon(); + String expected2 = + "ST_ClosestPoint doesn't support empty geometry object. [GEOM] [POLYGON EMPTY, LINESTRING EMPTY]"; Exception e2 = assertThrows( - IllegalArgumentException.class, + IllegalGeometryException.class, () -> Functions.closestPoint(emptyPolygon, emptyLineString)); - assertEquals(expected, e2.getMessage()); + assertEquals(expected2, e2.getMessage()); } @Test @@ -3275,9 +3289,10 @@ public void hausdorffDistanceInvalidDensityFrac() throws Exception { LineString lineString = GEOMETRY_FACTORY.createLineString(coordArray(1, 2, 1, 5, 2, 6, 1, 2)); Exception e = assertThrows( - IllegalArgumentException.class, + IllegalGeometryException.class, () -> Functions.hausdorffDistance(point, lineString, 3)); - String expected = "Fraction is not in range (0.0 - 1.0]"; + String expected = + "Fraction is not in range (0.0 - 1.0] [GEOM] [POINT (10 34), LINESTRING (1 2, 1 5, 2 6, 1 2)]"; String actual = e.getMessage(); assertEquals(expected, actual); } @@ -3356,7 +3371,7 @@ public void transform() throws FactoryException, TransformException { // The source CRS is an invalid SRID e = assertThrows( - FactoryException.class, + IllegalGeometryException.class, () -> FunctionsGeoTools.transform(geomExpected, "abcde", "EPSG:3857")); assertTrue(e.getMessage().contains("First failed to read as a well-known CRS code")); @@ -3485,9 +3500,9 @@ public void locateAlong() throws ParseException { geom = Constructors.geomFromEWKT("POLYGON M((0 0 1, 1 1 1, 5 1 1, 5 0 1, 1 0 1, 0 0 1))"); Geometry finalGeom = geom; Exception e = - assertThrows(IllegalArgumentException.class, () -> Functions.locateAlong(finalGeom, 1)); + assertThrows(IllegalGeometryException.class, () -> Functions.locateAlong(finalGeom, 1)); assertEquals( - "Polygon geometry type not supported, supported types are: (Multi)Point and (Multi)LineString.", + "ST_LocateAlong failed to evaluate. Polygon geometry type not supported, supported types are: (Multi)Point and (Multi)LineString. [GEOM] [POLYGON ((0 0, 1 1, 5 1, 5 0, 1 0, 0 0))]", e.getMessage()); } diff --git a/spark/common/src/test/scala/org/apache/sedona/sql/dataFrameAPITestScala.scala b/spark/common/src/test/scala/org/apache/sedona/sql/dataFrameAPITestScala.scala index 11465afa93..3c8a84b1a3 100644 --- a/spark/common/src/test/scala/org/apache/sedona/sql/dataFrameAPITestScala.scala +++ b/spark/common/src/test/scala/org/apache/sedona/sql/dataFrameAPITestScala.scala @@ -2204,7 +2204,7 @@ class dataFrameAPITestScala extends TestBaseScala { baseDf.select(ST_Rotate("geom1", 50, "geom2")).take(1) } // Check the exception geometry - assert(exception.getGeometry != null) + assert(exception.getGeometries != null && exception.getGeometries.length > 0) // Check the exception message assert(exception.getMessage.contains( "The origin must be a non-empty Point geometry. [GEOM] POLYGON ((0 0, 1 0, 1 1, 0 1, 1 1, 0 0))")) From 59181ac49f10824c97a8da39095e22b3da4ca4ed Mon Sep 17 00:00:00 2001 From: zhangfengcdt Date: Thu, 18 Jul 2024 09:19:26 -0700 Subject: [PATCH 06/10] fix additional tests --- .../scala/org/apache/sedona/sql/dataFrameAPITestScala.scala | 2 +- .../test/scala/org/apache/sedona/sql/functionTestScala.scala | 5 +++-- 2 files changed, 4 insertions(+), 3 deletions(-) diff --git a/spark/common/src/test/scala/org/apache/sedona/sql/dataFrameAPITestScala.scala b/spark/common/src/test/scala/org/apache/sedona/sql/dataFrameAPITestScala.scala index 3c8a84b1a3..a47a761541 100644 --- a/spark/common/src/test/scala/org/apache/sedona/sql/dataFrameAPITestScala.scala +++ b/spark/common/src/test/scala/org/apache/sedona/sql/dataFrameAPITestScala.scala @@ -2207,7 +2207,7 @@ class dataFrameAPITestScala extends TestBaseScala { assert(exception.getGeometries != null && exception.getGeometries.length > 0) // Check the exception message assert(exception.getMessage.contains( - "The origin must be a non-empty Point geometry. [GEOM] POLYGON ((0 0, 1 0, 1 1, 0 1, 1 1, 0 0))")) + "The origin must be a non-empty Point geometry. [GEOM] [POLYGON ((0 0, 2 0, 2 2, 0 2, 1 1, 0 0)), POLYGON ((0 0, 1 0, 1 1, 0 1, 1 1, 0 0))]")) } } } diff --git a/spark/common/src/test/scala/org/apache/sedona/sql/functionTestScala.scala b/spark/common/src/test/scala/org/apache/sedona/sql/functionTestScala.scala index ea399f4f15..7cf0c0eb29 100644 --- a/spark/common/src/test/scala/org/apache/sedona/sql/functionTestScala.scala +++ b/spark/common/src/test/scala/org/apache/sedona/sql/functionTestScala.scala @@ -20,6 +20,7 @@ package org.apache.sedona.sql import org.apache.commons.codec.binary.Hex import org.apache.sedona.common.FunctionsGeoTools +import org.apache.sedona.common.exception.IllegalGeometryException import org.apache.sedona.sql.implicits._ import org.apache.spark.sql.catalyst.expressions.GenericRowWithSchema import org.apache.spark.sql.functions._ @@ -410,11 +411,11 @@ class functionTestScala val reader = new WKTReader val polygeom = reader.read(polygon) - intercept[FactoryException] { + intercept[IllegalGeometryException] { val d = FunctionsGeoTools.transform(polygeom, epsgString, epsgFactoryErrorString) } - intercept[FactoryException] { + intercept[IllegalGeometryException] { val d2 = FunctionsGeoTools.transform(polygeom, epsgString, epsgNoSuchAuthorityString) } From 0df050c9dbd09d4591eb6268aa565fcb239edd25 Mon Sep 17 00:00:00 2001 From: zhangfengcdt Date: Fri, 19 Jul 2024 15:57:52 -0700 Subject: [PATCH 07/10] refactor to add exception handling in InferredExpression --- .../org/apache/sedona/common/Functions.java | 105 ++++++------------ .../sedona/common/FunctionsGeoTools.java | 19 +--- .../exception/IllegalGeometryException.java | 42 ------- .../apache/sedona/common/utils/GeomUtils.java | 13 +-- .../apache/sedona/common/FunctionsTest.java | 59 ++++------ .../expressions/InferredExpression.scala | 53 ++++++++- .../sedona/sql/dataFrameAPITestScala.scala | 16 --- .../apache/sedona/sql/functionTestScala.scala | 5 +- 8 files changed, 113 insertions(+), 199 deletions(-) delete mode 100644 common/src/main/java/org/apache/sedona/common/exception/IllegalGeometryException.java diff --git a/common/src/main/java/org/apache/sedona/common/Functions.java b/common/src/main/java/org/apache/sedona/common/Functions.java index 37a8fdc9dc..f0ffd0bc57 100644 --- a/common/src/main/java/org/apache/sedona/common/Functions.java +++ b/common/src/main/java/org/apache/sedona/common/Functions.java @@ -27,7 +27,6 @@ import java.util.*; import java.util.stream.Collectors; import org.apache.commons.lang3.tuple.Pair; -import org.apache.sedona.common.exception.IllegalGeometryException; import org.apache.sedona.common.geometryObjects.Circle; import org.apache.sedona.common.sphere.Spheroid; import org.apache.sedona.common.subDivide.GeometrySubDivider; @@ -101,7 +100,7 @@ public static Geometry buffer(Geometry geometry, double radius, boolean useSpher public static Geometry buffer( Geometry geometry, double radius, boolean useSpheroid, String params) - throws IllegalGeometryException { + throws IllegalArgumentException { BufferParameters bufferParameters = new BufferParameters(); // Processing parameters @@ -123,8 +122,7 @@ public static Geometry buffer( try { return bufferSpheroid(geometry, radius, bufferParameters); } catch (RuntimeException e) { - throw new IllegalGeometryException( - "Error processing spheroidal buffer" + e.getMessage(), new Geometry[] {geometry}); + throw new RuntimeException("Error processing spheroidal buffer", e); } } else { // Existing planar buffer logic with params handling @@ -254,9 +252,8 @@ public static int bestSRID(Geometry geometry) throws IllegalArgumentException { Double xwidth = Spheroid.angularWidth(envelope); Double ywidth = Spheroid.angularHeight(envelope); if (xwidth.isNaN() | ywidth.isNaN()) { - throw new IllegalGeometryException( - "Only lon/lat coordinate systems are supported by ST_BestSRID", - new Geometry[] {geometry}); + throw new IllegalArgumentException( + "Only lon/lat coordinate systems are supported by ST_BestSRID"); } // Prioritize polar regions for Lambert Azimuthal Equal Area projection @@ -600,9 +597,8 @@ public static String asHexEWKB(Geometry geom, String endian) { } else if (endian.equalsIgnoreCase("XDR")) { return GeomUtils.getHexEWKB(geom, ByteOrderValues.BIG_ENDIAN); } - throw new IllegalGeometryException( - "You must select either NDR (little-endian) or XDR (big-endian) as the endian format.", - new Geometry[] {geom}); + throw new IllegalArgumentException( + "You must select either NDR (little-endian) or XDR (big-endian) as the endian format."); } public static String asHexEWKB(Geometry geom) { @@ -825,8 +821,7 @@ public static Geometry closestPoint(Geometry left, Geometry right) { Coordinate[] closestPoints = distanceOp.nearestPoints(); return left.getFactory().createPoint(closestPoints[0]); } catch (Exception e) { - throw new IllegalGeometryException( - "ST_ClosestPoint doesn't support empty geometry object.", new Geometry[] {left, right}); + throw new IllegalArgumentException("ST_ClosestPoint doesn't support empty geometry object."); } } @@ -1111,12 +1106,7 @@ public static double lineLocatePoint(Geometry geom, Geometry point) { } public static Geometry locateAlong(Geometry linear, double measure, double offset) { - try { - return GeometryLocateAlongProcessor.processGeometry(linear, measure, offset); - } catch (Exception e) { - throw new IllegalGeometryException( - "ST_LocateAlong failed to evaluate. " + e.getMessage(), new Geometry[] {linear}); - } + return GeometryLocateAlongProcessor.processGeometry(linear, measure, offset); } public static Geometry locateAlong(Geometry linear, double measure) { @@ -1546,9 +1536,8 @@ public static Geometry makeLine(Geometry[] geoms) { coordinates.add(coord); } } else { - throw new IllegalGeometryException( - "ST_MakeLine only supports Point, MultiPoint and LineString geometries", - new Geometry[] {geom}); + throw new IllegalArgumentException( + "ST_MakeLine only supports Point, MultiPoint and LineString geometries"); } } @@ -1633,8 +1622,7 @@ public static Geometry collectionExtract(Geometry geometry, Integer geomType) { emptyResult = factory.createMultiPolygon(); break; default: - throw new IllegalGeometryException( - "Invalid geometry type: " + geomType, new Geometry[] {geometry}); + throw new IllegalArgumentException("Invalid geometry type"); } List geometries = GeomUtils.extractGeometryCollection(geometry, geomClass); if (geometries.isEmpty()) { @@ -1780,9 +1768,10 @@ private static Coordinate[] extractCoordinates(Geometry geometry) { public static int numPoints(Geometry geometry) throws Exception { String geometryType = geometry.getGeometryType(); if (!(Geometry.TYPENAME_LINESTRING.equalsIgnoreCase(geometryType))) { - throw new IllegalGeometryException( - "Unsupported geometry type: " + geometryType + ", only LineString geometry is supported.", - new Geometry[] {geometry}); + throw new IllegalArgumentException( + "Unsupported geometry type: " + + geometryType + + ", only LineString geometry is supported."); } return geometry.getNumPoints(); } @@ -1844,11 +1833,10 @@ public static Geometry generatePoints(Geometry geom, int numPoints, Random rando public static Integer nRings(Geometry geometry) throws Exception { String geometryType = geometry.getGeometryType(); if (!(geometry instanceof Polygon || geometry instanceof MultiPolygon)) { - throw new IllegalGeometryException( + throw new IllegalArgumentException( "Unsupported geometry type: " + geometryType - + ", only Polygon or MultiPolygon geometries are supported.", - new Geometry[] {geometry}); + + ", only Polygon or MultiPolygon geometries are supported."); } int numRings = 0; if (geometry instanceof Polygon) { @@ -1914,8 +1902,7 @@ public static Geometry geometricMedian( String geometryType = geometry.getGeometryType(); if (!(Geometry.TYPENAME_POINT.equals(geometryType) || Geometry.TYPENAME_MULTIPOINT.equals(geometryType))) { - throw new IllegalGeometryException( - "Unsupported geometry type: " + geometryType, new Geometry[] {geometry}); + throw new Exception("Unsupported geometry type: " + geometryType); } Coordinate[] coordinates = extractCoordinates(geometry); if (coordinates.length == 0) return new Point(null, factory); @@ -1926,10 +1913,9 @@ public static Geometry geometricMedian( for (int i = 0; i < maxIter && delta > tolerance; i++) delta = iteratePoints(median, coordinates, distances); if (failIfNotConverged && delta > tolerance) - throw new IllegalGeometryException( + throw new Exception( String.format( - "Median failed to converge within %.1E after %d iterations.", tolerance, maxIter), - new Geometry[] {geometry}); + "Median failed to converge within %.1E after %d iterations.", tolerance, maxIter)); boolean is3d = !Double.isNaN(geometry.getCoordinate().z); if (!is3d) median.z = Double.NaN; return factory.createPoint(median); @@ -1993,21 +1979,17 @@ public static Geometry boundingDiagonal(Geometry geometry) { } public static double angle(Geometry point1, Geometry point2, Geometry point3, Geometry point4) - throws IllegalGeometryException { + throws IllegalArgumentException { if (point3 == null && point4 == null) return Functions.angle(point1, point2); else if (point4 == null) return Functions.angle(point1, point2, point3); if (GeomUtils.isAnyGeomEmpty(point1, point2, point3, point4)) - throw new IllegalGeometryException( - "ST_Angle cannot support empty geometries, empty index = " - + Arrays.toString(GeomUtils.emptyGeometries(point1, point2, point3, point4)), - new Geometry[] {point1, point2, point3, point4}); + throw new IllegalArgumentException("ST_Angle cannot support empty geometries."); if (!(point1 instanceof Point && point2 instanceof Point && point3 instanceof Point && point4 instanceof Point)) - throw new IllegalGeometryException( - "ST_Angle supports either only POINT or only LINESTRING geometries.", - new Geometry[] {point1, point2, point3, point4}); + throw new IllegalArgumentException( + "ST_Angle supports either only POINT or only LINESTRING geometries."); return GeomUtils.calcAngle( point1.getCoordinate(), point2.getCoordinate(), @@ -2016,16 +1998,12 @@ public static double angle(Geometry point1, Geometry point2, Geometry point3, Ge } public static double angle(Geometry point1, Geometry point2, Geometry point3) - throws IllegalGeometryException { + throws IllegalArgumentException { if (GeomUtils.isAnyGeomEmpty(point1, point2, point3)) - throw new IllegalGeometryException( - "ST_Angle cannot support empty geometries, empty index = " - + Arrays.toString(GeomUtils.emptyGeometries(point1, point2, point3)), - new Geometry[] {point1, point2, point3}); + throw new IllegalArgumentException("ST_Angle cannot support empty geometries."); if (!(point1 instanceof Point && point2 instanceof Point && point3 instanceof Point)) - throw new IllegalGeometryException( - "ST_Angle supports either only POINT or only LINESTRING geometries.", - new Geometry[] {point1, point2, point3}); + throw new IllegalArgumentException( + "ST_Angle supports either only POINT or only LINESTRING geometries."); return GeomUtils.calcAngle( point2.getCoordinate(), point1.getCoordinate(), @@ -2033,14 +2011,12 @@ public static double angle(Geometry point1, Geometry point2, Geometry point3) point3.getCoordinate()); } - public static double angle(Geometry line1, Geometry line2) throws IllegalGeometryException { + public static double angle(Geometry line1, Geometry line2) throws IllegalArgumentException { if (GeomUtils.isAnyGeomEmpty(line1, line2)) - throw new IllegalGeometryException( - "ST_Angle cannot support empty geometries.", new Geometry[] {line1, line2}); + throw new IllegalArgumentException("ST_Angle cannot support empty geometries."); if (!(line1 instanceof LineString && line2 instanceof LineString)) - throw new IllegalGeometryException( - "ST_Angle supports either only POINT or only LINESTRING geometries.", - new Geometry[] {line1, line2}); + throw new IllegalArgumentException( + "ST_Angle supports either only POINT or only LINESTRING geometries."); Coordinate[] startEndLine1 = GeomUtils.getStartEndCoordinates(line1); Coordinate[] startEndLine2 = GeomUtils.getStartEndCoordinates(line2); assert startEndLine1 != null; @@ -2054,19 +2030,11 @@ public static double degrees(double angleInRadian) { } public static Double hausdorffDistance(Geometry g1, Geometry g2, double densityFrac) { - try { - return GeomUtils.getHausdorffDistance(g1, g2, densityFrac); - } catch (IllegalArgumentException e) { - throw new IllegalGeometryException(e.getMessage(), new Geometry[] {g1, g2}); - } + return GeomUtils.getHausdorffDistance(g1, g2, densityFrac); } public static Double hausdorffDistance(Geometry g1, Geometry g2) { - try { - return GeomUtils.getHausdorffDistance(g1, g2, -1); - } catch (IllegalArgumentException e) { - throw new IllegalGeometryException(e.getMessage(), new Geometry[] {g1, g2}); - } + return GeomUtils.getHausdorffDistance(g1, g2, -1); } private static IsValidOp getIsValidOpObject(Geometry geom, int flag) { @@ -2200,15 +2168,14 @@ public static Geometry rotate(Geometry geometry, double angle, double originX, d * @param angle The angle in radians to rotate the geometry. * @param pointOrigin The origin point around which to rotate. * @return The rotated geometry. - * @throws IllegalGeometryException if the pointOrigin is not a Point geometry. + * @throws IllegalArgumentException if the pointOrigin is not a Point geometry. */ public static Geometry rotate(Geometry geometry, double angle, Geometry pointOrigin) { if (geometry == null || geometry.isEmpty()) { return geometry; } if (pointOrigin == null || pointOrigin.isEmpty() || !(pointOrigin instanceof Point)) { - throw new IllegalGeometryException( - "The origin must be a non-empty Point geometry.", new Geometry[] {geometry, pointOrigin}); + throw new IllegalArgumentException("The origin must be a non-empty Point geometry."); } Point origin = (Point) pointOrigin; double originX = origin.getX(); diff --git a/common/src/main/java/org/apache/sedona/common/FunctionsGeoTools.java b/common/src/main/java/org/apache/sedona/common/FunctionsGeoTools.java index 995c580aa8..28bf6b9045 100644 --- a/common/src/main/java/org/apache/sedona/common/FunctionsGeoTools.java +++ b/common/src/main/java/org/apache/sedona/common/FunctionsGeoTools.java @@ -19,7 +19,6 @@ package org.apache.sedona.common; import java.util.Set; -import org.apache.sedona.common.exception.IllegalGeometryException; import org.geotools.geometry.jts.JTS; import org.geotools.referencing.CRS; import org.geotools.referencing.ReferencingFactoryFinder; @@ -40,31 +39,19 @@ public class FunctionsGeoTools { public static Geometry transform(Geometry geometry, String targetCRS) throws FactoryException, TransformException { - try { - return transform(geometry, null, targetCRS, true); - } catch (FactoryException e) { - throw new IllegalGeometryException(e.getMessage(), new Geometry[] {geometry}); - } + return transform(geometry, null, targetCRS, true); } public static Geometry transform(Geometry geometry, String sourceCRS, String targetCRS) throws FactoryException, TransformException { - try { - return transform(geometry, sourceCRS, targetCRS, true); - } catch (FactoryException e) { - throw new IllegalGeometryException(e.getMessage(), new Geometry[] {geometry}); - } + return transform(geometry, sourceCRS, targetCRS, true); } public static Geometry transform( Geometry geometry, String sourceCRScode, String targetCRScode, boolean lenient) throws FactoryException, TransformException { CoordinateReferenceSystem targetCRS = parseCRSString(targetCRScode); - try { - return transformToGivenTarget(geometry, sourceCRScode, targetCRS, lenient); - } catch (FactoryException e) { - throw new IllegalGeometryException(e.getMessage(), new Geometry[] {geometry}); - } + return transformToGivenTarget(geometry, sourceCRScode, targetCRS, lenient); } /** diff --git a/common/src/main/java/org/apache/sedona/common/exception/IllegalGeometryException.java b/common/src/main/java/org/apache/sedona/common/exception/IllegalGeometryException.java deleted file mode 100644 index 299a2cddba..0000000000 --- a/common/src/main/java/org/apache/sedona/common/exception/IllegalGeometryException.java +++ /dev/null @@ -1,42 +0,0 @@ -/* - * 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.sedona.common.exception; - -import java.util.Arrays; -import org.locationtech.jts.geom.Geometry; - -/** Exception for illegal geometries. */ -public class IllegalGeometryException extends RuntimeException { - // The geometries that caused the exception - private Geometry[] geometries; - - public IllegalGeometryException(String message, Geometry[] geometries) { - super(message + " [GEOM] " + Arrays.toString(geometries)); - this.geometries = geometries; - } - - public IllegalGeometryException(String message, Geometry[] geometries, Throwable cause) { - super(message + " " + Arrays.toString(geometries), cause); - this.geometries = geometries; - } - - public Geometry[] getGeometries() { - return geometries; - } -} diff --git a/common/src/main/java/org/apache/sedona/common/utils/GeomUtils.java b/common/src/main/java/org/apache/sedona/common/utils/GeomUtils.java index 234c43a830..9ec7d47398 100644 --- a/common/src/main/java/org/apache/sedona/common/utils/GeomUtils.java +++ b/common/src/main/java/org/apache/sedona/common/utils/GeomUtils.java @@ -23,7 +23,6 @@ import java.nio.ByteOrder; import java.util.*; import org.apache.sedona.common.Functions; -import org.apache.sedona.common.exception.IllegalGeometryException; import org.locationtech.jts.algorithm.Angle; import org.locationtech.jts.algorithm.distance.DiscreteFrechetDistance; import org.locationtech.jts.algorithm.distance.DiscreteHausdorffDistance; @@ -480,15 +479,6 @@ public static boolean isAnyGeomEmpty(Geometry... geometries) { return false; } - public static int[] emptyGeometries(Geometry... geometries) { - List emptyGeometries = new ArrayList<>(); - int i = 0; - for (Geometry geometry : geometries) { - if (geometry != null) if (geometry.isEmpty()) emptyGeometries.add(i); - } - return emptyGeometries.stream().mapToInt(Integer::intValue).toArray(); - } - public static Coordinate[] getStartEndCoordinates(Geometry line) { if (line.getNumPoints() < 2) return null; Coordinate[] coordinates = line.getCoordinates(); @@ -564,8 +554,7 @@ public static Double getHausdorffDistance(Geometry g1, Geometry g2, double densi public static Geometry addMeasure(Geometry geom, double measure_start, double measure_end) { if (!(geom instanceof LineString) && !(geom instanceof MultiLineString)) { - throw new IllegalGeometryException( - "Geometry must be a LineString or MultiLineString.", new Geometry[] {geom}); + throw new IllegalArgumentException("Geometry must be a LineString or MultiLineString."); } if (geom instanceof LineString) { diff --git a/common/src/test/java/org/apache/sedona/common/FunctionsTest.java b/common/src/test/java/org/apache/sedona/common/FunctionsTest.java index a85712586c..a4a3a8c9a9 100644 --- a/common/src/test/java/org/apache/sedona/common/FunctionsTest.java +++ b/common/src/test/java/org/apache/sedona/common/FunctionsTest.java @@ -26,7 +26,6 @@ import com.google.common.math.DoubleMath; import java.util.*; import java.util.stream.Collectors; -import org.apache.sedona.common.exception.IllegalGeometryException; import org.apache.sedona.common.sphere.Haversine; import org.apache.sedona.common.sphere.Spheroid; import org.apache.sedona.common.utils.*; @@ -1217,9 +1216,7 @@ public void geometricMedianTolerance() throws Exception { public void geometricMedianUnsupported() { LineString lineString = GEOMETRY_FACTORY.createLineString(coordArray(1480, 0, 620, 0)); Exception e = assertThrows(Exception.class, () -> Functions.geometricMedian(lineString)); - assertEquals( - "Unsupported geometry type: LineString [GEOM] [LINESTRING (1480 0, 620 0)]", - e.getMessage()); + assertEquals("Unsupported geometry type: LineString", e.getMessage()); } @Test @@ -1229,9 +1226,7 @@ public void geometricMedianFailConverge() { coordArray(12, 5, 62, 7, 100, -1, 100, -5, 10, 20, 105, -5)); Exception e = assertThrows(Exception.class, () -> Functions.geometricMedian(multiPoint, 1e-6, 5, true)); - assertEquals( - "Median failed to converge within 1.0E-06 after 5 iterations. [GEOM] [MULTIPOINT ((12 5), (62 7), (100 -1), (100 -5), (10 20), (105 -5))]", - e.getMessage()); + assertEquals("Median failed to converge within 1.0E-06 after 5 iterations.", e.getMessage()); } @Test @@ -1585,10 +1580,8 @@ public void numPoints() throws Exception { public void numPointsUnsupported() throws Exception { Polygon polygon = GEOMETRY_FACTORY.createPolygon(coordArray(0, 0, 0, 90, 0, 0)); String expected = - "Unsupported geometry type: " - + "Polygon" - + ", only LineString geometry is supported. [GEOM] [POLYGON ((0 0, 0 90, 0 0))]"; - Exception e = assertThrows(IllegalGeometryException.class, () -> Functions.numPoints(polygon)); + "Unsupported geometry type: " + "Polygon" + ", only LineString geometry is supported."; + Exception e = assertThrows(IllegalArgumentException.class, () -> Functions.numPoints(polygon)); assertEquals(expected, e.getMessage()); } @@ -2025,10 +2018,9 @@ public void makeLineWithWrongType() { Polygon polygon2 = GEOMETRY_FACTORY.createPolygon(coordArray(0, 0, 0, 10, 10, 10, 10, 0, 0, 0)); Exception e = - assertThrows(IllegalGeometryException.class, () -> Functions.makeLine(polygon1, polygon2)); + assertThrows(IllegalArgumentException.class, () -> Functions.makeLine(polygon1, polygon2)); assertEquals( - "ST_MakeLine only supports Point, MultiPoint and LineString geometries [GEOM] [POLYGON ((0 0, 0 90, 0 0))]", - e.getMessage()); + "ST_MakeLine only supports Point, MultiPoint and LineString geometries", e.getMessage()); } @Test @@ -2580,8 +2572,8 @@ public void nRingsUnsupported() { String expected = "Unsupported geometry type: " + "LineString" - + ", only Polygon or MultiPolygon geometries are supported. [GEOM] [LINESTRING (0 1, 1 2, 1 2)]"; - Exception e = assertThrows(IllegalGeometryException.class, () -> Functions.nRings(lineString)); + + ", only Polygon or MultiPolygon geometries are supported."; + Exception e = assertThrows(IllegalArgumentException.class, () -> Functions.nRings(lineString)); assertEquals(expected, e.getMessage()); } @@ -2914,10 +2906,8 @@ public void angleInvalidEmptyGeom() { Point point3 = GEOMETRY_FACTORY.createPoint(new Coordinate(1, 1)); Exception e = - assertThrows(IllegalGeometryException.class, () -> Functions.angle(point1, point2, point3)); - assertEquals( - "ST_Angle cannot support empty geometries, empty index = [0] [GEOM] [POINT (3 5), POINT EMPTY, POINT (1 1)]", - e.getMessage()); + assertThrows(IllegalArgumentException.class, () -> Functions.angle(point1, point2, point3)); + assertEquals("ST_Angle cannot support empty geometries.", e.getMessage()); } @Test @@ -2928,10 +2918,9 @@ public void angleInvalidUnsupportedGeom() { Exception e = assertThrows( - IllegalGeometryException.class, () -> Functions.angle(point1, polygon, point3)); + IllegalArgumentException.class, () -> Functions.angle(point1, polygon, point3)); assertEquals( - "ST_Angle supports either only POINT or only LINESTRING geometries. [GEOM] [POINT (3 5), POLYGON ((1 0, 1 1, 2 1, 2 0, 1 0)), POINT (1 1)]", - e.getMessage()); + "ST_Angle supports either only POINT or only LINESTRING geometries.", e.getMessage()); } @Test @@ -3268,22 +3257,19 @@ public void closestPointEmpty() { // One of the object is empty Point point = GEOMETRY_FACTORY.createPoint(new Coordinate(1, 1)); LineString emptyLineString = GEOMETRY_FACTORY.createLineString(); - String expected1 = - "ST_ClosestPoint doesn't support empty geometry object. [GEOM] [POINT (1 1), LINESTRING EMPTY]"; + String expected = "ST_ClosestPoint doesn't support empty geometry object."; Exception e1 = assertThrows( - IllegalGeometryException.class, () -> Functions.closestPoint(point, emptyLineString)); - assertEquals(expected1, e1.getMessage()); + IllegalArgumentException.class, () -> Functions.closestPoint(point, emptyLineString)); + assertEquals(expected, e1.getMessage()); // Both objects are empty Polygon emptyPolygon = GEOMETRY_FACTORY.createPolygon(); - String expected2 = - "ST_ClosestPoint doesn't support empty geometry object. [GEOM] [POLYGON EMPTY, LINESTRING EMPTY]"; Exception e2 = assertThrows( - IllegalGeometryException.class, + IllegalArgumentException.class, () -> Functions.closestPoint(emptyPolygon, emptyLineString)); - assertEquals(expected2, e2.getMessage()); + assertEquals(expected, e2.getMessage()); } @Test @@ -3331,10 +3317,9 @@ public void hausdorffDistanceInvalidDensityFrac() throws Exception { LineString lineString = GEOMETRY_FACTORY.createLineString(coordArray(1, 2, 1, 5, 2, 6, 1, 2)); Exception e = assertThrows( - IllegalGeometryException.class, + IllegalArgumentException.class, () -> Functions.hausdorffDistance(point, lineString, 3)); - String expected = - "Fraction is not in range (0.0 - 1.0] [GEOM] [POINT (10 34), LINESTRING (1 2, 1 5, 2 6, 1 2)]"; + String expected = "Fraction is not in range (0.0 - 1.0]"; String actual = e.getMessage(); assertEquals(expected, actual); } @@ -3413,7 +3398,7 @@ public void transform() throws FactoryException, TransformException { // The source CRS is an invalid SRID e = assertThrows( - IllegalGeometryException.class, + FactoryException.class, () -> FunctionsGeoTools.transform(geomExpected, "abcde", "EPSG:3857")); assertTrue(e.getMessage().contains("First failed to read as a well-known CRS code")); @@ -3542,9 +3527,9 @@ public void locateAlong() throws ParseException { geom = Constructors.geomFromEWKT("POLYGON M((0 0 1, 1 1 1, 5 1 1, 5 0 1, 1 0 1, 0 0 1))"); Geometry finalGeom = geom; Exception e = - assertThrows(IllegalGeometryException.class, () -> Functions.locateAlong(finalGeom, 1)); + assertThrows(IllegalArgumentException.class, () -> Functions.locateAlong(finalGeom, 1)); assertEquals( - "ST_LocateAlong failed to evaluate. Polygon geometry type not supported, supported types are: (Multi)Point and (Multi)LineString. [GEOM] [POLYGON ((0 0, 1 1, 5 1, 5 0, 1 0, 0 0))]", + "Polygon geometry type not supported, supported types are: (Multi)Point and (Multi)LineString.", e.getMessage()); } diff --git a/spark/common/src/main/scala/org/apache/spark/sql/sedona_sql/expressions/InferredExpression.scala b/spark/common/src/main/scala/org/apache/spark/sql/sedona_sql/expressions/InferredExpression.scala index c068b546a7..3ef0dfccbd 100644 --- a/spark/common/src/main/scala/org/apache/spark/sql/sedona_sql/expressions/InferredExpression.scala +++ b/spark/common/src/main/scala/org/apache/spark/sql/sedona_sql/expressions/InferredExpression.scala @@ -19,11 +19,11 @@ package org.apache.spark.sql.sedona_sql.expressions import org.apache.spark.sql.catalyst.InternalRow -import org.apache.spark.sql.catalyst.expressions.{Expression, ImplicitCastInputTypes} +import org.apache.spark.sql.catalyst.expressions.{Expression, ImplicitCastInputTypes, Literal} import org.apache.spark.sql.catalyst.expressions.codegen.CodegenFallback import org.apache.spark.sql.catalyst.util.ArrayData import org.apache.spark.sql.sedona_sql.UDT.GeometryUDT -import org.apache.spark.sql.types.{AbstractDataType, BinaryType, BooleanType, DataType, DataTypes, DoubleType, IntegerType, LongType, StringType} +import org.apache.spark.sql.types.{AbstractDataType, BinaryType, BooleanType, DataType, DataTypes, DoubleType, IntegerType, LongType, StringType, StructField, StructType} import org.apache.spark.unsafe.types.UTF8String import org.locationtech.jts.geom.Geometry import org.apache.spark.sql.sedona_sql.expressions.implicits._ @@ -33,6 +33,12 @@ import scala.reflect.runtime.universe.TypeTag import scala.reflect.runtime.universe.Type import scala.reflect.runtime.universe.typeOf +/** + * Custom exception to include the input row and the original exception message. + */ +class InferredExpressionException(message: String, cause: Throwable) + extends Exception(s"$message, cause: " + cause.getMessage, cause) + /** * This is the base class for wrapping Java/Scala functions as a catalyst expression in Spark SQL. * @param fSeq @@ -74,8 +80,47 @@ abstract class InferredExpression(fSeq: InferrableFunction*) private lazy val argExtractors: Array[InternalRow => Any] = f.buildExtractors(inputExpressions) private lazy val evaluator: InternalRow => Any = f.evaluatorBuilder(argExtractors) - override def eval(input: InternalRow): Any = f.serializer(evaluator(input)) - override def evalWithoutSerialization(input: InternalRow): Any = evaluator(input) + private def findAllLiterals(expression: Expression): Seq[Literal] = { + expression match { + case lit: Literal => Seq(lit) + case _ => expression.children.flatMap(findAllLiterals) + } + } + + private def findAllLiteralsInExpressions(expressions: Seq[Expression]): Seq[String] = { + expressions.flatMap(findAllLiterals).map(_.value.toString) + } + + override def eval(input: InternalRow): Any = { + + try { + f.serializer(evaluator(input)) + } catch { + case e: Exception => + val literalsAsStrings = if (input == null) { + findAllLiteralsInExpressions(inputExpressions) + } else { + Seq.empty[String] + } + val literalsOrInputString = literalsAsStrings.mkString(", ") + throw new InferredExpressionException(s"Exception occurred while evaluating expression - source: [$literalsOrInputString]", e) + } + } + + override def evalWithoutSerialization(input: InternalRow): Any = { + try { + evaluator(input) + } catch { + case e: Exception => + val literalsOrInputStrings = if (input == null) { + findAllLiteralsInExpressions(inputExpressions) + } else { + Seq.empty[String] + } + val literalsOrInputString = literalsOrInputStrings.mkString(", ") + throw new InferredExpressionException(s"Exception occurred while evaluating input row without serialization. Literals found: [$literalsOrInputString]", e) + } + } } // This is a compile time type shield for the types we are able to infer. Anything diff --git a/spark/common/src/test/scala/org/apache/sedona/sql/dataFrameAPITestScala.scala b/spark/common/src/test/scala/org/apache/sedona/sql/dataFrameAPITestScala.scala index f2cde51fb9..d01cb67990 100644 --- a/spark/common/src/test/scala/org/apache/sedona/sql/dataFrameAPITestScala.scala +++ b/spark/common/src/test/scala/org/apache/sedona/sql/dataFrameAPITestScala.scala @@ -19,7 +19,6 @@ package org.apache.sedona.sql import org.apache.commons.codec.binary.Hex -import org.apache.sedona.common.exception.IllegalGeometryException import org.apache.spark.sql.Row import org.apache.spark.sql.functions.{array, col, element_at, lit} import org.apache.spark.sql.sedona_sql.expressions.st_aggregates._ @@ -2216,20 +2215,5 @@ class dataFrameAPITestScala extends TestBaseScala { "SRID=4326;POLYGON ((-0.4546817643920842 0.5948176504236309, 1.4752502925921425 0.0700679430157733, 2 2, 0.0700679430157733 2.5247497074078575, 0.7726591178039579 1.2974088252118154, -0.4546817643920842 0.5948176504236309))" assert(expected.equals(actual)) } - - it("Returning exception with geometry when exception is thrown by ST_Rotate") { - val baseDf = sparkSession.sql( - "SELECT ST_GeomFromEWKT('SRID=4326;POLYGON ((0 0, 2 0, 2 2, 0 2, 1 1, 0 0))') AS geom1, ST_GeomFromEWKT('SRID=4326;POLYGON ((0 0, 1 0, 1 1, 0 1, 1 1, 0 0))') AS geom2") - - // Use intercept to assert that an exception is thrown - val exception = intercept[IllegalGeometryException] { - baseDf.select(ST_Rotate("geom1", 50, "geom2")).take(1) - } - // Check the exception geometry - assert(exception.getGeometries != null && exception.getGeometries.length > 0) - // Check the exception message - assert(exception.getMessage.contains( - "The origin must be a non-empty Point geometry. [GEOM] [POLYGON ((0 0, 2 0, 2 2, 0 2, 1 1, 0 0)), POLYGON ((0 0, 1 0, 1 1, 0 1, 1 1, 0 0))]")) - } } } diff --git a/spark/common/src/test/scala/org/apache/sedona/sql/functionTestScala.scala b/spark/common/src/test/scala/org/apache/sedona/sql/functionTestScala.scala index e81bf9d138..176ab296ad 100644 --- a/spark/common/src/test/scala/org/apache/sedona/sql/functionTestScala.scala +++ b/spark/common/src/test/scala/org/apache/sedona/sql/functionTestScala.scala @@ -20,7 +20,6 @@ package org.apache.sedona.sql import org.apache.commons.codec.binary.Hex import org.apache.sedona.common.FunctionsGeoTools -import org.apache.sedona.common.exception.IllegalGeometryException import org.apache.sedona.sql.implicits._ import org.apache.spark.sql.catalyst.expressions.GenericRowWithSchema import org.apache.spark.sql.functions._ @@ -411,11 +410,11 @@ class functionTestScala val reader = new WKTReader val polygeom = reader.read(polygon) - intercept[IllegalGeometryException] { + intercept[FactoryException] { val d = FunctionsGeoTools.transform(polygeom, epsgString, epsgFactoryErrorString) } - intercept[IllegalGeometryException] { + intercept[FactoryException] { val d2 = FunctionsGeoTools.transform(polygeom, epsgString, epsgNoSuchAuthorityString) } From 9734f98f866cf28471a80e86ebb2cf2f863e4d2e Mon Sep 17 00:00:00 2001 From: zhangfengcdt Date: Fri, 19 Jul 2024 16:09:49 -0700 Subject: [PATCH 08/10] add tests and comments --- .../expressions/InferredExpression.scala | 12 +++++++++--- .../apache/sedona/sql/dataFrameAPITestScala.scala | 15 +++++++++++++++ 2 files changed, 24 insertions(+), 3 deletions(-) diff --git a/spark/common/src/main/scala/org/apache/spark/sql/sedona_sql/expressions/InferredExpression.scala b/spark/common/src/main/scala/org/apache/spark/sql/sedona_sql/expressions/InferredExpression.scala index 3ef0dfccbd..5742658c29 100644 --- a/spark/common/src/main/scala/org/apache/spark/sql/sedona_sql/expressions/InferredExpression.scala +++ b/spark/common/src/main/scala/org/apache/spark/sql/sedona_sql/expressions/InferredExpression.scala @@ -37,7 +37,7 @@ import scala.reflect.runtime.universe.typeOf * Custom exception to include the input row and the original exception message. */ class InferredExpressionException(message: String, cause: Throwable) - extends Exception(s"$message, cause: " + cause.getMessage, cause) + extends Exception(s"$message, cause: " + cause.getMessage, cause) /** * This is the base class for wrapping Java/Scala functions as a catalyst expression in Spark SQL. @@ -98,12 +98,15 @@ abstract class InferredExpression(fSeq: InferrableFunction*) } catch { case e: Exception => val literalsAsStrings = if (input == null) { + // In case no input row is provided, we can't extract literals from the input expressions. findAllLiteralsInExpressions(inputExpressions) } else { Seq.empty[String] } val literalsOrInputString = literalsAsStrings.mkString(", ") - throw new InferredExpressionException(s"Exception occurred while evaluating expression - source: [$literalsOrInputString]", e) + throw new InferredExpressionException( + s"Exception occurred while evaluating expression - source: [$literalsOrInputString]", + e) } } @@ -113,12 +116,15 @@ abstract class InferredExpression(fSeq: InferrableFunction*) } catch { case e: Exception => val literalsOrInputStrings = if (input == null) { + // In case no input row is provided, we can't extract literals from the input expressions. findAllLiteralsInExpressions(inputExpressions) } else { Seq.empty[String] } val literalsOrInputString = literalsOrInputStrings.mkString(", ") - throw new InferredExpressionException(s"Exception occurred while evaluating input row without serialization. Literals found: [$literalsOrInputString]", e) + throw new InferredExpressionException( + s"Exception occurred while evaluating input row without serialization. Literals found: [$literalsOrInputString]", + e) } } } diff --git a/spark/common/src/test/scala/org/apache/sedona/sql/dataFrameAPITestScala.scala b/spark/common/src/test/scala/org/apache/sedona/sql/dataFrameAPITestScala.scala index d01cb67990..4163dda0aa 100644 --- a/spark/common/src/test/scala/org/apache/sedona/sql/dataFrameAPITestScala.scala +++ b/spark/common/src/test/scala/org/apache/sedona/sql/dataFrameAPITestScala.scala @@ -21,6 +21,7 @@ package org.apache.sedona.sql import org.apache.commons.codec.binary.Hex import org.apache.spark.sql.Row import org.apache.spark.sql.functions.{array, col, element_at, lit} +import org.apache.spark.sql.sedona_sql.expressions.InferredExpressionException import org.apache.spark.sql.sedona_sql.expressions.st_aggregates._ import org.apache.spark.sql.sedona_sql.expressions.st_constructors._ import org.apache.spark.sql.sedona_sql.expressions.st_functions._ @@ -2215,5 +2216,19 @@ class dataFrameAPITestScala extends TestBaseScala { "SRID=4326;POLYGON ((-0.4546817643920842 0.5948176504236309, 1.4752502925921425 0.0700679430157733, 2 2, 0.0700679430157733 2.5247497074078575, 0.7726591178039579 1.2974088252118154, -0.4546817643920842 0.5948176504236309))" assert(expected.equals(actual)) } + + it("Passed returning exception with geometry when exception is thrown") { + val baseDf = sparkSession.sql( + "SELECT ST_GeomFromEWKT('SRID=4326;POLYGON ((0 0, 2 0, 2 2, 0 2, 1 1, 0 0))') AS geom1, ST_GeomFromEWKT('SRID=4326;POLYGON ((0 0, 1 0, 1 1, 0 1, 1 1, 0 0))') AS geom2") + + // Use intercept to assert that an exception is thrown + val exception = intercept[InferredExpressionException] { + baseDf.select(ST_Rotate("geom1", 50, "geom2")).take(1) + } + + // Check the exception message + assert(exception.getMessage.contains("[SRID=4326;POLYGON ((0 0, 2 0, 2 2, 0 2, 1 1, 0 0)), 50.0, SRID=4326;POLYGON ((0 0, 1 0, 1 1, 0 1, 1 1, 0 0))]")) + assert(exception.getMessage.contains("The origin must be a non-empty Point geometry.")) + } } } From 4acf6dce30acada5740c40733cafd855380f93dc Mon Sep 17 00:00:00 2001 From: zhangfengcdt Date: Fri, 19 Jul 2024 16:34:17 -0700 Subject: [PATCH 09/10] Refactor InferredExpression to include the input row in the exception message --- .../scala/org/apache/sedona/sql/constructorTestScala.scala | 6 ++++-- .../scala/org/apache/sedona/sql/dataFrameAPITestScala.scala | 3 ++- 2 files changed, 6 insertions(+), 3 deletions(-) diff --git a/spark/common/src/test/scala/org/apache/sedona/sql/constructorTestScala.scala b/spark/common/src/test/scala/org/apache/sedona/sql/constructorTestScala.scala index 8bf503927a..2c9564a3c6 100644 --- a/spark/common/src/test/scala/org/apache/sedona/sql/constructorTestScala.scala +++ b/spark/common/src/test/scala/org/apache/sedona/sql/constructorTestScala.scala @@ -216,7 +216,8 @@ class constructorTestScala extends TestBaseScala { val thrown = intercept[Exception] { sparkSession.sql("SELECT ST_GeomFromWKT('not wkt')").collect() } - assert(thrown.getMessage == "Unknown geometry type: NOT (line 1)") + assert( + thrown.getMessage == "Exception occurred while evaluating expression - source: [not wkt, 0], cause: Unknown geometry type: NOT (line 1)") } it("Passed ST_GeomFromEWKT") { @@ -245,7 +246,8 @@ class constructorTestScala extends TestBaseScala { val thrown = intercept[Exception] { sparkSession.sql("SELECT ST_GeomFromEWKT('not wkt')").collect() } - assert(thrown.getMessage == "Unknown geometry type: NOT (line 1)") + assert( + thrown.getMessage == "Exception occurred while evaluating expression - source: [not wkt], cause: Unknown geometry type: NOT (line 1)") } it("Passed ST_LineFromText") { diff --git a/spark/common/src/test/scala/org/apache/sedona/sql/dataFrameAPITestScala.scala b/spark/common/src/test/scala/org/apache/sedona/sql/dataFrameAPITestScala.scala index 4163dda0aa..b45fa7da55 100644 --- a/spark/common/src/test/scala/org/apache/sedona/sql/dataFrameAPITestScala.scala +++ b/spark/common/src/test/scala/org/apache/sedona/sql/dataFrameAPITestScala.scala @@ -2227,7 +2227,8 @@ class dataFrameAPITestScala extends TestBaseScala { } // Check the exception message - assert(exception.getMessage.contains("[SRID=4326;POLYGON ((0 0, 2 0, 2 2, 0 2, 1 1, 0 0)), 50.0, SRID=4326;POLYGON ((0 0, 1 0, 1 1, 0 1, 1 1, 0 0))]")) + assert(exception.getMessage.contains( + "[SRID=4326;POLYGON ((0 0, 2 0, 2 2, 0 2, 1 1, 0 0)), 50.0, SRID=4326;POLYGON ((0 0, 1 0, 1 1, 0 1, 1 1, 0 0))]")) assert(exception.getMessage.contains("The origin must be a non-empty Point geometry.")) } } From dcb640221c2be93cc32ae6e9a8b158fbce078163 Mon Sep 17 00:00:00 2001 From: zhangfengcdt Date: Fri, 19 Jul 2024 17:50:26 -0700 Subject: [PATCH 10/10] reformat --- .../sedona_sql/expressions/Constructors.scala | 167 +++++++++++------- .../sedona_sql/expressions/Functions.scala | 80 ++++++--- .../expressions/InferredExpression.scala | 47 ++--- .../sedona_sql/expressions/Predicates.scala | 11 +- .../expressions/collect/ST_Collect.scala | 41 +++-- 5 files changed, 208 insertions(+), 138 deletions(-) diff --git a/spark/common/src/main/scala/org/apache/spark/sql/sedona_sql/expressions/Constructors.scala b/spark/common/src/main/scala/org/apache/spark/sql/sedona_sql/expressions/Constructors.scala index b1a82ba105..6c3212e6cc 100644 --- a/spark/common/src/main/scala/org/apache/spark/sql/sedona_sql/expressions/Constructors.scala +++ b/spark/common/src/main/scala/org/apache/spark/sql/sedona_sql/expressions/Constructors.scala @@ -160,16 +160,21 @@ case class ST_GeomFromWKB(inputExpressions: Seq[Expression]) override def nullable: Boolean = true override def eval(inputRow: InternalRow): Any = { - (inputExpressions.head.eval(inputRow)) match { - case (geomString: UTF8String) => { - // Parse UTF-8 encoded wkb string - Constructors.geomFromText(geomString.toString, FileDataSplitter.WKB).toGenericArrayData - } - case (wkb: Array[Byte]) => { - // convert raw wkb byte array to geometry - Constructors.geomFromWKB(wkb).toGenericArrayData + try { + (inputExpressions.head.eval(inputRow)) match { + case (geomString: UTF8String) => { + // Parse UTF-8 encoded wkb string + Constructors.geomFromText(geomString.toString, FileDataSplitter.WKB).toGenericArrayData + } + case (wkb: Array[Byte]) => { + // convert raw wkb byte array to geometry + Constructors.geomFromWKB(wkb).toGenericArrayData + } + case null => null } - case null => null + } catch { + case e: Exception => + InferredExpression.throwExpressionInferenceException(inputRow, inputExpressions, e) } } @@ -196,16 +201,21 @@ case class ST_GeomFromEWKB(inputExpressions: Seq[Expression]) override def nullable: Boolean = true override def eval(inputRow: InternalRow): Any = { - (inputExpressions.head.eval(inputRow)) match { - case (geomString: UTF8String) => { - // Parse UTF-8 encoded wkb string - Constructors.geomFromText(geomString.toString, FileDataSplitter.WKB).toGenericArrayData - } - case (wkb: Array[Byte]) => { - // convert raw wkb byte array to geometry - Constructors.geomFromWKB(wkb).toGenericArrayData + try { + (inputExpressions.head.eval(inputRow)) match { + case (geomString: UTF8String) => { + // Parse UTF-8 encoded wkb string + Constructors.geomFromText(geomString.toString, FileDataSplitter.WKB).toGenericArrayData + } + case (wkb: Array[Byte]) => { + // convert raw wkb byte array to geometry + Constructors.geomFromWKB(wkb).toGenericArrayData + } + case null => null } - case null => null + } catch { + case e: Exception => + InferredExpression.throwExpressionInferenceException(inputRow, inputExpressions, e) } } @@ -238,21 +248,26 @@ case class ST_LineFromWKB(inputExpressions: Seq[Expression]) if (inputExpressions.length > 1) inputExpressions(1).eval(inputRow).asInstanceOf[Int] else -1 - wkb match { - case geomString: UTF8String => - // Parse UTF-8 encoded WKB string - val geom = Constructors.lineStringFromText(geomString.toString, "wkb") - if (geom.getGeometryType == "LineString") { - (if (srid != -1) Functions.setSRID(geom, srid) else geom).toGenericArrayData - } else { - null - } - - case wkbArray: Array[Byte] => - // Convert raw WKB byte array to geometry - Constructors.lineFromWKB(wkbArray, srid).toGenericArrayData - - case _ => null + try { + wkb match { + case geomString: UTF8String => + // Parse UTF-8 encoded WKB string + val geom = Constructors.lineStringFromText(geomString.toString, "wkb") + if (geom.getGeometryType == "LineString") { + (if (srid != -1) Functions.setSRID(geom, srid) else geom).toGenericArrayData + } else { + null + } + + case wkbArray: Array[Byte] => + // Convert raw WKB byte array to geometry + Constructors.lineFromWKB(wkbArray, srid).toGenericArrayData + + case _ => null + } + } catch { + case e: Exception => + InferredExpression.throwExpressionInferenceException(inputRow, inputExpressions, e) } } @@ -287,21 +302,26 @@ case class ST_LinestringFromWKB(inputExpressions: Seq[Expression]) if (inputExpressions.length > 1) inputExpressions(1).eval(inputRow).asInstanceOf[Int] else -1 - wkb match { - case geomString: UTF8String => - // Parse UTF-8 encoded WKB string - val geom = Constructors.lineStringFromText(geomString.toString, "wkb") - if (geom.getGeometryType == "LineString") { - (if (srid != -1) Functions.setSRID(geom, srid) else geom).toGenericArrayData - } else { - null - } - - case wkbArray: Array[Byte] => - // Convert raw WKB byte array to geometry - Constructors.lineFromWKB(wkbArray, srid).toGenericArrayData - - case _ => null + try { + wkb match { + case geomString: UTF8String => + // Parse UTF-8 encoded WKB string + val geom = Constructors.lineStringFromText(geomString.toString, "wkb") + if (geom.getGeometryType == "LineString") { + (if (srid != -1) Functions.setSRID(geom, srid) else geom).toGenericArrayData + } else { + null + } + + case wkbArray: Array[Byte] => + // Convert raw WKB byte array to geometry + Constructors.lineFromWKB(wkbArray, srid).toGenericArrayData + + case _ => null + } + } catch { + case e: Exception => + InferredExpression.throwExpressionInferenceException(inputRow, inputExpressions, e) } } @@ -336,21 +356,26 @@ case class ST_PointFromWKB(inputExpressions: Seq[Expression]) if (inputExpressions.length > 1) inputExpressions(1).eval(inputRow).asInstanceOf[Int] else -1 - wkb match { - case geomString: UTF8String => - // Parse UTF-8 encoded WKB string - val geom = Constructors.pointFromText(geomString.toString, "wkb") - if (geom.getGeometryType == "Point") { - (if (srid != -1) Functions.setSRID(geom, srid) else geom).toGenericArrayData - } else { - null - } - - case wkbArray: Array[Byte] => - // Convert raw WKB byte array to geometry - Constructors.pointFromWKB(wkbArray, srid).toGenericArrayData - - case _ => null + try { + wkb match { + case geomString: UTF8String => + // Parse UTF-8 encoded WKB string + val geom = Constructors.pointFromText(geomString.toString, "wkb") + if (geom.getGeometryType == "Point") { + (if (srid != -1) Functions.setSRID(geom, srid) else geom).toGenericArrayData + } else { + null + } + + case wkbArray: Array[Byte] => + // Convert raw WKB byte array to geometry + Constructors.pointFromWKB(wkbArray, srid).toGenericArrayData + + case _ => null + } + } catch { + case e: Exception => + InferredExpression.throwExpressionInferenceException(inputRow, inputExpressions, e) } } @@ -387,12 +412,18 @@ case class ST_GeomFromGeoJSON(inputExpressions: Seq[Expression]) override def eval(inputRow: InternalRow): Any = { val geomString = inputExpressions.head.eval(inputRow).asInstanceOf[UTF8String].toString - val geometry = Constructors.geomFromText(geomString, FileDataSplitter.GEOJSON) - // If the user specify a bunch of attributes to go with each geometry, we need to store all of them in this geometry - if (inputExpressions.length > 1) { - geometry.setUserData(generateUserData(minInputLength, inputExpressions, inputRow)) + try { + + val geometry = Constructors.geomFromText(geomString, FileDataSplitter.GEOJSON) + // If the user specify a bunch of attributes to go with each geometry, we need to store all of them in this geometry + if (inputExpressions.length > 1) { + geometry.setUserData(generateUserData(minInputLength, inputExpressions, inputRow)) + } + GeometrySerializer.serialize(geometry) + } catch { + case e: Exception => + InferredExpression.throwExpressionInferenceException(inputRow, inputExpressions, e) } - GeometrySerializer.serialize(geometry) } override def dataType: DataType = GeometryUDT diff --git a/spark/common/src/main/scala/org/apache/spark/sql/sedona_sql/expressions/Functions.scala b/spark/common/src/main/scala/org/apache/spark/sql/sedona_sql/expressions/Functions.scala index 70e3582f20..cf52d5bc75 100644 --- a/spark/common/src/main/scala/org/apache/spark/sql/sedona_sql/expressions/Functions.scala +++ b/spark/common/src/main/scala/org/apache/spark/sql/sedona_sql/expressions/Functions.scala @@ -332,13 +332,18 @@ case class ST_IsValidDetail(children: Seq[Expression]) throw new IllegalArgumentException(s"Invalid number of arguments: $nArgs") } - if (validDetail.location == null) { - return InternalRow.fromSeq(Seq(validDetail.valid, null, null)) - } + try { + if (validDetail.location == null) { + return InternalRow.fromSeq(Seq(validDetail.valid, null, null)) + } - val serLocation = GeometrySerializer.serialize(validDetail.location) - InternalRow.fromSeq( - Seq(validDetail.valid, UTF8String.fromString(validDetail.reason), serLocation)) + val serLocation = GeometrySerializer.serialize(validDetail.location) + InternalRow.fromSeq( + Seq(validDetail.valid, UTF8String.fromString(validDetail.reason), serLocation)) + } catch { + case e: Exception => + InferredExpression.throwExpressionInferenceException(input, children, e) + } } protected def withNewChildrenInternal(newChildren: IndexedSeq[Expression]): Expression = { @@ -609,14 +614,20 @@ case class ST_MinimumBoundingRadius(inputExpressions: Seq[Expression]) override def eval(input: InternalRow): Any = { val expr = inputExpressions(0) - val geometry = expr match { - case s: SerdeAware => s.evalWithoutSerialization(input) - case _ => expr.toGeometry(input) - } - geometry match { - case geometry: Geometry => getMinimumBoundingRadius(geometry) - case _ => null + try { + val geometry = expr match { + case s: SerdeAware => s.evalWithoutSerialization(input) + case _ => expr.toGeometry(input) + } + + geometry match { + case geometry: Geometry => getMinimumBoundingRadius(geometry) + case _ => null + } + } catch { + case e: Exception => + InferredExpression.throwExpressionInferenceException(input, inputExpressions, e) } } @@ -910,17 +921,23 @@ case class ST_SubDivideExplode(children: Seq[Expression]) extends Generator with override def eval(input: InternalRow): TraversableOnce[InternalRow] = { val geometryRaw = children.head val maxVerticesRaw = children(1) - geometryRaw.toGeometry(input) match { - case geom: Geometry => - ArrayData.toArrayData( - Functions.subDivide(geom, maxVerticesRaw.toInt(input)).map(_.toGenericArrayData)) - Functions - .subDivide(geom, maxVerticesRaw.toInt(input)) - .map(_.toGenericArrayData) - .map(InternalRow(_)) - case _ => new Array[InternalRow](0) + try { + geometryRaw.toGeometry(input) match { + case geom: Geometry => + ArrayData.toArrayData( + Functions.subDivide(geom, maxVerticesRaw.toInt(input)).map(_.toGenericArrayData)) + Functions + .subDivide(geom, maxVerticesRaw.toInt(input)) + .map(_.toGenericArrayData) + .map(InternalRow(_)) + case _ => new Array[InternalRow](0) + } + } catch { + case e: Exception => + InferredExpression.throwExpressionInferenceException(input, children, e) } } + override def elementSchema: StructType = { new StructType() .add("geom", GeometryUDT, true) @@ -978,13 +995,18 @@ case class ST_MaximumInscribedCircle(children: Seq[Expression]) with CodegenFallback { override def eval(input: InternalRow): Any = { - val geometry = children.head.toGeometry(input) - var inscribedCircle: InscribedCircle = null - inscribedCircle = Functions.maximumInscribedCircle(geometry) - - val serCenter = GeometrySerializer.serialize(inscribedCircle.center) - val serNearest = GeometrySerializer.serialize(inscribedCircle.nearest) - InternalRow.fromSeq(Seq(serCenter, serNearest, inscribedCircle.radius)) + try { + val geometry = children.head.toGeometry(input) + var inscribedCircle: InscribedCircle = null + inscribedCircle = Functions.maximumInscribedCircle(geometry) + + val serCenter = GeometrySerializer.serialize(inscribedCircle.center) + val serNearest = GeometrySerializer.serialize(inscribedCircle.nearest) + InternalRow.fromSeq(Seq(serCenter, serNearest, inscribedCircle.radius)) + } catch { + case e: Exception => + InferredExpression.throwExpressionInferenceException(input, children, e) + } } protected def withNewChildrenInternal(newChildren: IndexedSeq[Expression]): Expression = { diff --git a/spark/common/src/main/scala/org/apache/spark/sql/sedona_sql/expressions/InferredExpression.scala b/spark/common/src/main/scala/org/apache/spark/sql/sedona_sql/expressions/InferredExpression.scala index 5742658c29..44d16b08a1 100644 --- a/spark/common/src/main/scala/org/apache/spark/sql/sedona_sql/expressions/InferredExpression.scala +++ b/spark/common/src/main/scala/org/apache/spark/sql/sedona_sql/expressions/InferredExpression.scala @@ -97,16 +97,7 @@ abstract class InferredExpression(fSeq: InferrableFunction*) f.serializer(evaluator(input)) } catch { case e: Exception => - val literalsAsStrings = if (input == null) { - // In case no input row is provided, we can't extract literals from the input expressions. - findAllLiteralsInExpressions(inputExpressions) - } else { - Seq.empty[String] - } - val literalsOrInputString = literalsAsStrings.mkString(", ") - throw new InferredExpressionException( - s"Exception occurred while evaluating expression - source: [$literalsOrInputString]", - e) + InferredExpression.throwExpressionInferenceException(input, inputExpressions, e) } } @@ -115,16 +106,32 @@ abstract class InferredExpression(fSeq: InferrableFunction*) evaluator(input) } catch { case e: Exception => - val literalsOrInputStrings = if (input == null) { - // In case no input row is provided, we can't extract literals from the input expressions. - findAllLiteralsInExpressions(inputExpressions) - } else { - Seq.empty[String] - } - val literalsOrInputString = literalsOrInputStrings.mkString(", ") - throw new InferredExpressionException( - s"Exception occurred while evaluating input row without serialization. Literals found: [$literalsOrInputString]", - e) + InferredExpression.throwExpressionInferenceException(input, inputExpressions, e) + } + } +} + +object InferredExpression { + def throwExpressionInferenceException( + input: InternalRow, + inputExpressions: Seq[Expression], + e: Exception): Nothing = { + val literalsAsStrings = if (input == null) { + // In case no input row is provided, we can't extract literals from the input expressions. + inputExpressions.flatMap(findAllLiterals).map(_.value.toString) + } else { + Seq.empty[String] + } + val literalsOrInputString = literalsAsStrings.mkString(", ") + throw new InferredExpressionException( + s"Exception occurred while evaluating expression - source: [$literalsOrInputString]", + e) + } + + def findAllLiterals(expression: Expression): Seq[Literal] = { + expression match { + case lit: Literal => Seq(lit) + case _ => expression.children.flatMap(findAllLiterals) } } } diff --git a/spark/common/src/main/scala/org/apache/spark/sql/sedona_sql/expressions/Predicates.scala b/spark/common/src/main/scala/org/apache/spark/sql/sedona_sql/expressions/Predicates.scala index d2e7c2ccba..abdc066b0b 100644 --- a/spark/common/src/main/scala/org/apache/spark/sql/sedona_sql/expressions/Predicates.scala +++ b/spark/common/src/main/scala/org/apache/spark/sql/sedona_sql/expressions/Predicates.scala @@ -55,9 +55,14 @@ abstract class ST_Predicate if (rightArray == null) { null } else { - val leftGeometry = GeometrySerializer.deserialize(leftArray) - val rightGeometry = GeometrySerializer.deserialize(rightArray) - evalGeom(leftGeometry, rightGeometry) + try { + val leftGeometry = GeometrySerializer.deserialize(leftArray) + val rightGeometry = GeometrySerializer.deserialize(rightArray) + evalGeom(leftGeometry, rightGeometry) + } catch { + case e: Exception => + InferredExpression.throwExpressionInferenceException(inputRow, inputExpressions, e) + } } } } diff --git a/spark/common/src/main/scala/org/apache/spark/sql/sedona_sql/expressions/collect/ST_Collect.scala b/spark/common/src/main/scala/org/apache/spark/sql/sedona_sql/expressions/collect/ST_Collect.scala index bae1259901..95846526b4 100644 --- a/spark/common/src/main/scala/org/apache/spark/sql/sedona_sql/expressions/collect/ST_Collect.scala +++ b/spark/common/src/main/scala/org/apache/spark/sql/sedona_sql/expressions/collect/ST_Collect.scala @@ -25,7 +25,7 @@ import org.apache.spark.sql.catalyst.expressions.Expression import org.apache.spark.sql.catalyst.util.ArrayData import org.apache.spark.sql.sedona_sql.UDT.GeometryUDT import org.apache.spark.sql.sedona_sql.expressions.implicits._ -import org.apache.spark.sql.sedona_sql.expressions.SerdeAware +import org.apache.spark.sql.sedona_sql.expressions.{InferredExpression, SerdeAware} import org.apache.spark.sql.types.{ArrayType, _} import org.locationtech.jts.geom.Geometry @@ -44,24 +44,29 @@ case class ST_Collect(inputExpressions: Seq[Expression]) override def evalWithoutSerialization(input: InternalRow): Any = { val firstElement = inputExpressions.head - firstElement.dataType match { - case ArrayType(elementType, _) => - elementType match { - case _: GeometryUDT => - val data = firstElement.eval(input).asInstanceOf[ArrayData] - val numElements = data.numElements() - val geomElements = (0 until numElements) - .map(element => data.getBinary(element)) - .filter(_ != null) - .map(_.toGeometry) + try { + firstElement.dataType match { + case ArrayType(elementType, _) => + elementType match { + case _: GeometryUDT => + val data = firstElement.eval(input).asInstanceOf[ArrayData] + val numElements = data.numElements() + val geomElements = (0 until numElements) + .map(element => data.getBinary(element)) + .filter(_ != null) + .map(_.toGeometry) - Functions.createMultiGeometry(geomElements.toArray) - case _ => Functions.createMultiGeometry(Array()) - } - case _ => - val geomElements = - inputExpressions.map(_.toGeometry(input)).filter(_ != null) - Functions.createMultiGeometry(geomElements.toArray) + Functions.createMultiGeometry(geomElements.toArray) + case _ => Functions.createMultiGeometry(Array()) + } + case _ => + val geomElements = + inputExpressions.map(_.toGeometry(input)).filter(_ != null) + Functions.createMultiGeometry(geomElements.toArray) + } + } catch { + case e: Exception => + InferredExpression.throwExpressionInferenceException(input, inputExpressions, e) } }