This is an automated email from the ASF dual-hosted git repository. amanin pushed a commit to branch refactor/sql-store in repository https://gitbox.apache.org/repos/asf/sis.git
commit c832d0e2b1a7f4daf6c08742bfd5259aa009d09a Author: Alexis Manin <[email protected]> AuthorDate: Mon Nov 4 18:01:09 2019 +0100 fix(SQLStore): fix default bbox filter conversion to sql. --- .../java/org/apache/sis/filter/DefaultBBOX.java | 2 +- .../java/org/apache/sis/internal/feature/ESRI.java | 8 ++++- .../java/org/apache/sis/internal/feature/JTS.java | 33 ++++++++++++++++++- .../org/apache/sis/internal/feature/Java2D.java | 12 +++++-- .../org/apache/sis/internal/feature/jts/JTS.java | 38 +++++++++++++++++++--- .../sis/internal/sql/feature/ANSIInterpreter.java | 32 ++++++------------ .../sql/feature/FilterInterpreterTest.java | 9 ++--- 7 files changed, 96 insertions(+), 38 deletions(-) diff --git a/core/sis-feature/src/main/java/org/apache/sis/filter/DefaultBBOX.java b/core/sis-feature/src/main/java/org/apache/sis/filter/DefaultBBOX.java index a597e45..6444070 100644 --- a/core/sis-feature/src/main/java/org/apache/sis/filter/DefaultBBOX.java +++ b/core/sis-feature/src/main/java/org/apache/sis/filter/DefaultBBOX.java @@ -79,7 +79,7 @@ final class DefaultBBOX implements BBOX { private boolean nonOptimizedIntersect(Object candidate) { Envelope leftEval = left == null ? null : asEnvelope(left, candidate); - Envelope rightEval = left == null ? null : asEnvelope(right, candidate); + Envelope rightEval = right == null ? null : asEnvelope(right, candidate); if (left == null) { return multiIntersect(candidate, rightEval); } else if (right == null) { diff --git a/core/sis-feature/src/main/java/org/apache/sis/internal/feature/ESRI.java b/core/sis-feature/src/main/java/org/apache/sis/internal/feature/ESRI.java index 80a71dab..9659549 100644 --- a/core/sis-feature/src/main/java/org/apache/sis/internal/feature/ESRI.java +++ b/core/sis-feature/src/main/java/org/apache/sis/internal/feature/ESRI.java @@ -146,6 +146,12 @@ final class ESRI extends Geometries<Geometry> { return path; } + @Override + public Geometry toPolygon(Geometry polyline) throws IllegalArgumentException { + if (polyline instanceof Polygon) return polyline; + return createMultiPolygonImpl(polyline); + } + /** * Merges a sequence of points or paths if the first instance is an implementation of this library. * @@ -206,7 +212,7 @@ add: for (;;) { } @Override - Object createMultiPolygonImpl(Object... polygonsOrLinearRings) { + Polygon createMultiPolygonImpl(Object... polygonsOrLinearRings) { final Polygon poly = new Polygon(); for (final Object polr : polygonsOrLinearRings) { if (polr instanceof MultiPath) { diff --git a/core/sis-feature/src/main/java/org/apache/sis/internal/feature/JTS.java b/core/sis-feature/src/main/java/org/apache/sis/internal/feature/JTS.java index 2e9b184..34c5ba0 100644 --- a/core/sis-feature/src/main/java/org/apache/sis/internal/feature/JTS.java +++ b/core/sis-feature/src/main/java/org/apache/sis/internal/feature/JTS.java @@ -21,10 +21,14 @@ import java.util.Arrays; import java.util.Iterator; import java.util.List; +import org.opengis.referencing.crs.CoordinateReferenceSystem; +import org.opengis.util.FactoryException; + import org.apache.sis.geometry.GeneralEnvelope; import org.apache.sis.math.Vector; import org.apache.sis.setup.GeometryLibrary; import org.apache.sis.util.Classes; +import org.apache.sis.util.collection.BackingStoreException; import org.locationtech.jts.geom.Coordinate; import org.locationtech.jts.geom.Envelope; @@ -33,6 +37,7 @@ import org.locationtech.jts.geom.GeometryFactory; import org.locationtech.jts.geom.LineString; import org.locationtech.jts.geom.LinearRing; import org.locationtech.jts.geom.MultiLineString; +import org.locationtech.jts.geom.MultiPolygon; import org.locationtech.jts.geom.Point; import org.locationtech.jts.geom.Polygon; import org.locationtech.jts.io.ParseException; @@ -191,6 +196,32 @@ final class JTS extends Geometries<Geometry> { return toGeometry(lines); } + @Override + public Geometry toPolygon(Geometry polyline) throws IllegalArgumentException { + if (polyline instanceof Polygon) return polyline; + + Polygon result = null; + if (polyline instanceof LinearRing) { + result = factory.createPolygon((LinearRing) polyline); + } else if (polyline instanceof LineString) { + final LineString myLine = (LineString) polyline; + if (myLine.getEndPoint().equals(myLine.getStartPoint())) { + result = factory.createPolygon(myLine.getCoordinateSequence()); + } + } + + if (result == null) throw new IllegalArgumentException("Input is not a closed line."); + + try { + final CoordinateReferenceSystem crs = org.apache.sis.internal.feature.jts.JTS.getCoordinateReferenceSystem(polyline); + org.apache.sis.internal.feature.jts.JTS.setCoordinateReferenceSystem(result, crs); + } catch (FactoryException e) { + throw new BackingStoreException("Cannot extract CRS from geometry", e); + } + + return result; + } + /** * Makes a line string or linear ring from the given coordinates, and add the line string to the given list. * If the given coordinates array is empty, then this method does nothing. @@ -291,7 +322,7 @@ add: for (;;) { } @Override - Object createMultiPolygonImpl(Object... polygonsOrLinearRings) { + MultiPolygon createMultiPolygonImpl(Object... polygonsOrLinearRings) { final Polygon[] polys = new Polygon[polygonsOrLinearRings.length]; for (int i = 0 ; i < polys.length ; i++) { Object o = polygonsOrLinearRings[i]; diff --git a/core/sis-feature/src/main/java/org/apache/sis/internal/feature/Java2D.java b/core/sis-feature/src/main/java/org/apache/sis/internal/feature/Java2D.java index 991a723..35f9a04 100644 --- a/core/sis-feature/src/main/java/org/apache/sis/internal/feature/Java2D.java +++ b/core/sis-feature/src/main/java/org/apache/sis/internal/feature/Java2D.java @@ -178,7 +178,6 @@ final class Java2D extends Geometries<Shape> { } if (Double.isNaN(x) || Double.isNaN(y)) { if (lastX == startX && lastY == startY) path.closePath(); - startX = Double.NaN; lineTo = false; startX = startY = Double.NaN; } else if (lineTo) { @@ -258,13 +257,20 @@ add: for (;;) { } @Override - Object createMultiPolygonImpl(Object... polygonsOrLinearRings) { + Shape createMultiPolygonImpl(Object... polygonsOrLinearRings) { ensureNonEmpty("Polygons or linear rings to merge", polygonsOrLinearRings); - if (polygonsOrLinearRings.length == 1) return polygonsOrLinearRings[0]; + if (polygonsOrLinearRings.length == 1 && polygonsOrLinearRings[0] instanceof Shape) + return (Shape) polygonsOrLinearRings[0]; final Iterator<Object> it = Arrays.asList(polygonsOrLinearRings).iterator(); return tryMergePolylines(it.next(), it); } + @Override + public Shape toPolygon(Shape polyline) throws IllegalArgumentException { + // TODO: check that path ends with close. + return polyline; + } + /** * If the given object is a Java2D shape, builds its WKT representation. * Current implementation assumes that all closed shapes are polygons and that polygons have no hole diff --git a/core/sis-feature/src/main/java/org/apache/sis/internal/feature/jts/JTS.java b/core/sis-feature/src/main/java/org/apache/sis/internal/feature/jts/JTS.java index af9d29a..d3e1174 100644 --- a/core/sis-feature/src/main/java/org/apache/sis/internal/feature/jts/JTS.java +++ b/core/sis-feature/src/main/java/org/apache/sis/internal/feature/jts/JTS.java @@ -17,23 +17,29 @@ package org.apache.sis.internal.feature.jts; import java.util.Map; -import org.opengis.util.FactoryException; +import java.util.Optional; + import org.opengis.referencing.crs.CoordinateReferenceSystem; import org.opengis.referencing.operation.CoordinateOperation; import org.opengis.referencing.operation.MathTransform; import org.opengis.referencing.operation.TransformException; +import org.opengis.util.FactoryException; + +import org.apache.sis.geometry.Envelope2D; +import org.apache.sis.internal.system.Loggers; import org.apache.sis.internal.util.Constants; +import org.apache.sis.metadata.iso.extent.DefaultGeographicBoundingBox; import org.apache.sis.referencing.CRS; import org.apache.sis.util.Static; import org.apache.sis.util.Utilities; import org.apache.sis.util.logging.Logging; import org.apache.sis.util.resources.Errors; -import org.apache.sis.metadata.iso.extent.DefaultGeographicBoundingBox; -import org.apache.sis.geometry.Envelope2D; -import org.apache.sis.internal.system.Loggers; + import org.locationtech.jts.geom.Envelope; import org.locationtech.jts.geom.Geometry; +import static org.apache.sis.util.ArgumentChecks.ensureNonNull; + /** * Utilities for Java Topology Suite (JTS) objects. @@ -101,6 +107,30 @@ public final class JTS extends Static { return null; } + public static Optional<CoordinateReferenceSystem> setCoordinateReferenceSystem(final Geometry target, final CoordinateReferenceSystem toSet) { + ensureNonNull("Target geometry", target); + final Object ud = target.getUserData(); + if (ud == null) { + // By security, we reset SRID in case old CRS was defined this way. + target.setSRID(0); + target.setUserData(toSet); + return Optional.empty(); + } else if (ud instanceof CoordinateReferenceSystem) { + target.setUserData(toSet); + return Optional.of((CoordinateReferenceSystem)ud); + } else if (ud instanceof Map) { + final Map asMap = (Map) ud; + // In case user-data contains other useful data, we don't switch from map to CRS. We also reset SRID. + final Object oldVal = asMap.put(CRS_KEY, toSet); + // By security, we reset SRID in case old CRS was defined this way. + if (oldVal == null) { + target.setSRID(0); + } + } + + throw new IllegalArgumentException("Cannot modify input geometry, because user-data does not comply with SIS convention (should be a map or null, but was "+ud.getClass().getCanonicalName()+")."); + } + /** * Transforms the given geometry to the specified Coordinate Reference System (CRS). * If the given CRS or the given geometry is null, the geometry is returned unchanged. diff --git a/storage/sis-sqlstore/src/main/java/org/apache/sis/internal/sql/feature/ANSIInterpreter.java b/storage/sis-sqlstore/src/main/java/org/apache/sis/internal/sql/feature/ANSIInterpreter.java index 676fa1a..943df9e 100644 --- a/storage/sis-sqlstore/src/main/java/org/apache/sis/internal/sql/feature/ANSIInterpreter.java +++ b/storage/sis-sqlstore/src/main/java/org/apache/sis/internal/sql/feature/ANSIInterpreter.java @@ -182,26 +182,9 @@ public class ANSIInterpreter implements FilterVisitor, ExpressionVisitor { @Override public Object visit(BBOX filter, Object extraData) { - final CharSequence left = evaluateMandatory(filter.getExpression1(), extraData); - final CharSequence right = evaluateMandatory(filter.getExpression2(), extraData); - - // TODO: In case source expression is already an envelope, we do not need to force envelope conversion. It would - // only be micro-optimisation however. - boolean leftToEnvelope = true; - boolean rightToEnvelope = true; - - final StringBuilder sb = new StringBuilder("ST_Intersects("); - if (leftToEnvelope) { - sb.append("ST_Envelope(").append(left).append(')'); - } else sb.append(left); - - sb.append(", "); - - if (rightToEnvelope) { - sb.append("ST_Envelope(").append(right).append(')'); - } else sb.append(right); - - return sb.append(')'); + // TODO: This is a wrong interpretation, but sqlmm has no equivalent of filter encoding bbox, so we'll + // fallback on a standard intersection. However, PostGIS, H2, etc. have their own versions of such filters. + return function("ST_Intersects(", filter, extraData); } @Override @@ -442,6 +425,11 @@ public class ANSIInterpreter implements FilterVisitor, ExpressionVisitor { return join(candidate::getExpression1, candidate::getExpression2, operator, extraData); } + + protected CharSequence join(BinarySpatialOperator candidate, String operator, Object extraData) { + return join(candidate::getExpression1, candidate::getExpression2, operator, extraData); + } + protected CharSequence join( Supplier<Expression> leftOperand, Supplier<Expression> rightOperand, @@ -526,9 +514,9 @@ public class ANSIInterpreter implements FilterVisitor, ExpressionVisitor { .mapToDouble(env::getSpan) .average() .orElseThrow(() -> new IllegalArgumentException("Given geometry envelope dimension is 0")); - return new StringBuilder("ST_GeomFromText(") + return new StringBuilder("ST_GeomFromText('") .append(Geometries.formatWKT(source, flatness)) - .append(')'); + .append("')"); } protected static double clampInfinity(final double candidate) { diff --git a/storage/sis-sqlstore/src/test/java/org/apache/sis/internal/sql/feature/FilterInterpreterTest.java b/storage/sis-sqlstore/src/test/java/org/apache/sis/internal/sql/feature/FilterInterpreterTest.java index 7c73016..b9f59d7 100644 --- a/storage/sis-sqlstore/src/test/java/org/apache/sis/internal/sql/feature/FilterInterpreterTest.java +++ b/storage/sis-sqlstore/src/test/java/org/apache/sis/internal/sql/feature/FilterInterpreterTest.java @@ -18,15 +18,12 @@ public class FilterInterpreterTest extends TestCase { @Test public void testGeometricFilter() { - final ANSIInterpreter interpreter = new ANSIInterpreter(); final BBOX filter = FF.bbox(FF.property("Toto"), new GeneralEnvelope(new DefaultGeographicBoundingBox(-12.3, 2.1, 43.3, 51.7))); assertConversion(filter, "ST_Intersects(" + - "ST_Envelope(\"Toto\"), " + - "ST_Envelope(" + - "ST_GeomFromText(" + - "POLYGON ((-12.3 43.3, -12.3 51.7, 2.1 51.7, 2.1 43.3, -12.3 43.3))" + - ")" + + "\"Toto\", " + + "ST_GeomFromText(" + + "'POLYGON ((-12.3 43.3, -12.3 51.7, 2.1 51.7, 2.1 43.3, -12.3 43.3))'" + ")" + ")" );
