This is an automated email from the ASF dual-hosted git repository. desruisseaux pushed a commit to branch geoapi-4.0 in repository https://gitbox.apache.org/repos/asf/sis.git
commit c4484bf53d35c7063f0bce040a0760b0ee2d2bbc Author: Martin Desruisseaux <[email protected]> AuthorDate: Thu Oct 30 17:02:42 2025 +0100 When features are filtered by identifiers, the `ResourceId` should be translated to a SQL `WHERE` clause when possible. --- .../apache/sis/filter/BinaryGeometryFilter.java | 2 +- .../org/apache/sis/filter/IdentifierFilter.java | 58 ++++++++++++++++++---- .../main/org/apache/sis/filter/PropertyValue.java | 3 +- .../org/apache/sis/filter/base/XPathSource.java | 33 ++++++++++++ .../apache/sis/filter/visitor/FunctionNames.java | 13 ++++- .../sis/storage/sql/feature/SelectionClause.java | 9 ++-- .../storage/sql/feature/SelectionClauseWriter.java | 20 ++++++-- .../org/apache/sis/storage/sql/SQLStoreTest.java | 43 ++++++++++------ 8 files changed, 144 insertions(+), 37 deletions(-) diff --git a/endorsed/src/org.apache.sis.feature/main/org/apache/sis/filter/BinaryGeometryFilter.java b/endorsed/src/org.apache.sis.feature/main/org/apache/sis/filter/BinaryGeometryFilter.java index a8518c4ad5..8df570ae0c 100644 --- a/endorsed/src/org.apache.sis.feature/main/org/apache/sis/filter/BinaryGeometryFilter.java +++ b/endorsed/src/org.apache.sis.feature/main/org/apache/sis/filter/BinaryGeometryFilter.java @@ -238,7 +238,7 @@ abstract class BinaryGeometryFilter<R> extends Node implements SpatialOperator<R * If one of the "effective" parameter has been modified, recreate a new filter. * If all operands are literal, we can evaluate that filter immediately. */ - Filter<R> filter = this; + BinaryGeometryFilter<R> filter = this; if ((effective1 != geometry1) || (effective2 != geometry2)) { filter = recreate(effective1, effective2); } diff --git a/endorsed/src/org.apache.sis.feature/main/org/apache/sis/filter/IdentifierFilter.java b/endorsed/src/org.apache.sis.feature/main/org/apache/sis/filter/IdentifierFilter.java index 4bd0b7a365..c3439df00c 100644 --- a/endorsed/src/org.apache.sis.feature/main/org/apache/sis/filter/IdentifierFilter.java +++ b/endorsed/src/org.apache.sis.feature/main/org/apache/sis/filter/IdentifierFilter.java @@ -20,10 +20,13 @@ import java.util.List; import java.util.Collection; import java.util.Objects; import org.apache.sis.filter.base.Node; +import org.apache.sis.filter.base.XPathSource; +import org.apache.sis.feature.Features; import org.apache.sis.feature.internal.shared.AttributeConvention; // Specific to the geoapi-3.1 and geoapi-4.0 branches: import org.opengis.feature.Feature; +import org.opengis.feature.FeatureType; import org.opengis.feature.PropertyNotFoundException; import org.opengis.filter.Expression; import org.opengis.filter.ResourceId; @@ -31,17 +34,26 @@ import org.opengis.filter.Filter; /** - * Filter features using a set of predefined identifiers and discarding features - * whose identifier is not in the set. + * Filter features using a set of predefined identifiers. + * Features without identifiers are discarded. * * @author Johann Sorel (Geomatys) * @author Martin Desruisseaux (Geomatys) */ -final class IdentifierFilter extends Node implements ResourceId<Feature>, Optimization.OnFilter<Feature> { +final class IdentifierFilter extends Node + implements ResourceId<Feature>, XPathSource, Optimization.OnFilter<Feature> +{ /** * For cross-version compatibility. */ - private static final long serialVersionUID = 1404452049863376235L; + private static final long serialVersionUID = 5917022442937908715L; + + /** + * Name of the property which contains the identifier. + * The initial value is {@value AttributeConvention#IDENTIFIER}, + * but this name may be replaced by a more direct target if known. + */ + private final String property; /** * The identifier of features to retain. @@ -49,18 +61,36 @@ final class IdentifierFilter extends Node implements ResourceId<Feature>, Optimi private final String identifier; /** - * Creates a new filter using the given identifier. + * Creates a new filter for filtering features having the given identifier. */ - IdentifierFilter(final String identifier) { + public IdentifierFilter(final String identifier) { + this.property = AttributeConvention.IDENTIFIER; this.identifier = Objects.requireNonNull(identifier); } /** - * Nothing to optimize here. The {@code Optimization.OnFilter} interface - * is implemented for inheriting the AND, OR and NOT methods overriding. + * Creates a new filter searching for the same identifier than the original filter, + * but looking in a different property. + */ + private IdentifierFilter(final IdentifierFilter original, final String property) { + this.property = property; + this.identifier = original.identifier; + } + + /** + * If the evaluated property is a link, replaces this filter by a more direct reference to the target property. + * This optimization helps {@code SQLStore} to put the column name in the <abbr>SQL</abbr> {@code WHERE} clause. + * It can make the difference between using or not the database index. */ @Override public Filter<Feature> optimize(Optimization optimization) { + final FeatureType type = optimization.getFeatureType(); + if (type != null) try { + return Features.getLinkTarget(type.getProperty(property)).map((n) -> new IdentifierFilter(this, n)).orElse(this); + } catch (PropertyNotFoundException e) { + warning(e, true); + return Filter.exclude(); + } return this; } @@ -72,6 +102,14 @@ final class IdentifierFilter extends Node implements ResourceId<Feature>, Optimi return Feature.class; } + /** + * Returns the path to the property which will be used by the {@code test(R)} method. + */ + @Override + public String getXPath() { + return property; + } + /** * Returns the identifiers of feature instances to accept. */ @@ -94,7 +132,7 @@ final class IdentifierFilter extends Node implements ResourceId<Feature>, Optimi */ @Override protected Collection<?> getChildren() { - return List.of(identifier); + return List.of(property, identifier); } /** @@ -104,7 +142,7 @@ final class IdentifierFilter extends Node implements ResourceId<Feature>, Optimi @Override public boolean test(final Feature object) { if (object != null) try { - Object id = object.getPropertyValue(AttributeConvention.IDENTIFIER); + Object id = object.getPropertyValue(property); if (id != null) return identifier.equals(id.toString()); } catch (PropertyNotFoundException e) { warning(e, false); diff --git a/endorsed/src/org.apache.sis.feature/main/org/apache/sis/filter/PropertyValue.java b/endorsed/src/org.apache.sis.feature/main/org/apache/sis/filter/PropertyValue.java index a650bc3d5d..c4d56289c6 100644 --- a/endorsed/src/org.apache.sis.feature/main/org/apache/sis/filter/PropertyValue.java +++ b/endorsed/src/org.apache.sis.feature/main/org/apache/sis/filter/PropertyValue.java @@ -25,6 +25,7 @@ import org.apache.sis.util.ObjectConverters; import org.apache.sis.util.UnconvertibleObjectException; import org.apache.sis.feature.internal.shared.FeatureProjectionBuilder; import org.apache.sis.filter.base.XPath; +import org.apache.sis.filter.base.XPathSource; import org.apache.sis.util.resources.Errors; // Specific to the geoapi-3.1 and geoapi-4.0 branches: @@ -50,7 +51,7 @@ import org.opengis.filter.ValueReference; * @see AssociationValue */ abstract class PropertyValue<V> extends LeafExpression<Feature,V> - implements ValueReference<Feature,V>, Optimization.OnExpression<Feature,V> + implements ValueReference<Feature,V>, XPathSource, Optimization.OnExpression<Feature,V> { /** * For cross-version compatibility. diff --git a/endorsed/src/org.apache.sis.feature/main/org/apache/sis/filter/base/XPathSource.java b/endorsed/src/org.apache.sis.feature/main/org/apache/sis/filter/base/XPathSource.java new file mode 100644 index 0000000000..564ef31946 --- /dev/null +++ b/endorsed/src/org.apache.sis.feature/main/org/apache/sis/filter/base/XPathSource.java @@ -0,0 +1,33 @@ +/* + * 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.filter.base; + + +/** + * Filters or expressions fetching their data from an XPath. + * + * @author Martin Desruisseaux (Geomatys) + */ +public interface XPathSource { + /** + * Returns the path to the property which will be used by the {@code test(R)} or {@code apply(R)} method. + * Usually, this path is simply the name of a property in a feature. + * + * @return path to the property which will be used by the {@code apply(R)} method. + */ + String getXPath(); +} diff --git a/endorsed/src/org.apache.sis.feature/main/org/apache/sis/filter/visitor/FunctionNames.java b/endorsed/src/org.apache.sis.feature/main/org/apache/sis/filter/visitor/FunctionNames.java index 6a67034365..e1d1b1ad0f 100644 --- a/endorsed/src/org.apache.sis.feature/main/org/apache/sis/filter/visitor/FunctionNames.java +++ b/endorsed/src/org.apache.sis.feature/main/org/apache/sis/filter/visitor/FunctionNames.java @@ -16,11 +16,13 @@ */ package org.apache.sis.filter.visitor; +import org.opengis.util.CodeList; +import org.apache.sis.filter.DefaultFilterFactory; import org.apache.sis.filter.sqlmm.SQLMM; /** - * Names of some expressions used in Apache <abbr>SIS</abbr>. + * Names of some filters and expressions used in Apache <abbr>SIS</abbr>. * This class defines only the names that need to be referenced from at least two different classes. * * @author Martin Desruisseaux (Geomatys) @@ -85,4 +87,13 @@ public final class FunctionNames { */ private FunctionNames() { } + + /** + * Workaround for the fact that there is no public constant for the identifier of the {@code ResourceId} filter. + * + * @return the identifier of the {@code ResourceId} filter. + */ + public static CodeList<?> resourceId() { + return DefaultFilterFactory.forFeatures().resourceId("resourceId").getOperatorType(); + } } diff --git a/endorsed/src/org.apache.sis.storage.sql/main/org/apache/sis/storage/sql/feature/SelectionClause.java b/endorsed/src/org.apache.sis.storage.sql/main/org/apache/sis/storage/sql/feature/SelectionClause.java index f429a52b75..7e10d67412 100644 --- a/endorsed/src/org.apache.sis.storage.sql/main/org/apache/sis/storage/sql/feature/SelectionClause.java +++ b/endorsed/src/org.apache.sis.storage.sql/main/org/apache/sis/storage/sql/feature/SelectionClause.java @@ -154,14 +154,17 @@ public final class SelectionClause extends SQLBuilder { /** * Writes the name of a column, or marks the SQL as invalid if the column is not found. * - * @param ref reference to a property to insert in SQL statement. + * @param column the column name to insert in <abbr>SQL</abbr> statement. + * @return whether the given column name is valid. */ - final void appendColumnName(final ValueReference<Feature,?> ref) { - final Column c = table.getColumn(ref.getXPath()); + final boolean appendColumnName(final String column) { + final Column c = table.getColumn(column); if (c != null) { appendIdentifier(c.name); + return true; } else { invalidate(); + return false; } } diff --git a/endorsed/src/org.apache.sis.storage.sql/main/org/apache/sis/storage/sql/feature/SelectionClauseWriter.java b/endorsed/src/org.apache.sis.storage.sql/main/org/apache/sis/storage/sql/feature/SelectionClauseWriter.java index dc189e4eaa..f60aa46b36 100644 --- a/endorsed/src/org.apache.sis.storage.sql/main/org/apache/sis/storage/sql/feature/SelectionClauseWriter.java +++ b/endorsed/src/org.apache.sis.storage.sql/main/org/apache/sis/storage/sql/feature/SelectionClauseWriter.java @@ -24,6 +24,7 @@ import java.sql.Connection; import java.sql.DatabaseMetaData; import java.sql.ResultSet; import java.sql.SQLException; +import org.apache.sis.filter.base.XPathSource; import org.apache.sis.filter.visitor.FunctionNames; import org.apache.sis.filter.visitor.Visitor; @@ -40,6 +41,7 @@ import org.opengis.filter.ComparisonOperatorName; import org.opengis.filter.BinaryComparisonOperator; import org.opengis.filter.SpatialOperatorName; import org.opengis.filter.BetweenComparisonOperator; +import org.opengis.filter.ResourceId; /** @@ -80,11 +82,19 @@ public class SelectionClauseWriter extends Visitor<Feature, SelectionClause> { setFilterHandler(ComparisonOperatorName.PROPERTY_IS_LESS_THAN, new Comparison(" < ")); setFilterHandler(ComparisonOperatorName.PROPERTY_IS_LESS_THAN_OR_EQUAL_TO, new Comparison(" <= ")); setFilterHandler(ComparisonOperatorName.valueOf(FunctionNames.PROPERTY_IS_BETWEEN), (f,sql) -> { - final BetweenComparisonOperator<Feature> filter = (BetweenComparisonOperator<Feature>) f; + final var filter = (BetweenComparisonOperator<Feature>) f; /* Nothing to append */ if (write(sql, filter.getExpression())) return; sql.append(" BETWEEN "); if (write(sql, filter.getLowerBoundary())) return; sql.append(" AND "); write(sql, filter.getUpperBoundary()); }); + setFilterHandler(FunctionNames.resourceId(), (f,sql) -> { + if (f instanceof XPathSource && sql.appendColumnName(((XPathSource) f).getXPath())) { + final var filter = (ResourceId<?>) f; + sql.append(" = ").appendValue(filter.getIdentifier()); + } else { + sql.invalidate(); + } + }); setNullAndNilHandlers((filter, sql) -> { final List<Expression<Feature, ?>> expressions = filter.getExpressions(); if (expressions.size() == 1) { @@ -113,8 +123,8 @@ public class SelectionClauseWriter extends Visitor<Feature, SelectionClause> { setExpressionHandler(FunctionNames.Divide, new Arithmetic(" / ")); setExpressionHandler(FunctionNames.Multiply, new Arithmetic(" * ")); setExpressionHandler(FunctionNames.Literal, (e,sql) -> sql.appendLiteral(((Literal<Feature,?>) e).getValue())); - setExpressionHandler(FunctionNames.ValueReference, (e,sql) -> sql.appendColumnName((ValueReference<Feature,?>) e)); - // Filters created from Filter Encoding XML can specify "PropertyName" instead of "Value reference". + setExpressionHandler(FunctionNames.ValueReference, (e,sql) -> sql.appendColumnName(((ValueReference<Feature,?>) e).getXPath())); + // Filters created from Filter Encoding XML may specify "PropertyName" instead of "Value reference". setExpressionHandler("PropertyName", getExpressionHandler(FunctionNames.ValueReference)); } @@ -316,7 +326,7 @@ public class SelectionClauseWriter extends Visitor<Feature, SelectionClause> { /** Invoked when a logical filter needs to be converted to SQL clause. */ @Override public void accept(final Filter<Feature> f, final SelectionClause sql) { - final LogicalOperator<Feature> filter = (LogicalOperator<Feature>) f; + final var filter = (LogicalOperator<Feature>) f; final List<Filter<Feature>> operands = filter.getOperands(); final int n = operands.size(); if (unary ? (n != 1) : (n == 0)) { @@ -354,7 +364,7 @@ public class SelectionClauseWriter extends Visitor<Feature, SelectionClause> { /** Invoked when a comparison needs to be converted to SQL clause. */ @Override public void accept(final Filter<Feature> f, final SelectionClause sql) { - final BinaryComparisonOperator<Feature> filter = (BinaryComparisonOperator<Feature>) f; + final var filter = (BinaryComparisonOperator<Feature>) f; if (filter.isMatchingCase()) { writeBinaryOperator(sql, filter, operator); } else { diff --git a/endorsed/src/org.apache.sis.storage.sql/test/org/apache/sis/storage/sql/SQLStoreTest.java b/endorsed/src/org.apache.sis.storage.sql/test/org/apache/sis/storage/sql/SQLStoreTest.java index f71123d66c..35a30f8961 100644 --- a/endorsed/src/org.apache.sis.storage.sql/test/org/apache/sis/storage/sql/SQLStoreTest.java +++ b/endorsed/src/org.apache.sis.storage.sql/test/org/apache/sis/storage/sql/SQLStoreTest.java @@ -190,6 +190,7 @@ public final class SQLStoreTest extends TestOnAllDatabases { */ verifySimpleQuerySorting(store); verifySimpleQueryWithLimit(store); + verifyWhereResourceId(store); verifySimpleWhere(store); verifyWhereOnLink(store); verifyLinkInProjection(store); @@ -386,6 +387,23 @@ public final class SQLStoreTest extends TestOnAllDatabases { assertEquals(2, subset.features(false).count()); } + /** + * Requests a new set of features filtered by an identifier. + * + * @param dataset the store on which to query the features. + * @throws DataStoreException if an error occurred during query execution. + */ + private void verifyWhereResourceId(final SimpleFeatureStore dataset) throws Exception { + final FeatureQuery query = new FeatureQuery(); + query.setSelection(FF.resourceId("FRA")); + final Object[] result; + final FeatureSet countries = dataset.findResource("Countries"); + try (Stream<Feature> features = countries.subset(query).features(false)) { + result = features.map(f -> f.getPropertyValue("native_name")).toArray(); + } + assertArrayEquals(new String[] {"France"}, result); + } + /** * Requests a new set of features filtered by an arbitrary condition, * and verifies that we get only the expected values. @@ -394,26 +412,19 @@ public final class SQLStoreTest extends TestOnAllDatabases { * @throws DataStoreException if an error occurred during query execution. */ private void verifySimpleWhere(final SimpleFeatureStore dataset) throws Exception { + final FeatureQuery query = new FeatureQuery(); + query.setSelection(FF.equal(FF.property("country"), FF.literal("CAN"))); /* - * Property that we are going to request and expected result. - */ - final String desiredProperty = "native_name"; - final String[] expectedValues = { - "Montréal", "Québec" - }; - /* - * Build the query and get a new set of features. The values returned by the database can be in any order, - * so we use `assertSetEquals(…)` for making the test insensitive to feature order. An alternative would be - * to add a `query.setSortBy(…)` call, but we avoid that for making this test only about the `WHERE` clause. + * The values returned by the database can be in any order, so we use `assertSetEquals(…)` for making + * the test insensitive to feature order. An alternative would be to add a `query.setSortBy(…)` call, + * but we avoid that for making this test only about the `WHERE` clause. */ - final FeatureSet cities = dataset.findResource("Cities"); - final FeatureQuery query = new FeatureQuery(); - query.setSelection(FF.equal(FF.property("country"), FF.literal("CAN"))); - final Object[] names; + final Object[] result; + final FeatureSet cities = dataset.findResource("Cities"); try (Stream<Feature> features = cities.subset(query).features(false)) { - names = features.map(f -> f.getPropertyValue(desiredProperty)).toArray(); + result = features.map(f -> f.getPropertyValue("native_name")).toArray(); } - assertSetEquals(Arrays.asList(expectedValues), Arrays.asList(names)); + assertSetEquals(Arrays.asList("Montréal", "Québec"), Arrays.asList(result)); } /**
