This is an automated email from the ASF dual-hosted git repository.

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


The following commit(s) were added to refs/heads/geoapi-4.0 by this push:
     new 6723ac3a80 feat(Shapefile): optimize bbox filter and field selection, 
declare store in registry
6723ac3a80 is described below

commit 6723ac3a80fcbfe9dac11a61705d7117c1fad04a
Author: jsorel <johann.so...@geomatys.com>
AuthorDate: Mon Nov 13 17:05:57 2023 +0100

    feat(Shapefile): optimize bbox filter and field selection, declare store in 
registry
---
 .../org.apache.sis.feature/main/module-info.java   |   6 +-
 .../org.apache.sis.storage.DataStoreProvider       |   4 +
 .../main/module-info.java                          |   3 +
 .../storage/shapefile/ListingPropertyVisitor.java  |  79 +++++
 .../sis/storage/shapefile/ShapefileStore.java      | 348 ++++++++++++++++++---
 .../sis/storage/shapefile/ShapefileStoreTest.java  |  53 +++-
 6 files changed, 439 insertions(+), 54 deletions(-)

diff --git a/endorsed/src/org.apache.sis.feature/main/module-info.java 
b/endorsed/src/org.apache.sis.feature/main/module-info.java
index 2c55dac851..4baecf5816 100644
--- a/endorsed/src/org.apache.sis.feature/main/module-info.java
+++ b/endorsed/src/org.apache.sis.feature/main/module-info.java
@@ -45,7 +45,8 @@ module org.apache.sis.feature {
             org.apache.sis.storage,
             org.apache.sis.storage.sql,
             org.apache.sis.cql,                     // In the "incubator" 
sub-project.
-            org.apache.sis.portrayal;
+            org.apache.sis.portrayal,
+            org.apache.sis.storage.shapefile;
 
     exports org.apache.sis.feature.internal to
             org.apache.sis.storage,
@@ -60,7 +61,8 @@ module org.apache.sis.feature {
             org.apache.sis.storage.xml,
             org.apache.sis.storage.sql,
             org.apache.sis.storage.netcdf,
-            org.apache.sis.cql;                     // In the "incubator" 
sub-project.
+            org.apache.sis.storage.shapefile,       // In the "incubator" 
sub-project.
+            org.apache.sis.cql;
 
     exports org.apache.sis.geometry.wrapper.j2d to
             org.apache.sis.gui;                     // In the "optional" 
sub-project.
diff --git 
a/incubator/src/org.apache.sis.storage.shapefile/main/META-INF/services/org.apache.sis.storage.DataStoreProvider
 
b/incubator/src/org.apache.sis.storage.shapefile/main/META-INF/services/org.apache.sis.storage.DataStoreProvider
new file mode 100644
index 0000000000..3cb460a2cb
--- /dev/null
+++ 
b/incubator/src/org.apache.sis.storage.shapefile/main/META-INF/services/org.apache.sis.storage.DataStoreProvider
@@ -0,0 +1,4 @@
+# Workaround for Maven bug https://issues.apache.org/jira/browse/MNG-7855
+# The content of this file is automatically derived from module-info.class 
file.
+# Should be used only if the JAR file was on class-path rather than 
module-path.
+org.apache.sis.storage.shapefile.ShapefileProvider
diff --git 
a/incubator/src/org.apache.sis.storage.shapefile/main/module-info.java 
b/incubator/src/org.apache.sis.storage.shapefile/main/module-info.java
index d9cd721f2f..9055e720e2 100644
--- a/incubator/src/org.apache.sis.storage.shapefile/main/module-info.java
+++ b/incubator/src/org.apache.sis.storage.shapefile/main/module-info.java
@@ -30,4 +30,7 @@ module org.apache.sis.storage.shapefile {
     exports org.apache.sis.storage.shapefile.cpg;
     exports org.apache.sis.storage.shapefile.shp;
     exports org.apache.sis.storage.shapefile.dbf;
+
+    provides org.apache.sis.storage.DataStoreProvider
+            with org.apache.sis.storage.shapefile.ShapefileProvider;
 }
diff --git 
a/incubator/src/org.apache.sis.storage.shapefile/main/org/apache/sis/storage/shapefile/ListingPropertyVisitor.java
 
b/incubator/src/org.apache.sis.storage.shapefile/main/org/apache/sis/storage/shapefile/ListingPropertyVisitor.java
new file mode 100644
index 0000000000..819d9340d0
--- /dev/null
+++ 
b/incubator/src/org.apache.sis.storage.shapefile/main/org/apache/sis/storage/shapefile/ListingPropertyVisitor.java
@@ -0,0 +1,79 @@
+/*
+ * 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.storage.shapefile;
+
+import java.util.Collection;
+import org.apache.sis.filter.internal.FunctionNames;
+import org.apache.sis.filter.internal.Visitor;
+import org.opengis.filter.BetweenComparisonOperator;
+import org.opengis.filter.Filter;
+import org.opengis.filter.Expression;
+import org.opengis.filter.ValueReference;
+import org.opengis.filter.ComparisonOperatorName;
+import org.opengis.filter.LikeOperator;
+import org.opengis.filter.LogicalOperator;
+import org.opengis.util.CodeList;
+
+/**
+ * Expression visitor that returns a list of all Feature attributs requiered 
by this expression.
+ *
+ * @author Johann Sorel (Geomatys)
+ */
+final class ListingPropertyVisitor extends Visitor<Object,Collection<String>> {
+
+    public static final ListingPropertyVisitor VISITOR = new 
ListingPropertyVisitor();
+
+    protected ListingPropertyVisitor() {
+        setLogicalHandlers((f, names) -> {
+            final LogicalOperator<Object> filter = (LogicalOperator<Object>) f;
+            for (Filter<Object> child : filter.getOperands()) {
+                visit(child, names);
+            }
+        });
+        
setFilterHandler(ComparisonOperatorName.valueOf(FunctionNames.PROPERTY_IS_BETWEEN),
 (f, names) -> {
+            final BetweenComparisonOperator<Object> filter = 
(BetweenComparisonOperator<Object>) f;
+            visit(filter.getExpression(),    names);
+            visit(filter.getLowerBoundary(), names);
+            visit(filter.getUpperBoundary(), names);
+        });
+        
setFilterHandler(ComparisonOperatorName.valueOf(FunctionNames.PROPERTY_IS_LIKE),
 (f, names) -> {
+            final LikeOperator<Object> filter = (LikeOperator<Object>) f;
+            visit(filter.getExpressions().get(0), names);
+        });
+        setExpressionHandler(FunctionNames.ValueReference, (e, names) -> {
+            final ValueReference<Object,?> expression = 
(ValueReference<Object,?>) e;
+            final String propName = expression.getXPath();
+            if (!propName.trim().isEmpty()) {
+                names.add(propName);
+            }
+        });
+    }
+
+    @Override
+    protected void typeNotFound(final CodeList<?> type, final Filter<Object> 
filter, final Collection<String> names) {
+        for (final Expression<? super Object, ?> f : filter.getExpressions()) {
+            visit(f, names);
+        }
+    }
+
+    @Override
+    protected void typeNotFound(final String type, final Expression<Object, ?> 
expression, final Collection<String> names) {
+        for (final Expression<? super Object, ?> p : 
expression.getParameters()) {
+            visit(p, names);
+        }
+    }
+}
diff --git 
a/incubator/src/org.apache.sis.storage.shapefile/main/org/apache/sis/storage/shapefile/ShapefileStore.java
 
b/incubator/src/org.apache.sis.storage.shapefile/main/org/apache/sis/storage/shapefile/ShapefileStore.java
index a95c5de705..52dafdd7de 100644
--- 
a/incubator/src/org.apache.sis.storage.shapefile/main/org/apache/sis/storage/shapefile/ShapefileStore.java
+++ 
b/incubator/src/org.apache.sis.storage.shapefile/main/org/apache/sis/storage/shapefile/ShapefileStore.java
@@ -26,8 +26,12 @@ import java.nio.charset.StandardCharsets;
 import java.nio.file.Files;
 import java.nio.file.Path;
 import java.nio.file.StandardOpenOption;
+import java.util.AbstractMap;
+import java.util.ArrayList;
+import java.util.HashSet;
 import java.util.Iterator;
 import java.util.List;
+import java.util.Map.Entry;
 import java.util.Optional;
 import java.util.Set;
 import java.util.Spliterator;
@@ -37,8 +41,12 @@ import java.util.concurrent.locks.ReentrantReadWriteLock;
 import java.util.function.Consumer;
 import java.util.function.Predicate;
 import java.util.function.UnaryOperator;
+import java.util.logging.Level;
+import java.util.logging.Logger;
 import java.util.stream.Stream;
 import java.util.stream.StreamSupport;
+
+import org.apache.sis.geometry.wrapper.Geometries;
 import org.opengis.geometry.Envelope;
 import org.opengis.metadata.Metadata;
 import org.opengis.parameter.ParameterValueGroup;
@@ -48,6 +56,11 @@ import org.opengis.util.GenericName;
 import org.apache.sis.feature.builder.AttributeRole;
 import org.apache.sis.feature.builder.AttributeTypeBuilder;
 import org.apache.sis.feature.builder.FeatureTypeBuilder;
+import org.apache.sis.feature.internal.AttributeConvention;
+import org.apache.sis.filter.DefaultFilterFactory;
+import org.apache.sis.filter.Optimization;
+import org.apache.sis.filter.internal.FunctionNames;
+import org.apache.sis.geometry.Envelopes;
 import org.apache.sis.io.stream.ChannelDataInput;
 import org.apache.sis.io.stream.ChannelDataOutput;
 import org.apache.sis.io.stream.IOUtilities;
@@ -78,7 +91,13 @@ import org.opengis.feature.Feature;
 import org.opengis.feature.FeatureType;
 import org.opengis.filter.Expression;
 import org.opengis.filter.Filter;
+import org.opengis.filter.Literal;
+import org.opengis.filter.LogicalOperator;
+import org.opengis.filter.LogicalOperatorName;
 import org.opengis.filter.SpatialOperatorName;
+import org.opengis.filter.ValueReference;
+import org.opengis.referencing.operation.TransformException;
+import org.opengis.util.CodeList;
 
 
 /**
@@ -95,7 +114,7 @@ public final class ShapefileStore extends DataStore 
implements FeatureSet {
     /**
      * Internal class to inherit AbstractFeatureSet.
      */
-    private final AsFeatureSet featureSetView = new AsFeatureSet(null, null);
+    private final AsFeatureSet featureSetView = new AsFeatureSet(null, true, 
null);
     private Charset charset;
 
     /**
@@ -158,14 +177,21 @@ public final class ShapefileStore extends DataStore 
implements FeatureSet {
         private final Rectangle2D.Double filter;
         private final Set<String> dbfProperties;
         private int[] dbfPropertiesIndex;
+        private final boolean readShp;
+        /**
+         * Name of the field used as identifier, may be null.
+         */
+        private String idField;
+        private CoordinateReferenceSystem crs;
         private FeatureType type;
 
         /**
          * @param filter optional shape filter, must be in data CRS
          * @param properties dbf properties to read, null for all properties
          */
-        private AsFeatureSet(Rectangle2D.Double filter, Set<String> 
properties) {
+        private AsFeatureSet(Rectangle2D.Double filter, boolean readShp, 
Set<String> properties) {
             super(null);
+            this.readShp = readShp;
             this.filter = filter;
             this.dbfProperties = properties;
         }
@@ -180,31 +206,32 @@ public final class ShapefileStore extends DataStore 
implements FeatureSet {
                 final FeatureTypeBuilder ftb = new FeatureTypeBuilder();
                 ftb.setName(files.baseName);
 
-                //read shp header to obtain geometry type
-                final Class geometryClass;
-                try (final ShapeReader reader = new 
ShapeReader(ShpFiles.openReadChannel(shpPath), filter)) {
-                    final ShapeHeader header = reader.getHeader();
-                    geometryClass = 
ShapeGeometryEncoder.getEncoder(header.shapeType).getValueClass();
-                } catch (IOException ex) {
-                    throw new DataStoreException("Failed to parse shape file 
header.", ex);
-                }
+                if (readShp) {
+                    //read shp header to obtain geometry type
+                    final Class geometryClass;
+                    try (final ShapeReader reader = new 
ShapeReader(ShpFiles.openReadChannel(shpPath), filter)) {
+                        final ShapeHeader header = reader.getHeader();
+                        geometryClass = 
ShapeGeometryEncoder.getEncoder(header.shapeType).getValueClass();
+                    } catch (IOException ex) {
+                        throw new DataStoreException("Failed to parse shape 
file header.", ex);
+                    }
 
-                //read prj file for projection
-                final Path prjFile = files.getPrj(false);
-                final CoordinateReferenceSystem crs;
-                if (prjFile != null) {
-                    try {
-                        crs = CRS.fromWKT(Files.readString(prjFile, 
StandardCharsets.UTF_8));
-                    } catch (IOException | FactoryException ex) {
-                        throw new DataStoreException("Failed to parse prj 
file.", ex);
+                    //read prj file for projection
+                    final Path prjFile = files.getPrj(false);
+                    if (prjFile != null) {
+                        try {
+                            crs = CRS.fromWKT(Files.readString(prjFile, 
StandardCharsets.UTF_8));
+                        } catch (IOException | FactoryException ex) {
+                            throw new DataStoreException("Failed to parse prj 
file.", ex);
+                        }
+                    } else {
+                        //shapefile often do not have a .prj, mostly those are 
in CRS:84.
+                        //we do not raise an error otherwise we would not be 
able to read a lot of data.
+                        crs = CommonCRS.WGS84.normalizedGeographic();
                     }
-                } else {
-                    //shapefile often do not have a .prj, mostly those are in 
CRS:84.
-                    //we do not raise an error otherwise we would not be able 
to read a lot of data.
-                    crs = CommonCRS.WGS84.normalizedGeographic();
-                }
 
-                
ftb.addAttribute(geometryClass).setName(GEOMETRY_NAME).setCRS(crs).addRole(AttributeRole.DEFAULT_GEOMETRY);
+                    
ftb.addAttribute(geometryClass).setName(GEOMETRY_NAME).setCRS(crs).addRole(AttributeRole.DEFAULT_GEOMETRY);
+                }
 
                 //read cpg for dbf file charset
                 final Path cpgFile = files.getCpg(false);
@@ -243,6 +270,7 @@ public final class ShapefileStore extends DataStore 
implements FeatureSet {
                             final AttributeTypeBuilder atb = 
ftb.addAttribute(field.getEncoder().getValueClass()).setName(field.fieldName);
                             //no official but 'id' field is common
                             if (!hasId && 
"id".equalsIgnoreCase(field.fieldName) || 
"identifier".equalsIgnoreCase(field.fieldName)) {
+                                idField = field.fieldName;
                                 
atb.addRole(AttributeRole.IDENTIFIER_COMPONENT);
                                 hasId = true;
                             }
@@ -265,41 +293,84 @@ public final class ShapefileStore extends DataStore 
implements FeatureSet {
             final ShapeReader shpreader;
             final DBFReader dbfreader;
             try {
-                shpreader = new 
ShapeReader(ShpFiles.openReadChannel(files.shpFile), filter);
-                dbfreader = new 
DBFReader(ShpFiles.openReadChannel(files.getDbf(false)), charset, 
dbfPropertiesIndex);
+                shpreader = readShp ? new 
ShapeReader(ShpFiles.openReadChannel(files.shpFile), filter) : null;
+                dbfreader = (dbfPropertiesIndex.length > 0) ? new 
DBFReader(ShpFiles.openReadChannel(files.getDbf(false)), charset, 
dbfPropertiesIndex) : null;
             } catch (IOException ex) {
                 throw new DataStoreException("Faild to open shp and dbf 
files.", ex);
             }
-            final DBFHeader header = dbfreader.getHeader();
 
-            final Spliterator spliterator = new 
Spliterators.AbstractSpliterator(Long.MAX_VALUE, Spliterator.ORDERED) {
-                @Override
-                public boolean tryAdvance(Consumer action) {
-                    try {
-                        final ShapeRecord shpRecord = shpreader.next();
-                        if (shpRecord == null) return false;
-                        //move dbf to record offset, some shp record might 
have been skipped because of filter
-                        dbfreader.moveToOffset(header.headerSize + 
(shpRecord.recordNumber-1) * header.recordSize);
-                        final DBFRecord dbfRecord = dbfreader.next();
-                        final Feature next = type.newInstance();
-                        next.setPropertyValue(GEOMETRY_NAME, 
shpRecord.geometry);
-                        for (int i = 0; i < dbfPropertiesIndex.length; i++) {
-                            next.setPropertyValue(header.fields[i].fieldName, 
dbfRecord.fields[i]);
+            final Spliterator spliterator;
+            if (readShp && dbfPropertiesIndex.length > 0) {
+                //read both shp and dbf
+                final DBFHeader header = dbfreader.getHeader();
+
+                spliterator = new 
Spliterators.AbstractSpliterator(Long.MAX_VALUE, Spliterator.ORDERED) {
+                    @Override
+                    public boolean tryAdvance(Consumer action) {
+                        try {
+                            final ShapeRecord shpRecord = shpreader.next();
+                            if (shpRecord == null) return false;
+                            //move dbf to record offset, some shp record might 
have been skipped because of filter
+                            dbfreader.moveToOffset(header.headerSize + 
(shpRecord.recordNumber-1) * header.recordSize);
+                            final DBFRecord dbfRecord = dbfreader.next();
+                            final Feature next = type.newInstance();
+                            next.setPropertyValue(GEOMETRY_NAME, 
shpRecord.geometry);
+                            for (int i = 0; i < dbfPropertiesIndex.length; 
i++) {
+                                
next.setPropertyValue(header.fields[dbfPropertiesIndex[i]].fieldName, 
dbfRecord.fields[i]);
+                            }
+                            action.accept(next);
+                            return true;
+                        } catch (IOException ex) {
+                            throw new BackingStoreException(ex.getMessage(), 
ex);
                         }
-                        action.accept(next);
-                        return true;
-                    } catch (IOException ex) {
-                        throw new BackingStoreException(ex.getMessage(), ex);
                     }
-                }
-            };
+                };
+            } else if (readShp) {
+                //read only the shp
+                spliterator = new 
Spliterators.AbstractSpliterator(Long.MAX_VALUE, Spliterator.ORDERED) {
+                    @Override
+                    public boolean tryAdvance(Consumer action) {
+                        try {
+                            final ShapeRecord shpRecord = shpreader.next();
+                            if (shpRecord == null) return false;
+                            final Feature next = type.newInstance();
+                            next.setPropertyValue(GEOMETRY_NAME, 
shpRecord.geometry);
+                            action.accept(next);
+                            return true;
+                        } catch (IOException ex) {
+                            throw new BackingStoreException(ex.getMessage(), 
ex);
+                        }
+                    }
+                };
+            } else {
+                //read only dbf
+                final DBFHeader header = dbfreader.getHeader();
+                spliterator = new 
Spliterators.AbstractSpliterator(Long.MAX_VALUE, Spliterator.ORDERED) {
+                    @Override
+                    public boolean tryAdvance(Consumer action) {
+                        try {
+                            final DBFRecord dbfRecord = dbfreader.next();
+                            if (dbfRecord == null) return false;
+                            final Feature next = type.newInstance();
+                            for (int i = 0; i < dbfPropertiesIndex.length; 
i++) {
+                                
next.setPropertyValue(header.fields[dbfPropertiesIndex[i]].fieldName, 
dbfRecord.fields[i]);
+                            }
+                            action.accept(next);
+                            return true;
+                        } catch (IOException ex) {
+                            throw new BackingStoreException(ex.getMessage(), 
ex);
+                        }
+                    }
+                };
+            }
+
             final Stream<Feature> stream = StreamSupport.stream(spliterator, 
false);
             return stream.onClose(new Runnable() {
                 @Override
                 public void run() {
                     try {
-                        shpreader.close();
-                        dbfreader.close();
+                        if (shpreader != null) shpreader.close();
+                        if (dbfreader != null) dbfreader.close();
                     } catch (IOException ex) {
                         throw new BackingStoreException(ex.getMessage(), ex);
                     }
@@ -311,9 +382,96 @@ public final class ShapefileStore extends DataStore 
implements FeatureSet {
         @Override
         public FeatureSet subset(Query query) throws 
UnsupportedQueryException, DataStoreException {
             //try to optimise the query for common cases
+            opti:
             if (query instanceof FeatureQuery) {
                 final FeatureQuery fq = (FeatureQuery) query;
-                //todo
+                FeatureQuery.NamedExpression[] projection = fq.getProjection();
+                Filter<? super Feature> selection = fq.getSelection();
+
+                if (selection == null && projection == null) {
+                    //no optimisation
+                    break opti;
+                }
+
+                //force loading
+                final FeatureType type = getType();
+
+                //extract bbox
+                Envelope bbox = null;
+                if (selection != null) {
+                    //run optimizations
+                    final Optimization optimization = new Optimization();
+                    optimization.setFeatureType(type);
+                    selection = optimization.apply(selection);
+                    final Entry<Envelope, Filter> split = 
extractBbox(selection);
+                    bbox = split.getKey();
+                    selection = split.getValue();
+                }
+
+                //extract field names
+                boolean simpleSelection = true; //true if there are no alias 
and all expressions are ValueReference
+                Set<String> properties = null;
+                if (projection != null) {
+                    properties = new HashSet<>();
+                    if (selection!=null) 
ListingPropertyVisitor.VISITOR.visit((Filter) selection, properties);
+                    for (FeatureQuery.NamedExpression ne : projection) {
+                        ListingPropertyVisitor.VISITOR.visit((Expression) 
ne.expression, properties);
+                        simpleSelection &= (ne.alias == null);
+                        simpleSelection &= 
(ne.expression.getFunctionName().tip().toString().equals(FunctionNames.ValueReference));
+                    }
+
+                    //if link fields are referenced, add target fields
+                    if (properties.contains(AttributeConvention.IDENTIFIER)) 
simpleSelection &= !properties.add(idField);
+                    if (properties.contains(AttributeConvention.GEOMETRY)) 
simpleSelection &= !properties.add(GEOMETRY_NAME);
+                    if (properties.contains(AttributeConvention.ENVELOPE)) 
simpleSelection &= !properties.add(GEOMETRY_NAME);
+                }
+
+                final boolean readShp = projection == null || 
properties.contains(GEOMETRY_NAME);
+                Rectangle2D.Double area = null;
+                if (bbox != null) {
+                    try {
+                        bbox = Envelopes.transform(bbox, crs);
+                    } catch (TransformException ex) {
+                        throw new DataStoreException("Failed to transform bbox 
filter", ex);
+                    }
+                    area = new Rectangle2D.Double(bbox.getMinimum(0), 
bbox.getMinimum(1), bbox.getSpan(0), bbox.getSpan(1));
+
+                    //combine this area with the one we already have since 
this is a subset
+                    if (filter != null) {
+                        area = (Rectangle2D.Double) 
area.createIntersection(filter);
+                    }
+                }
+
+                FeatureSet fs = new AsFeatureSet(area, readShp, properties);
+                //see if there are elements we could not handle
+                final FeatureQuery subQuery = new FeatureQuery();
+                boolean needSubProcessing = false;
+                if (fq.getLimit().isPresent()){
+                    needSubProcessing = true;
+                    subQuery.setLimit(fq.getLimit().getAsLong());
+                }
+                if (fq.getLinearResolution() != null) {
+                    needSubProcessing = true;
+                    subQuery.setLinearResolution(fq.getLinearResolution());
+                }
+                if (fq.getOffset() != 0) {
+                    needSubProcessing = true;
+                    subQuery.setOffset(fq.getOffset());
+                }
+                if (fq.getSortBy() != null) {
+                    needSubProcessing = true;
+                    subQuery.setSortBy(fq.getSortBy());
+                }
+                if (selection != null) {
+                    needSubProcessing = true;
+                    subQuery.setSelection(selection);
+                }
+                if (!simpleSelection) {
+                    needSubProcessing = true;
+                    subQuery.setProjection(projection);
+                }
+
+                return needSubProcessing ? fs.subset(subQuery) : fs;
             }
 
             return super.subset(query);
@@ -429,4 +587,96 @@ public final class ShapefileStore extends DataStore 
implements FeatureSet {
         }
     }
 
+
+    /**
+     * Will only split bbox with direct value reference to the geometry.
+     *
+     * @param filter to split, not null
+     * @return entry with key is a BBox filter and value what remains of the 
filter.
+     *        each filter can be null but not both.
+     */
+    private static Entry<Envelope,Filter> extractBbox(Filter<?> filter) {
+
+        final CodeList operatorType = filter.getOperatorType();
+
+        if (SpatialOperatorName.BBOX.equals(operatorType)) {
+            Envelope env = isDirectBbox(filter);
+            if (env != null) {
+                return new AbstractMap.SimpleImmutableEntry<>(env, null);
+            } else {
+                return new AbstractMap.SimpleImmutableEntry<>(null, filter);
+            }
+
+        } else if (LogicalOperatorName.AND.equals(operatorType)) {
+
+            boolean rebuildAnd = false;
+            List<Filter<?>> lst = (List<Filter<?>>) 
((LogicalOperator<?>)filter).getOperands();
+            Envelope bbox = null;
+            for (int i = 0, n = lst.size(); i < n; i++) {
+                final Filter<?> f = lst.get(i);
+                final Entry<Envelope, Filter> split = extractBbox(f);
+                Envelope cdtBbox = split.getKey();
+                Filter cdtOther = split.getValue();
+                if (cdtBbox != null) {
+                    if (bbox == null) {
+                        bbox = cdtBbox;
+                    } else {
+                        throw new RuntimeException("Combine bbox");
+                    }
+
+                    //see if we need to rebuild the AND filter
+                    if (cdtOther != f) {
+                        if (!rebuildAnd) {
+                            rebuildAnd = true;
+                            lst = new ArrayList<>(lst);
+                        }
+                        //replace in list
+                        if (cdtOther != null) {
+                            lst.set(i, cdtOther);
+                        } else {
+                            lst.remove(i);
+                            i--;
+                        }
+                    }
+                }
+            }
+
+            if (rebuildAnd) {
+                if (lst.isEmpty()) {
+                    filter = null;
+                } else {
+                    filter = DefaultFilterFactory.forFeatures().and((List)lst);
+                }
+            }
+
+            return new AbstractMap.SimpleImmutableEntry<>(bbox, filter);
+        } else {
+            //can do nothing
+            return new AbstractMap.SimpleImmutableEntry<>(null, filter);
+        }
+    }
+
+    /**
+     * Returns envelope if the other expression is a direct value reference.
+     * @param bbox
+     * @return filter envelope
+     */
+    private static Envelope isDirectBbox(Filter<?> bbox) {
+        Envelope env = null;
+        for (Expression exp : bbox.getExpressions()) {
+            if (exp instanceof ValueReference) {
+                final ValueReference<Object,?> expression = 
(ValueReference<Object,?>) exp;
+                final String propName = expression.getXPath();
+                if ( !(GEOMETRY_NAME.equals(propName) || 
AttributeConvention.GEOMETRY.equals(propName))) {
+                    return null;
+                }
+            } else if (exp instanceof Literal) {
+                Object value = ((Literal) exp).getValue();
+                env = Geometries.wrap(value).get().getEnvelope();
+            }
+        }
+        return env;
+    }
+
+
 }
diff --git 
a/incubator/src/org.apache.sis.storage.shapefile/test/org/apache/sis/storage/shapefile/ShapefileStoreTest.java
 
b/incubator/src/org.apache.sis.storage.shapefile/test/org/apache/sis/storage/shapefile/ShapefileStoreTest.java
index 1a6ba9d6d0..ab490e7cfd 100644
--- 
a/incubator/src/org.apache.sis.storage.shapefile/test/org/apache/sis/storage/shapefile/ShapefileStoreTest.java
+++ 
b/incubator/src/org.apache.sis.storage.shapefile/test/org/apache/sis/storage/shapefile/ShapefileStoreTest.java
@@ -22,8 +22,13 @@ import java.nio.file.Paths;
 import java.time.LocalDate;
 import java.util.Iterator;
 import java.util.stream.Stream;
+import org.apache.sis.filter.DefaultFilterFactory;
+import org.apache.sis.geometry.GeneralEnvelope;
+import org.apache.sis.referencing.CommonCRS;
 import static org.junit.jupiter.api.Assertions.*;
 import org.apache.sis.storage.DataStoreException;
+import org.apache.sis.storage.FeatureQuery;
+import org.apache.sis.storage.FeatureSet;
 import org.junit.Ignore;
 import org.junit.Test;
 import org.locationtech.jts.geom.Point;
@@ -32,6 +37,7 @@ import org.locationtech.jts.geom.Point;
 import org.opengis.feature.AttributeType;
 import org.opengis.feature.Feature;
 import org.opengis.feature.FeatureType;
+import org.opengis.filter.FilterFactory;
 
 
 /**
@@ -90,15 +96,26 @@ public class ShapefileStoreTest {
     }
 
     /**
-     * TODO not implemented yet.
+     * Test optimized envelope filter.
      */
-    @Ignore
     @Test
     public void testEnvelopeFilter() throws URISyntaxException, 
DataStoreException {
         final URL url = 
ShapefileStoreTest.class.getResource("/org/apache/sis/storage/shapefile/point.shp");
         final ShapefileStore store = new 
ShapefileStore(Paths.get(url.toURI()));
 
-        try (Stream<Feature> stream = store.features(false)) {
+        final FilterFactory<Feature, Object, Object> ff = 
DefaultFilterFactory.forFeatures();
+
+        final GeneralEnvelope env = new 
GeneralEnvelope(CommonCRS.WGS84.normalizedGeographic());
+        env.setRange(0, 2, 3);
+        env.setRange(1, 42, 43);
+
+        final FeatureQuery query = new FeatureQuery();
+        query.setSelection(ff.bbox(ff.property("geometry"), env));
+        FeatureSet featureset = store.subset(query);
+        //ensure we obtained an optimized version
+        
assertEquals("org.apache.sis.storage.shapefile.ShapefileStore$AsFeatureSet", 
featureset.getClass().getName());
+
+        try (Stream<Feature> stream = featureset.features(false)) {
             Iterator<Feature> iterator = stream.iterator();
             assertTrue(iterator.hasNext());
             Feature feature = iterator.next();
@@ -113,4 +130,34 @@ public class ShapefileStoreTest {
         }
     }
 
+    /**
+     * Test optimized field selection.
+     */
+    @Test
+    public void testFieldFilter() throws URISyntaxException, 
DataStoreException {
+        final URL url = 
ShapefileStoreTest.class.getResource("/org/apache/sis/storage/shapefile/point.shp");
+        final ShapefileStore store = new 
ShapefileStore(Paths.get(url.toURI()));
+
+
+        final FeatureQuery query = new FeatureQuery();
+        query.setProjection("text", "float");
+        FeatureSet featureset = store.subset(query);
+        //ensure we obtained an optimized version
+        
assertEquals("org.apache.sis.storage.shapefile.ShapefileStore$AsFeatureSet", 
featureset.getClass().getName());
+
+        try (Stream<Feature> stream = featureset.features(false)) {
+            Iterator<Feature> iterator = stream.iterator();
+            assertTrue(iterator.hasNext());
+            Feature feature1 = iterator.next();
+            assertEquals("text1", feature1.getPropertyValue("text"));
+            assertEquals(20.0, feature1.getPropertyValue("float"));
+
+            assertTrue(iterator.hasNext());
+            Feature feature2 = iterator.next();
+            assertEquals("text2", feature2.getPropertyValue("text"));
+            assertEquals(60.0, feature2.getPropertyValue("float"));
+
+            assertFalse(iterator.hasNext());
+        }
+    }
 }

Reply via email to