sis-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From ama...@apache.org
Subject [sis] 05/07: fix(Storage): better handling of CRS for spatial operators
Date Fri, 30 Jul 2021 16:49:41 GMT
This is an automated email from the ASF dual-hosted git repository.

amanin pushed a commit to branch geoapi-4.0
in repository https://gitbox.apache.org/repos/asf/sis.git

commit ee5c697af0513bf5abc9b6573c9578996bc7f00b
Author: Alexis Manin <alexis.manin@geomatys.com>
AuthorDate: Wed Jul 28 18:23:13 2021 +0200

    fix(Storage): better handling of CRS for spatial operators
---
 .../org/apache/sis/internal/filter/Visitor.java    |   2 +-
 .../sis/internal/sql/feature/ANSIInterpreter.java  |  18 ++
 .../sql/feature/GeometryIdentification.java        |   6 +
 .../internal/sql/feature/PostGISInterpreter.java   |  81 +++++-
 .../sql/feature/PostGISSpatialFilterAdapter.java   | 309 +++++++++++++++++++++
 .../sis/internal/sql/feature/QueryFeatureSet.java  |   2 +-
 .../sis/internal/sql/feature/SQLQueryAdapter.java  |  15 +-
 .../sql/feature/FilterInterpreterTest.java         |   9 +-
 8 files changed, 429 insertions(+), 13 deletions(-)

diff --git a/core/sis-feature/src/main/java/org/apache/sis/internal/filter/Visitor.java b/core/sis-feature/src/main/java/org/apache/sis/internal/filter/Visitor.java
index 2ba8bc6..b518171 100644
--- a/core/sis-feature/src/main/java/org/apache/sis/internal/filter/Visitor.java
+++ b/core/sis-feature/src/main/java/org/apache/sis/internal/filter/Visitor.java
@@ -236,7 +236,7 @@ public abstract class Visitor<R,A> {
      * @param  accumulator  where to write the result of all actions.
      * @throws UnsupportedOperationException if there is no action registered for the given
filter.
      */
-    public final void visit(final Filter<R> filter, final A accumulator) {
+    public void visit(final Filter<R> filter, final A accumulator) {
         final CodeList<?> type = (filter != null) ? filter.getOperatorType() : null;
         final BiConsumer<Filter<R>, A> f = filters.get(type);
         if (f != null) {
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 77a148d..0f69b03 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
@@ -19,6 +19,7 @@ package org.apache.sis.internal.sql.feature;
 import java.util.List;
 import java.util.Collections;
 import java.util.function.BiConsumer;
+import java.util.function.UnaryOperator;
 import org.opengis.util.LocalName;
 import org.opengis.util.GenericName;
 import org.opengis.util.NameSpace;
@@ -79,7 +80,18 @@ class ANSIInterpreter extends Visitor<Feature,StringBuilder> {
 
     private final NameSpace scope;
 
+    /**
+     * Called on each and every filter/subfilter visited by this interpreter.
+     */
+    private final UnaryOperator<Filter<Feature>> filterAdapter;
+
     public ANSIInterpreter() {
+        this(null);
+    }
+
+    public ANSIInterpreter(UnaryOperator<Filter<Feature>> filterAdapter) {
+        if (filterAdapter == null) filterAdapter = UnaryOperator.identity();
+        this.filterAdapter = filterAdapter;
         nameFactory = DefaultFactories.forBuildin(NameFactory.class);
         scope = nameFactory.createNameSpace(nameFactory.createLocalName(null, "xpath"),
                                             Collections.singletonMap("separator", "/"));
@@ -335,6 +347,12 @@ class ANSIInterpreter extends Visitor<Feature,StringBuilder> {
         }
     }
 
+    @Override
+    public void visit(Filter<Feature> filter, StringBuilder accumulator) {
+        final Filter<Feature> adapted = filterAdapter.apply(filter);
+        super.visit(adapted, accumulator);
+    }
+
     private static void ensureMatchCase(final boolean isMatchingCase) {
         if (!isMatchingCase) {
             throw new UnsupportedOperationException("case insensitive match is not defined
by ANSI SQL");
diff --git a/storage/sis-sqlstore/src/main/java/org/apache/sis/internal/sql/feature/GeometryIdentification.java
b/storage/sis-sqlstore/src/main/java/org/apache/sis/internal/sql/feature/GeometryIdentification.java
index e0305b9..08efc4a 100644
--- a/storage/sis-sqlstore/src/main/java/org/apache/sis/internal/sql/feature/GeometryIdentification.java
+++ b/storage/sis-sqlstore/src/main/java/org/apache/sis/internal/sql/feature/GeometryIdentification.java
@@ -135,6 +135,12 @@ class GeometryIdentification implements SQLCloseable {
         // Note: we make a query per entry, which could impact performance. However, 99%
of defined tables
         // will have only one geometry column. Moreover, even with more than one, with prepared
statement, the
         // performance impact should stay low.
+
+        /* TODO: Trace column dependencies if this is a column from a view.
+         * Problem: for views, PostGIS will NOT fill neither srid nor geometry type, UNLESS
user has statically cast its
+         * column to match a precise geometry type/srid.
+         * Source: https://gis.stackexchange.com/a/376947/182809
+         */
         final CoordinateReferenceSystem crs = crsIdent.fetchCrs(pgSrid);
         return new GeometryColumn(name, dimension, pgSrid, type, crs);
     }
diff --git a/storage/sis-sqlstore/src/main/java/org/apache/sis/internal/sql/feature/PostGISInterpreter.java
b/storage/sis-sqlstore/src/main/java/org/apache/sis/internal/sql/feature/PostGISInterpreter.java
index 92c7c30..54d790e 100644
--- a/storage/sis-sqlstore/src/main/java/org/apache/sis/internal/sql/feature/PostGISInterpreter.java
+++ b/storage/sis-sqlstore/src/main/java/org/apache/sis/internal/sql/feature/PostGISInterpreter.java
@@ -17,9 +17,26 @@
 package org.apache.sis.internal.sql.feature;
 
 // Branch-dependent imports
+import java.util.Objects;
+import java.util.Optional;
+import java.util.function.Function;
+import java.util.function.UnaryOperator;
+import java.util.logging.Level;
+import java.util.stream.Stream;
+import org.apache.sis.feature.Features;
+import org.apache.sis.internal.feature.AttributeConvention;
+import org.apache.sis.internal.util.Constants;
 import org.opengis.feature.Feature;
+import org.opengis.feature.FeatureType;
+import org.opengis.feature.PropertyNotFoundException;
+import org.opengis.feature.PropertyType;
+import org.opengis.filter.Filter;
 import org.opengis.filter.SpatialOperator;
 import org.opengis.filter.SpatialOperatorName;
+import org.opengis.filter.ValueReference;
+import org.opengis.referencing.crs.CoordinateReferenceSystem;
+
+import static org.apache.sis.internal.sql.feature.PostGISSpatialFilterAdapter.LOGGER;
 
 
 /**
@@ -31,10 +48,13 @@ import org.opengis.filter.SpatialOperatorName;
  * @module
  */
 final class PostGISInterpreter extends ANSIInterpreter {
-    /**
-     * Creates a new instance.
-     */
-    PostGISInterpreter() {
+
+    PostGISInterpreter(final FeatureType target) {
+        this(new PostGISSpatialFilterAdapter(init(target)));
+    }
+
+    PostGISInterpreter(UnaryOperator<Filter<Feature>> filterAdapter) {
+        super(filterAdapter);
     }
 
     /**
@@ -47,6 +67,57 @@ final class PostGISInterpreter extends ANSIInterpreter {
      */
     @Override
     void bbox(final StringBuilder sb, final SpatialOperator<Feature> filter) {
-        join(sb, filter, "&&");
+        join(sb, filter, " && ");
+    }
+
+    /**
+     * Creates a functor capable of searching the CRS associated to a given value reference.
For now, the returned
+     * function will only search in the target feature type. However, providing a functor
allows to evolve it without
+     * breaking user API.
+     */
+    private static Function<ValueReference<Feature, ?>, Optional<CoordinateReferenceSystem>>
init(final FeatureType target) {
+        if (target == null) return it -> Optional.empty();
+        return it -> searchForPropertyCrs(target, it);
+    }
+
+    private static Optional<CoordinateReferenceSystem> searchForPropertyCrs(final FeatureType
target, final ValueReference<Feature, ?> ref) {
+        return searchReference(target, ref)
+                .flatMap(Features::toAttribute)
+                .map(attr -> attr.characteristics().get(AttributeConvention.CRS_CHARACTERISTIC.toString()))
+                .map(it -> {
+                    final Object value = it.getDefaultValue();
+                    return (value instanceof CoordinateReferenceSystem) ? (CoordinateReferenceSystem)
value : null;
+                });
+    }
+
+    private static Optional<PropertyType> searchReference(final FeatureType target,
final ValueReference<Feature, ?> ref) {
+        final String xPath = ref.getXPath();
+        if (xPath == null) return Optional.empty();
+        final PropertyType brutSearch = searchProperty(target, xPath);
+        if (brutSearch != null) return Optional.of(brutSearch);
+
+        /* TODO: find a more robust strategy to find the property related to a value reference
+         * Notes:
+         *  - In the specific case of an SQL based property name, given XPath could be of
the form
+         *    <table>.<attribute>. We try to get it.
+         *  - Another "tweak" is that it is a common pattern to get a name of the form <namespace>:<value>.
+         *  - Other use-cases are not supported yet (Ex: a real xpath -> association1/association2/attribute).
+         */
+        return Stream.of('.', ':')
+                .mapToInt(sep -> xPath.lastIndexOf(sep))
+                .filter(it -> it >= 0 && it < xPath.length())
+                .mapToObj(idx -> xPath.substring(idx+1))
+                .map(name -> searchProperty(target, name))
+                .filter(Objects::nonNull)
+                .findFirst();
+    }
+
+    private static PropertyType searchProperty(FeatureType datatype, String propertyName)
{
+        try {
+            return datatype.getProperty(propertyName);
+        } catch (PropertyNotFoundException e) {
+            LOGGER.log(Level.FINER, e, () -> String.format("No property %s in data type
%s", propertyName, datatype.getName()));
+            return null;
+        }
     }
 }
diff --git a/storage/sis-sqlstore/src/main/java/org/apache/sis/internal/sql/feature/PostGISSpatialFilterAdapter.java
b/storage/sis-sqlstore/src/main/java/org/apache/sis/internal/sql/feature/PostGISSpatialFilterAdapter.java
new file mode 100644
index 0000000..f1f5e25
--- /dev/null
+++ b/storage/sis-sqlstore/src/main/java/org/apache/sis/internal/sql/feature/PostGISSpatialFilterAdapter.java
@@ -0,0 +1,309 @@
+/*
+ * 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.sis.internal.sql.feature;
+
+import java.util.Arrays;
+import java.util.Collections;
+import java.util.HashMap;
+import java.util.List;
+import java.util.Locale;
+import java.util.Map;
+import java.util.Optional;
+import java.util.function.Function;
+import java.util.function.UnaryOperator;
+import java.util.logging.Level;
+import java.util.logging.Logger;
+import org.apache.sis.geometry.Envelopes;
+import org.apache.sis.internal.feature.Geometries;
+import org.apache.sis.internal.feature.GeometryWrapper;
+import org.apache.sis.internal.filter.sqlmm.SQLMM;
+import org.apache.sis.internal.system.Modules;
+import org.apache.sis.referencing.CRS;
+import org.apache.sis.referencing.crs.AbstractCRS;
+import org.apache.sis.referencing.cs.AxesConvention;
+import org.apache.sis.util.Utilities;
+import org.apache.sis.util.collection.BackingStoreException;
+import org.apache.sis.util.logging.Logging;
+import org.opengis.feature.Feature;
+import org.opengis.filter.BinarySpatialOperator;
+import org.opengis.filter.Expression;
+import org.opengis.filter.Filter;
+import org.opengis.filter.InvalidFilterValueException;
+import org.opengis.filter.Literal;
+import org.opengis.filter.SpatialOperatorName;
+import org.opengis.filter.ValueReference;
+import org.opengis.geometry.Envelope;
+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.CodeList;
+import org.opengis.util.FactoryException;
+
+/**
+ * Modify operands of a spatial operator to make their CRS match each other.
+ * This is required for PostGIS that expect user to provide geometries in the same CRS.
+ * It also force {@link AxesConvention#DISPLAY_ORIENTED display orientation} on resulting
CRS / geometries.
+ *
+ * @author  Alexis Manin (Geomatys)
+ * @version 1.1
+ * @since   1.1
+ * @module
+ */
+final class PostGISSpatialFilterAdapter implements UnaryOperator<Filter<Feature>>
{
+
+    static final Logger LOGGER = Logging.getLogger(Modules.SQL);
+
+    /**
+     * Allow to detect third-party filters that use SQLMM function names instead of filter
names.
+     */
+    private static final Map<String, SpatialOperatorName> SQLMM_TO_FILTER;
+
+    static {
+        final Map<String, SpatialOperatorName> tmp = new HashMap<>();
+        tmp.put(toKey(SQLMM.ST_Contains), SpatialOperatorName.CONTAINS);
+        tmp.put(toKey(SQLMM.ST_Crosses), SpatialOperatorName.CROSSES);
+        tmp.put(toKey(SQLMM.ST_Disjoint), SpatialOperatorName.DISJOINT);
+        tmp.put(toKey(SQLMM.ST_Equals), SpatialOperatorName.EQUALS);
+        tmp.put(toKey(SQLMM.ST_Intersects), SpatialOperatorName.INTERSECTS);
+        tmp.put(toKey(SQLMM.ST_Overlaps), SpatialOperatorName.OVERLAPS);
+        tmp.put(toKey(SQLMM.ST_Touches), SpatialOperatorName.TOUCHES);
+        tmp.put(toKey(SQLMM.ST_Within), SpatialOperatorName.WITHIN);
+        SQLMM_TO_FILTER = Collections.unmodifiableMap(tmp);
+    }
+
+    private static String toKey(SQLMM value) {
+        return value.name().toLowerCase(Locale.ENGLISH);
+    }
+
+    private final Function<ValueReference<Feature, ?>, Optional<CoordinateReferenceSystem>>
fetchCrs;
+
+    PostGISSpatialFilterAdapter(Function<ValueReference<Feature, ?>, Optional<CoordinateReferenceSystem>>
fetchCrs) {
+        this.fetchCrs = fetchCrs;
+    }
+
+    @Override
+    public Filter<Feature> apply(Filter<Feature> base) {
+        if (base instanceof BinarySpatialOperator) return adaptBinarySpatialOp((BinarySpatialOperator<Feature>)
base);
+        return getSpatialType(base)
+                .map(type -> {
+                    final List<Expression<? super Feature, ?>> operands = base.getExpressions();
+                    if (operands.size() != 2) return base; // TODO: manage single operand
-> Ex: bbox filter matching any geometric column
+                    return adaptBinarySpatialOp(type, operands.get(0), operands.get(1), base);
+                })
+                .orElse(base);
+    }
+
+    private Optional<SpatialOperatorName> getSpatialType(Filter<Feature> base)
{
+        if (base == null) return Optional.empty();
+        final CodeList<?> baseType = base.getOperatorType();
+        SpatialOperatorName son;
+        if (baseType instanceof SpatialOperatorName) return Optional.of((SpatialOperatorName)
baseType);
+        else if (baseType == null) return Optional.empty();
+
+        final String filterTypeIdent = baseType.identifier() == null ? baseType.name() :
baseType.identifier();
+        son = SQLMM_TO_FILTER.get(filterTypeIdent.toLowerCase(Locale.ENGLISH));
+        if (son == null) {
+            try {
+                SpatialOperatorName.valueOf(filterTypeIdent);
+            } catch (Exception e) {
+                LOGGER.log(Level.FINE, e, () -> "No match in spatial ops for "+filterTypeIdent);
+                son = null;
+            }
+        }
+
+        return Optional.ofNullable(son);
+    }
+
+    private Filter<Feature> adaptBinarySpatialOp(BinarySpatialOperator<Feature>
operator) {
+        return adaptBinarySpatialOp(operator.getOperatorType(), operator.getOperand1(), operator.getOperand2(),
operator);
+    }
+
+    /**
+     * Verify if input filter or any of its operand need to be adapted. If it's the case,
we return a new filter
+     * instance that contains wanted changes.
+     *
+     * @param opType Information from input binary spatial operator: its {@link Filter#getOperatorType()
operation name}
+     * @param left The {@link BinarySpatialOperator#getOperand1() first operand} of the filter
+     * @param right The {@link BinarySpatialOperator#getOperand2() second operand} of the
filter
+     * @param source The filter object other parameters have been extracted from, for reference.
Can be useful to get
+     *               other informations that are not part of {@link BinarySpatialOperator}
interface. It is also needed,
+     *               so we can return it directly if no change is necessary.
+     */
+    private Filter<Feature> adaptBinarySpatialOp(SpatialOperatorName opType, final
Expression<? super Feature, ?> left, final Expression<? super Feature, ?> right,
Filter<Feature> source) {
+        final Object constVal;
+        boolean leftIsConstant = false;
+        if (left instanceof Literal) {
+            constVal = ((Literal<?, ?>) left).getValue();
+            leftIsConstant = true;
+        } else if (right instanceof Literal) {
+            constVal = ((Literal<?, ?>) right).getValue();
+        } else constVal = null;
+
+        if (constVal == null) return source;
+        final CoordinateReferenceSystem constValCrs = getCrs(constVal).orElse(null);
+        if (constValCrs == null) return source; // assume same CRS as other expression.
+
+        Expression<? super Feature, ?> other = leftIsConstant ? right : left;
+        final CoordinateReferenceSystem otherCrs = getTargetCrs(other).orElse(null);
+        if (otherCrs == null) {
+            /* TODO: we could mitigate this problem by forcing a ST_Transform operation upon
the non literal operand.
+             * However, I am a little afraid to do so, because it requires to be able to
safely define a PostGIS SRID.
+             * Moreover, it would potentially prevent PostGIS to use its index, hurting performance
a lot.
+             */
+            final String newLine = System.lineSeparator();
+            LOGGER.warning(
+                    "Context: Spatial filter upon a PostGIS table"+newLine+
+                    "Issue: We cannot guarantee the coherence between the two geometric operands
CRS"+ newLine+
+                    "Result: You might get a wrong result for your geometric comparison filter"
+ newLine +
+                    "Solution: To fix this, you should:"+newLine+
+                    "- Verify SRID constraints on your geometric columns"+newLine+
+                    "- In case the data source is an SQL view, you should cast statically
any geometric column like:" +
+                            " `SELECT geom_column::GEOMETRY(<TYPE>, <SRID>)`."+newLine+
+                    "- Otherwise, manually wrap one (or both) of the operand with a ST_Transform
expression");
+        }
+
+        // PostGIS CRS are all forced with east axis first.
+        final CoordinateReferenceSystem normalizedTargetCrs = forceEastFirst(otherCrs ==
null ? constValCrs : otherCrs);
+
+        final Object adaptedVal = findTransform(constValCrs, normalizedTargetCrs)
+                .map(transform -> transform(constVal, transform))
+                .orElse(constVal);
+
+        if (adaptedVal != constVal) {
+            return new BinarySpatialOpProxy(source, opType, other, new Literal<Feature,
Object>() {
+
+                @Override
+                public Object getValue() {
+                    return adaptedVal;
+                }
+
+                @Override
+                public <N> Expression<Feature, N> toValueType(Class<N>
aClass) {
+                    throw new UnsupportedOperationException("Not supported yet");
+                }
+
+            });
+        }
+
+        return source;
+    }
+
+    private Optional<CoordinateOperation> findTransform(CoordinateReferenceSystem from,
CoordinateReferenceSystem to) {
+        if (from == null || to == null || from == to || Utilities.equalsIgnoreMetadata(from,
to)) return Optional.empty();
+        try {
+            final CoordinateOperation op = CRS.findOperation(from, to, null);
+            final MathTransform transform = op.getMathTransform();
+            // If the transform is a no-op, we avoid mutating filter for nothing.
+            if (transform == null || transform.isIdentity()) return Optional.empty();
+            else return Optional.of(op);
+        } catch (FactoryException e) {
+            throw new BackingStoreException("Cannot prepare CRS conversion for spatial filter",
e);
+        }
+    }
+
+    private Object transform(Object constVal, CoordinateOperation operation) {
+        try {
+            if (constVal instanceof Envelope) return Envelopes.transform(operation, (Envelope)
constVal);
+            else if (constVal instanceof GeometryWrapper)
+                return ((GeometryWrapper<?>) constVal).transform(operation, false);
+            else {
+                final GeometryWrapper<?> wrapper = Geometries.wrap(constVal)
+                        .orElseThrow(() -> new IllegalArgumentException("Expression value
is unsupported: " + constVal.getClass()))
+                        .transform(operation, false);
+                return wrapper.implementation();
+            }
+        } catch (TransformException | FactoryException e) {
+            throw new BackingStoreException("Cannot transform expression value of a spatial
filter", e);
+        }
+    }
+
+    private CoordinateReferenceSystem forceEastFirst(CoordinateReferenceSystem crs) {
+        return AbstractCRS.castOrCopy(crs).forConvention(AxesConvention.DISPLAY_ORIENTED);
+    }
+
+    private Optional<CoordinateReferenceSystem> getCrs(Object constVal) {
+        if (constVal instanceof Envelope) {
+            return Optional.ofNullable(((Envelope) constVal).getCoordinateReferenceSystem());
+        } else if (constVal instanceof org.opengis.geometry.Geometry) {
+            return Optional.ofNullable(((org.opengis.geometry.Geometry) constVal).getCoordinateReferenceSystem());
+        } else return Geometries.wrap(constVal)
+                .map(org.opengis.geometry.Geometry::getCoordinateReferenceSystem);
+    }
+
+    private Optional<CoordinateReferenceSystem> getTargetCrs(Expression<? super
Feature, ?> other) {
+        if (other instanceof ValueReference<?, ?>) {
+            return fetchCrs.apply((ValueReference<Feature, ?>) other);
+        } else {
+            final String fnName = other.getFunctionName().tip().toString().toLowerCase(Locale.ENGLISH);
+            if (SQLMM.ST_Transform.name().toLowerCase(Locale.ENGLISH).equals(fnName)) {
+                for (Expression<?, ?> arg : other.getParameters()) {
+                    try {
+                        // DO NOT apply toValueType. I want to avoid it in case it is "smart
enough" to extract a CRS
+                        // from the geometry to transform
+                        final Object value = arg.apply(null);
+                        if (value instanceof CoordinateReferenceSystem) return Optional.of((CoordinateReferenceSystem)
value);
+                    } catch (Exception e) {
+                        LOGGER.log(Level.FINEST, "Cannot evaluate expression without input",
e);
+                    }
+                }
+            }
+        }
+        return Optional.empty();
+    }
+
+    private static final class BinarySpatialOpProxy implements BinarySpatialOperator<Feature>
{
+        final Filter<Feature> source;
+
+        final SpatialOperatorName op;
+        final Expression<? super Feature, ?> left;
+        final Expression<? super Feature, ?> right;
+
+        private BinarySpatialOpProxy(Filter<Feature> source, SpatialOperatorName op,
Expression<? super Feature, ?> left, Expression<? super Feature, ?> right) {
+            this.source = source;
+            this.op = op;
+            this.left = left;
+            this.right = right;
+        }
+
+        @Override
+        public SpatialOperatorName getOperatorType() {
+            return op;
+        }
+
+        @Override
+        public Expression<? super Feature, ?> getOperand1() {
+            return left;
+        }
+
+        @Override
+        public Expression<? super Feature, ?> getOperand2() {
+            return right;
+        }
+
+        @Override
+        public List<Expression<? super Feature, ?>> getExpressions() {
+            return Arrays.asList(left, right);
+        }
+
+        @Override
+        public boolean test(Feature feature) throws InvalidFilterValueException {
+            LOGGER.fine("A filter meant to be optimized as a PostGIS query has been executed
by Java");
+            return source.test(feature);
+        }
+    }
+}
diff --git a/storage/sis-sqlstore/src/main/java/org/apache/sis/internal/sql/feature/QueryFeatureSet.java
b/storage/sis-sqlstore/src/main/java/org/apache/sis/internal/sql/feature/QueryFeatureSet.java
index 255fb06..57f2c00 100644
--- a/storage/sis-sqlstore/src/main/java/org/apache/sis/internal/sql/feature/QueryFeatureSet.java
+++ b/storage/sis-sqlstore/src/main/java/org/apache/sis/internal/sql/feature/QueryFeatureSet.java
@@ -219,7 +219,7 @@ public class QueryFeatureSet extends AbstractFeatureSet {
     class SubsetAdapter extends SQLQueryAdapter {
 
         SubsetAdapter() {
-            super(queryBuilder.dialect);
+            super(queryBuilder.dialect, adapter.type);
         }
 
         @Override
diff --git a/storage/sis-sqlstore/src/main/java/org/apache/sis/internal/sql/feature/SQLQueryAdapter.java
b/storage/sis-sqlstore/src/main/java/org/apache/sis/internal/sql/feature/SQLQueryAdapter.java
index e369936..cf8a21e 100644
--- a/storage/sis-sqlstore/src/main/java/org/apache/sis/internal/sql/feature/SQLQueryAdapter.java
+++ b/storage/sis-sqlstore/src/main/java/org/apache/sis/internal/sql/feature/SQLQueryAdapter.java
@@ -25,6 +25,7 @@ import org.apache.sis.storage.FeatureSet;
 
 // Branch-dependent imports
 import org.opengis.feature.Feature;
+import org.opengis.feature.FeatureType;
 import org.opengis.filter.Filter;
 import org.opengis.filter.SortProperty;
 
@@ -49,8 +50,8 @@ abstract class SQLQueryAdapter implements SubsetAdapter.AdapterBuilder {
         this(new ANSIInterpreter());
     }
 
-    protected SQLQueryAdapter(final Dialect dbDialect) {
-        this(forDialect(dbDialect));
+    protected SQLQueryAdapter(final Dialect dbDialect, FeatureType sourceDataType) {
+        this(forDialect(dbDialect, sourceDataType));
     }
 
     protected SQLQueryAdapter(final ANSIInterpreter filterInterpreter) {
@@ -129,7 +130,7 @@ abstract class SQLQueryAdapter implements SubsetAdapter.AdapterBuilder
{
     static class Table extends SQLQueryAdapter {
         final org.apache.sis.internal.sql.feature.Table parent;
         public Table(org.apache.sis.internal.sql.feature.Table parent) {
-            super(parent.createStatement().dialect);
+            super(parent.createStatement().dialect, parent.featureType);
             this.parent = parent;
         }
 
@@ -147,10 +148,14 @@ abstract class SQLQueryAdapter implements SubsetAdapter.AdapterBuilder
{
      *
      * @param dialect Database dialect that must be produced by the interpreter. If null,
{@link Dialect#ANSI} is used
      *                as a fallback.
+     * @param target An optional data type, that gives interpreter extra-information about
what dataset is going to be
+     *               filtered.
      */
-    private static ANSIInterpreter forDialect(final Dialect dialect) {
+    private static ANSIInterpreter forDialect(final Dialect dialect, final FeatureType target)
{
+        // TODO: maybe in the future, the feature type might be replaced with a dedicated
"companion" that
+        // provides various information to help the interpreter, whatever target dialect.
         switch (dialect) {
-            case POSTGRESQL: return new PostGISInterpreter();
+            case POSTGRESQL: return new PostGISInterpreter(target);
             default: return new ANSIInterpreter();
         }
     }
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 4b39e05..56cdda6 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
@@ -19,6 +19,7 @@ package org.apache.sis.internal.sql.feature;
 import java.util.Arrays;
 import java.util.Collections;
 import java.util.List;
+import org.apache.sis.feature.builder.FeatureTypeBuilder;
 import org.apache.sis.filter.DefaultFilterFactory;
 import org.apache.sis.geometry.GeneralEnvelope;
 import org.apache.sis.metadata.iso.extent.DefaultGeographicBoundingBox;
@@ -30,12 +31,14 @@ import static org.apache.sis.test.Assert.*;
 
 // Branch-dependent imports
 import org.opengis.feature.Feature;
+import org.opengis.feature.FeatureType;
 import org.opengis.filter.Expression;
 import org.opengis.filter.Filter;
 import org.opengis.filter.FilterFactory;
 import org.opengis.filter.InvalidFilterValueException;
 import org.opengis.filter.SpatialOperator;
 import org.opengis.filter.SpatialOperatorName;
+import org.opengis.metadata.acquisition.GeometryType;
 import org.opengis.util.CodeList;
 
 import static org.opengis.filter.SpatialOperatorName.BBOX;
@@ -86,7 +89,11 @@ public final strictfp class FilterInterpreterTest extends TestCase {
      */
     @Test
     public void testGeometricFilterWithTransform() {
-        final PostGISInterpreter interpreter = new PostGISInterpreter();
+        final FeatureTypeBuilder builder = new FeatureTypeBuilder().setName("Mock");
+        builder.addAttribute(GeometryType.POINT).setName("Toto").setCRS(CommonCRS.defaultGeographic());
+        final FeatureType mockType = builder.build();
+
+        final PostGISInterpreter interpreter = new PostGISInterpreter(mockType);
         final GeneralEnvelope bbox = new GeneralEnvelope(CommonCRS.WGS84.geographic());
         bbox.setEnvelope(-10, 20, -5, 25);
         String expectedQueryString = "ST_Intersects(\"Toto\"," +

Mime
View raw message