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 167d5f354693ebba58eda9ab8075f19334172480 Author: Alexis Manin <[email protected]> AuthorDate: Wed Oct 9 13:06:38 2019 +0200 WIP(Feature): BBOX filter --- .../java/org/apache/sis/filter/DefaultBBOX.java | 121 ++++++++++++++++++--- .../apache/sis/filter/DefaultFilterFactory.java | 39 ++++++- .../test/java/org/apache/sis/filter/SQLMMTest.java | 33 +++++- .../sql/feature/FilterInterpreterTest.java | 3 +- .../org/apache/sis/test/suite/SQLTestSuite.java | 3 +- 5 files changed, 176 insertions(+), 23 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 8f233ba..a465388 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 @@ -1,15 +1,23 @@ package org.apache.sis.filter; +import java.util.function.Predicate; + +import org.opengis.feature.AttributeType; +import org.opengis.feature.Feature; +import org.opengis.feature.FeatureType; import org.opengis.filter.FilterVisitor; import org.opengis.filter.expression.Expression; +import org.opengis.filter.expression.Literal; import org.opengis.filter.spatial.BBOX; import org.opengis.geometry.Envelope; import org.opengis.metadata.extent.GeographicBoundingBox; +import org.apache.sis.feature.Features; +import org.apache.sis.geometry.AbstractEnvelope; import org.apache.sis.geometry.GeneralEnvelope; +import org.apache.sis.geometry.ImmutableEnvelope; import org.apache.sis.internal.feature.Geometries; - -import static org.apache.sis.util.ArgumentChecks.ensureNonNull; +import org.apache.sis.util.NullArgumentException; /** * @implNote AMBIGUITY : Description of BBOX operator from <a href="http://docs.opengeospatial.org/is/09-026r2/09-026r2.html#60";> @@ -18,7 +26,10 @@ import static org.apache.sis.util.ArgumentChecks.ensureNonNull; * testing bbox only, because the test for a bbox against a complex geometry can be realized using ST_Intersect * operator. * - * TODO: optimisations in case one of the two operators is a literal. + * Border management: From above reference, bbox should be equivalent to Not ST_Disjoint (from SQLMM/ISO:19125). + * Disjoint operation specifies that both geometry boundaries must not touch (their intersection is an empty space), so + * we will consider as valid envelopes with only a common boundary. + * * TODO: CRS check. */ final class DefaultBBOX implements BBOX { @@ -26,11 +37,24 @@ final class DefaultBBOX implements BBOX { final Expression left; final Expression right; - public DefaultBBOX(Expression left, Expression right) { - ensureNonNull("Left expression", left); - ensureNonNull("Right expression", right); + private final Predicate intersects; + + DefaultBBOX(Expression left, Expression right) { + if (left == null && right == null) { + throw new NullArgumentException( + "Both arguments are null, but at least one must be given " + + "(as stated in OGC Filter encoding corrigendum 2.0.2, section 7.8.3.2)." + ); + } + this.left = left; this.right = right; + + if (left instanceof Literal) { + intersects = asOptimizedTest((Literal)left, right); + } else if (right instanceof Literal) { + intersects = asOptimizedTest((Literal)right, left); + } else intersects = this::nonOptimizedIntersect; } @Override @@ -45,13 +69,80 @@ final class DefaultBBOX implements BBOX { @Override public boolean evaluate(Object object) { - Envelope leftEval = asEnvelope(left, object); - Envelope rightEval = asEnvelope(right, object); - // If both are null, return a match. - if (leftEval == rightEval) return true; + return intersects.test(object); + } + + @Override + public Object accept(FilterVisitor visitor, Object extraData) { + return visitor.visit(this, extraData); + } + + private boolean nonOptimizedIntersect(Object candidate) { + Envelope leftEval = left == null? null : asEnvelope(left, candidate); + Envelope rightEval = left == null? null : asEnvelope(right, candidate); + if (left == null) { + return multiIntersect(candidate, rightEval); + } else if (right == null) { + return multiIntersect(candidate, leftEval); + } + + /* OGC Filter encoding corrigendum 2.0.2 section 7.8.3.4 states that false must be returned if any of the + * operand is null. It does not state what to do if both are null, but we'll follow the same behavior. + */ + if (leftEval == null || rightEval == null) return false; return GeneralEnvelope.castOrCopy(leftEval).intersects(rightEval); } + private static boolean intersect(final Object candidate, final Expression valueExtractor, final AbstractEnvelope constEnvelope) { + final Envelope candidateEnv = asEnvelope(valueExtractor, candidate); + if (candidateEnv == null) return false; + return constEnvelope.intersects(candidateEnv, true); + } + + /** + * Ensure that all geometric properties in given candidate intersect input envelope. This method tries to match OGC + * Filter Encoding corrigendum 2.0.2 section 7.8.3.2 that says if one of the expressions given at built is null, we + * have to ensure all geometric properties of the candidate intersect the other expression. + * + * @param candidate The object to extract all geometric properties from. + * @param fixed + * @return + */ + private static boolean multiIntersect(Object candidate, Envelope fixed) { + // TODO: We could optimize by caching feature-type properties. The best way would be an initialisation + // procedure freezing target data type, but I'm no sure such a mechanism would be possible. + final GeneralEnvelope constEnv = GeneralEnvelope.castOrCopy(fixed); + if (candidate instanceof Feature) { + final Feature f = (Feature) candidate; + final FeatureType type = f.getType(); + /* Note: for now, we could have doublons, but have no simple mean to eliminate link operations. Relying on + * convention naming is too risky, as some drivers could use it directly on their attributes, or create a + * computational operation (create point from numeric columns, reproject geometry, etc.). In such case, we + * would drop valuable information. + */ + return type.getProperties(true) + .stream() + .filter(p + -> Features.castOrUnwrap(p) + .map(AttributeType::getValueClass) + .filter(Geometries::isKnownType) + .isPresent() + ) + .map(p -> p.getName().toString()) + .map(f::getPropertyValue) + .map(Geometries::getEnvelope) + .allMatch(fEnv -> fEnv != null && constEnv.intersects(fEnv, true)); + } else if (candidate instanceof Envelope) { + return constEnv.intersects((Envelope) candidate); + } else { + final Envelope env = Geometries.getEnvelope(candidate); + if (env == null) throw new UnsupportedOperationException( + "Candidate type unsupported: "+candidate == null? "null" : candidate.getClass().getCanonicalName() + ); + return constEnv.intersects(env); + } + } + private static Envelope asEnvelope(final Expression evaluator, final Object data) { Envelope eval = evaluator.evaluate(data, Envelope.class); if (eval == null) { @@ -68,11 +159,15 @@ final class DefaultBBOX implements BBOX { return eval; } - @Override - public Object accept(FilterVisitor visitor, Object extraData) { - return visitor.visit(this, extraData); + private static Predicate asOptimizedTest(Literal constant, Expression other) { + final ImmutableEnvelope constEnv = new ImmutableEnvelope(asEnvelope(constant, null)); + return other == null? it -> multiIntersect(it, constEnv) : it -> intersect(it, other, constEnv); } + /* + * DEPRECATED OPERATIONS: NOT IMPLEMENTED + */ + @Override public String getPropertyName() { throw new UnsupportedOperationException("Not supported yet"); // "Alexis Manin (Geomatys)" on 08/10/2019 diff --git a/core/sis-feature/src/main/java/org/apache/sis/filter/DefaultFilterFactory.java b/core/sis-feature/src/main/java/org/apache/sis/filter/DefaultFilterFactory.java index b1b1197..b8c90a3 100644 --- a/core/sis-feature/src/main/java/org/apache/sis/filter/DefaultFilterFactory.java +++ b/core/sis-feature/src/main/java/org/apache/sis/filter/DefaultFilterFactory.java @@ -53,9 +53,16 @@ import org.opengis.filter.spatial.Within; import org.opengis.filter.temporal.*; import org.opengis.geometry.Envelope; import org.opengis.geometry.Geometry; +import org.opengis.referencing.NoSuchAuthorityCodeException; +import org.opengis.referencing.crs.CoordinateReferenceSystem; +import org.opengis.util.FactoryException; import org.opengis.util.GenericName; +import org.apache.sis.geometry.GeneralEnvelope; +import org.apache.sis.geometry.ImmutableEnvelope; import org.apache.sis.internal.feature.FunctionRegister; +import org.apache.sis.referencing.CRS; +import org.apache.sis.util.collection.BackingStoreException; /** @@ -130,7 +137,35 @@ public class DefaultFilterFactory implements FilterFactory2 { public BBOX bbox(final Expression e, final double minx, final double miny, final double maxx, final double maxy, final String srs) { - throw new UnsupportedOperationException("Not supported yet."); + final CoordinateReferenceSystem crs = readCrs(srs); + final GeneralEnvelope env = new GeneralEnvelope(2); + env.setEnvelope(minx, miny, maxx, maxy); + if (crs != null) env.setCoordinateReferenceSystem(crs); + return bbox(e, new ImmutableEnvelope(env)); + } + + /** + * Try to decode a full {@link CoordinateReferenceSystem} from given text. First, we try to interpret it as a code, + * and if it fails, we try to read it as a WKT. + * + * @param srs The text describing the system. If null or blank, a null value is returned. + * @return Possible null value if input text is empty. + * @throws BackingStoreException If an error occurs while decoding the text. + */ + private static CoordinateReferenceSystem readCrs(String srs) { + if (srs == null || (srs = srs.trim()).isEmpty()) return null; + try { + return CRS.forCode(srs); + } catch (NoSuchAuthorityCodeException e) { + try { + return CRS.fromWKT(srs); + } catch (FactoryException bis) { + e.addSuppressed(bis); + } + throw new BackingStoreException(e); + } catch (FactoryException e) { + throw new BackingStoreException(e); + } } /** @@ -138,7 +173,7 @@ public class DefaultFilterFactory implements FilterFactory2 { */ @Override public BBOX bbox(final Expression e, final Envelope bounds) { - throw new UnsupportedOperationException("Not supported yet."); + return new DefaultBBOX(e, literal(bounds)); } /** diff --git a/core/sis-feature/src/test/java/org/apache/sis/filter/SQLMMTest.java b/core/sis-feature/src/test/java/org/apache/sis/filter/SQLMMTest.java index d6ec945..9929d04 100644 --- a/core/sis-feature/src/test/java/org/apache/sis/filter/SQLMMTest.java +++ b/core/sis-feature/src/test/java/org/apache/sis/filter/SQLMMTest.java @@ -16,19 +16,24 @@ */ package org.apache.sis.filter; +import org.opengis.feature.Feature; +import org.opengis.feature.FeatureType; +import org.opengis.filter.FilterFactory2; +import org.opengis.filter.expression.Expression; +import org.opengis.filter.expression.Function; +import org.opengis.referencing.crs.CoordinateReferenceSystem; + import org.apache.sis.feature.builder.FeatureTypeBuilder; import org.apache.sis.referencing.CommonCRS; import org.apache.sis.test.TestCase; + import org.junit.Assert; import org.junit.Test; import org.locationtech.jts.geom.Coordinate; import org.locationtech.jts.geom.GeometryFactory; import org.locationtech.jts.geom.Point; -import org.opengis.feature.Feature; -import org.opengis.feature.FeatureType; -import org.opengis.filter.FilterFactory2; -import org.opengis.filter.expression.Function; -import org.opengis.referencing.crs.CoordinateReferenceSystem; + +import static org.junit.Assert.fail; /** * @@ -91,7 +96,23 @@ public class SQLMMTest extends TestCase { Assert.assertEquals(30.0, trs.getX(), 0.0); Assert.assertEquals(10.0, trs.getY(), 0.0); } - } + public void ST_Envelope() { + try { + new ST_Envelope(new Expression[2]); + fail("ST_Envelope operator should accept a single parameter"); + } catch (IllegalArgumentException e) { + // expected behavior + } + + try { + new ST_Envelope(null); + fail("ST_Envelope operator should accept a single parameter"); + } catch (IllegalArgumentException e) { + // expected behavior + } + + // TODO: update SIS version then add test cases. + } } 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 9aea3a0..ac3325f 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 @@ -7,10 +7,11 @@ import org.apache.sis.filter.DefaultFilterFactory; import org.apache.sis.geometry.GeneralEnvelope; import org.apache.sis.metadata.iso.extent.DefaultGeographicBoundingBox; import org.apache.sis.test.Assert; +import org.apache.sis.test.TestCase; import org.junit.Test; -public class FilterInterpreterTest { +public class FilterInterpreterTest extends TestCase { private static final FilterFactory2 FF = new DefaultFilterFactory(); @Test diff --git a/storage/sis-sqlstore/src/test/java/org/apache/sis/test/suite/SQLTestSuite.java b/storage/sis-sqlstore/src/test/java/org/apache/sis/test/suite/SQLTestSuite.java index 2ed67d8..3a24e21 100644 --- a/storage/sis-sqlstore/src/test/java/org/apache/sis/test/suite/SQLTestSuite.java +++ b/storage/sis-sqlstore/src/test/java/org/apache/sis/test/suite/SQLTestSuite.java @@ -25,7 +25,8 @@ import org.junit.BeforeClass; * All tests from the {@code sis-sqlstore} module, in rough dependency order. */ @Suite.SuiteClasses({ - org.apache.sis.storage.sql.SQLStoreTest.class + org.apache.sis.storage.sql.SQLStoreTest.class, + org.apache.sis.internal.sql.feature.FilterInterpreterTest.class }) public final strictfp class SQLTestSuite extends TestSuite { /**
