sis-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From ama...@apache.org
Subject [sis] 36/45: feat(SQLStore): improve query feature set to delegate count and subqueries to target database.
Date Tue, 12 Nov 2019 16:45:03 GMT
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 1c8df66ca3a0b0045b73bc5c0095cd795f8bc64e
Author: Alexis Manin <amanin@apache.org>
AuthorDate: Wed Oct 16 18:14:29 2019 +0200

    feat(SQLStore): improve query feature set to delegate count and subqueries to target database.
---
 .../apache/sis/internal/sql/feature/Analyzer.java  |  3 +-
 .../apache/sis/internal/sql/feature/Features.java  | 16 ++++--
 .../sis/internal/sql/feature/QueryFeatureSet.java  | 65 ++++++++++++++++++----
 .../sis/internal/sql/feature/SQLQueryAdapter.java  | 31 +++++++----
 .../org/apache/sis/internal/sql/feature/Table.java |  4 +-
 .../sis/internal/sql/feature/TableSubset.java      |  5 ++
 .../sis/internal/sql/feature/package-info.java     | 10 ++++
 .../org/apache/sis/storage/sql/SQLStoreTest.java   | 27 ++++++++-
 8 files changed, 126 insertions(+), 35 deletions(-)

diff --git a/storage/sis-sqlstore/src/main/java/org/apache/sis/internal/sql/feature/Analyzer.java
b/storage/sis-sqlstore/src/main/java/org/apache/sis/internal/sql/feature/Analyzer.java
index d947ad2..11c9e35 100644
--- a/storage/sis-sqlstore/src/main/java/org/apache/sis/internal/sql/feature/Analyzer.java
+++ b/storage/sis-sqlstore/src/main/java/org/apache/sis/internal/sql/feature/Analyzer.java
@@ -408,12 +408,11 @@ final class Analyzer {
 
     private void addImports(SQLTypeSpecification spec, FeatureTypeBuilder target) throws
SQLException {
         final List<Relation> imports = spec.getImports();
-        // TODO: add an abstraction here, so we can specify source table when origin is one.
         for (Relation r : imports) {
             final GenericName foreignTypeName = r.getName(Analyzer.this);
             final Table foreignTable;
             try {
-                foreignTable = table(r, foreignTypeName, null);
+                foreignTable = table(r, foreignTypeName, spec instanceof TableMetadata ?
((TableMetadata) spec).id : null);
             } catch (DataStoreException e) {
                 throw new BackingStoreException(e);
             }
diff --git a/storage/sis-sqlstore/src/main/java/org/apache/sis/internal/sql/feature/Features.java
b/storage/sis-sqlstore/src/main/java/org/apache/sis/internal/sql/feature/Features.java
index 5ce2195..42522b1 100644
--- a/storage/sis-sqlstore/src/main/java/org/apache/sis/internal/sql/feature/Features.java
+++ b/storage/sis-sqlstore/src/main/java/org/apache/sis/internal/sql/feature/Features.java
@@ -667,12 +667,7 @@ final class Features implements Spliterator<Feature> {
             sql.append(" FROM ").appendIdentifier(source.parent.name.catalog, source.parent.name.schema,
source.parent.name.table);
 
             appendWhere(sql, where);
-            if (!count && sort != null && sort.length > 0) {
-                sql.append(" ORDER BY ");
-                append(sql, sort[0]);
-                for (int i = 1 ; i < sort.length ; i++)
-                    append(sql.append(", "), sort[i]);
-            }
+            if (!count) appendOrderBy(sql, sort);
 
             addOffsetLimit(sql, source.offset, source.limit);
 
@@ -691,6 +686,15 @@ final class Features implements Spliterator<Feature> {
         }
     }
 
+    static void appendOrderBy(SQLBuilder sql, SortBy[] sort) {
+        if (sort != null && sort.length > 0) {
+            sql.append(" ORDER BY ");
+            append(sql, sort[0]);
+            for (int i = 1 ; i < sort.length ; i++)
+                append(sql.append(", "), sort[i]);
+        }
+    }
+
     private static void append(SQLBuilder target, SortBy toAppend) {
         target.appendIdentifier(toAppend.getPropertyName().getPropertyName()).append(" ");
         if (toAppend.getSortOrder() != null) target.append(toAppend.getSortOrder().toSQL());
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 6c16b6a..706130f 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
@@ -6,6 +6,7 @@ import java.sql.ResultSet;
 import java.sql.SQLException;
 import java.util.List;
 import java.util.Spliterator;
+import java.util.StringJoiner;
 import java.util.function.Consumer;
 import java.util.regex.Matcher;
 import java.util.regex.Pattern;
@@ -15,10 +16,15 @@ import javax.sql.DataSource;
 
 import org.opengis.feature.Feature;
 import org.opengis.feature.FeatureType;
+import org.opengis.filter.sort.SortBy;
 
 import org.apache.sis.internal.metadata.sql.SQLBuilder;
 import org.apache.sis.internal.storage.AbstractFeatureSet;
+import org.apache.sis.internal.storage.query.SimpleQuery;
 import org.apache.sis.storage.DataStoreException;
+import org.apache.sis.storage.FeatureSet;
+import org.apache.sis.storage.Query;
+import org.apache.sis.storage.UnsupportedQueryException;
 import org.apache.sis.util.collection.BackingStoreException;
 import org.apache.sis.util.logging.WarningListeners;
 
@@ -30,6 +36,8 @@ import org.apache.sis.util.logging.WarningListeners;
  *
  * Note that this component models query result as close as possible, so built data type
will be simple feature type (no
  * association).
+ *
+ * TODO: move query analysis in a dedicated class.
  */
 public class QueryFeatureSet extends AbstractFeatureSet {
 
@@ -127,10 +135,10 @@ public class QueryFeatureSet extends AbstractFeatureSet {
      *                 can use {@link #QueryFeatureSet(SQLBuilder, DataSource, Connection)
another constructor}.
      */
     QueryFeatureSet(SQLBuilder queryBuilder, Analyzer analyzer, DataSource source, Connection
conn) throws SQLException {
-        this(queryBuilder, createAdapter(queryBuilder, analyzer, conn), analyzer.listeners,
source, conn);
+        this(queryBuilder, createAdapter(queryBuilder, analyzer, conn), analyzer.listeners,
source);
     }
 
-    QueryFeatureSet(SQLBuilder queryBuilder, FeatureAdapter adapter, WarningListeners listeners,
DataSource source, Connection conn) throws SQLException {
+    QueryFeatureSet(SQLBuilder queryBuilder, FeatureAdapter adapter, WarningListeners listeners,
DataSource source) {
         super(listeners);
         this.source = source;
         this.adapter = adapter;
@@ -168,6 +176,26 @@ public class QueryFeatureSet extends AbstractFeatureSet {
         this.queryBuilder.append(sql);
     }
 
+    @Override
+    public FeatureType getType() {
+        return adapter.type;
+    }
+
+    @Override
+    public Stream<Feature> features(boolean parallel) {
+        return new StreamSQL(new QueryAdapter(queryBuilder, parallel), source, parallel);
+    }
+
+    @Override
+    public FeatureSet subset(Query query) throws UnsupportedQueryException, DataStoreException
{
+        if (query instanceof SimpleQuery) {
+            final org.apache.sis.internal.storage.SubsetAdapter subsetAdapter = new org.apache.sis.internal.storage.SubsetAdapter(fs
-> new SubsetAdapter());
+            return subsetAdapter.subset(this, (SimpleQuery) query);
+        }
+
+        return super.subset(query);
+    }
+
     /**
      * Acquire a connection over parent database, forcing a few parameters to ensure optimal
read performance and
      * limiting user rights :
@@ -196,14 +224,27 @@ public class QueryFeatureSet extends AbstractFeatureSet {
         return c;
     }
 
-    @Override
-    public FeatureType getType() {
-        return adapter.type;
+    class SubsetAdapter extends SQLQueryAdapter {
+
+        @Override
+        protected FeatureSet create(CharSequence where, SortBy[] sorting, ColumnRef[] columns)
{
+            // TODO: use columns.
+            final SQLBuilder newQuery = amendQuery(where, sorting);
+            return new QueryFeatureSet(newQuery, adapter, null, source);
+        }
     }
 
-    @Override
-    public Stream<Feature> features(boolean parallel) {
-        return new StreamSQL(new QueryAdapter(queryBuilder, parallel), source, parallel);
+    private SQLBuilder amendQuery(CharSequence where, SortBy[] sorting) {
+        // As we do not know user query complexity, what we'll do is make a query wrapper
to ensure we won't break the
+        // original query. Note that it will surely be less performant, though.
+        final SQLBuilder newBuilder = new SQLBuilder(queryBuilder);
+        newBuilder.append("SELECT * FROM (")
+                .append(queryBuilder.toString())
+                .append(')')
+                .append(" AS ORIGIN_QUERY");
+        if (where != null && where.length() > 0) newBuilder.append(" WHERE ").append(where.toString());
+        Features.appendOrderBy(newBuilder, sorting);
+        return newBuilder.append(";");
     }
 
     private final class QueryAdapter implements StreamSQL.QueryBuilder {
@@ -280,7 +321,7 @@ public class QueryFeatureSet extends AbstractFeatureSet {
         private final boolean parallel;
 
         private PreparedQueryConnector(String sql, long additionalOffset, long additionalLimit,
boolean parallel) {
-            this.sql = sql;
+            this.sql = sql.replaceFirst(";\\s*$", "");
             this.additionalOffset = additionalOffset;
             this.additionalLimit = additionalLimit;
             this.parallel = parallel;
@@ -314,8 +355,7 @@ public class QueryFeatureSet extends AbstractFeatureSet {
         @Override
         public String estimateStatement(boolean count) {
             if (count) {
-                // We should check if user query is already a count operation, in which case
we would return "select 1"
-                throw new UnsupportedOperationException("Not supported yet"); // "Alexis
Manin (Geomatys)" on 24/09/2019
+                return "SELECT COUNT(*) FROM ("+sql+") AS count_all";
             } else {
                 return sql;
             }
@@ -350,7 +390,7 @@ public class QueryFeatureSet extends AbstractFeatureSet {
 
         @Override
         public int characteristics() {
-            // TODO: determine if it's order by analysing user query. SIZED is not possible,
as limit is an upper threshold.
+            // TODO: determine if it's ordered by analysing user query. SIZED is not possible,
as limit is an upper threshold.
             return Spliterator.IMMUTABLE | Spliterator.NONNULL | (distinct ? Spliterator.DISTINCT
: 0);
         }
     }
@@ -400,6 +440,7 @@ public class QueryFeatureSet extends AbstractFeatureSet {
 
         int idx;
         List<Feature> chunk;
+
         /**
          * According to {@link Spliterator#trySplit()} documentation, the original size estimation
must be reduced after
          * split to remain consistent.
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 e471567..94c2d11 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
@@ -11,19 +11,13 @@ import org.apache.sis.internal.storage.SubsetAdapter;
 import org.apache.sis.internal.storage.query.SimpleQuery;
 import org.apache.sis.storage.FeatureSet;
 
-public class SQLQueryAdapter implements SubsetAdapter.AdapterBuilder {
-
-    final Table parent;
+abstract class SQLQueryAdapter implements SubsetAdapter.AdapterBuilder {
 
     private ColumnRef[] columns;
     private SortBy[] sorting;
 
     private CharSequence where;
 
-    public SQLQueryAdapter(Table parent) {
-        this.parent = parent;
-    }
-
     /**
      * No-op implementation. SQL optimisation is dynamically applied through {@link StreamSQL}.
      * @param offset The offset to handle.
@@ -45,7 +39,7 @@ public class SQLQueryAdapter implements SubsetAdapter.AdapterBuilder {
     }
 
     @Override
-    public Filter filter(Filter filter) {
+    public final Filter filter(Filter filter) {
         try {
             final Object result = filter.accept(new ANSIInterpreter(), null);
             if (ANSIInterpreter.isNonEmptyText(result)) {
@@ -78,14 +72,31 @@ public class SQLQueryAdapter implements SubsetAdapter.AdapterBuilder {
     }
 
     @Override
-    public Optional<FeatureSet> build() {
+    public final Optional<FeatureSet> build() {
         if (isNoOp()) return Optional.empty();
-        return Optional.of(new TableSubset(parent, sorting, where));
+        return Optional.of(create(where, sorting, columns));
     }
 
+    protected abstract FeatureSet create(final CharSequence where, final SortBy[] sorting,
final ColumnRef[] columns);
+
     private boolean isNoOp() {
         return (sorting == null || sorting.length < 1)
                 && (columns == null || columns.length < 1)
                 && (where == null || where.length() < 1);
     }
+
+    static class Table extends SQLQueryAdapter {
+        final org.apache.sis.internal.sql.feature.Table parent;
+        public Table(org.apache.sis.internal.sql.feature.Table parent) {
+            this.parent = parent;
+        }
+
+        @Override
+        protected FeatureSet create(CharSequence where, SortBy[] sorting, ColumnRef[] columns)
{
+            // TODO: column information is lost for now. What should be done is factorize/sanitize
feature set
+            // implementations from this package to better handle SQL filtering.
+            return new TableSubset(parent, sorting, where);
+        }
+    }
+
 }
diff --git a/storage/sis-sqlstore/src/main/java/org/apache/sis/internal/sql/feature/Table.java
b/storage/sis-sqlstore/src/main/java/org/apache/sis/internal/sql/feature/Table.java
index b60ec46..bd35fbb 100644
--- a/storage/sis-sqlstore/src/main/java/org/apache/sis/internal/sql/feature/Table.java
+++ b/storage/sis-sqlstore/src/main/java/org/apache/sis/internal/sql/feature/Table.java
@@ -208,7 +208,7 @@ final class Table extends AbstractFeatureSet {
     @Override
     public FeatureSet subset(Query query) throws UnsupportedQueryException, DataStoreException
{
         if (query instanceof SimpleQuery) {
-            final SubsetAdapter subsetAdapter = new SubsetAdapter(fs -> new SQLQueryAdapter(this));
+            final SubsetAdapter subsetAdapter = new SubsetAdapter(fs -> new SQLQueryAdapter.Table(this));
             return subsetAdapter.subset(this, (SimpleQuery) query);
         }
 
@@ -230,7 +230,7 @@ final class Table extends AbstractFeatureSet {
      *
      * @param  tables  all tables created.
      */
-    final void setDeferredSearchTables(final Analyzer analyzer, final Map<GenericName,Table>
tables) throws DataStoreException {
+    final void setDeferredSearchTables(final Analyzer analyzer, final Map<GenericName,
Table> tables) throws DataStoreException {
         for (final Relation.Direction direction : Relation.Direction.values()) {
             final Relation[] relations;
             switch (direction) {
diff --git a/storage/sis-sqlstore/src/main/java/org/apache/sis/internal/sql/feature/TableSubset.java
b/storage/sis-sqlstore/src/main/java/org/apache/sis/internal/sql/feature/TableSubset.java
index dc9e427..52da6a5 100644
--- a/storage/sis-sqlstore/src/main/java/org/apache/sis/internal/sql/feature/TableSubset.java
+++ b/storage/sis-sqlstore/src/main/java/org/apache/sis/internal/sql/feature/TableSubset.java
@@ -12,9 +12,14 @@ import org.opengis.util.GenericName;
 
 import org.apache.sis.storage.DataStoreException;
 import org.apache.sis.storage.FeatureSet;
+import org.apache.sis.storage.Query;
 import org.apache.sis.storage.event.ChangeEvent;
 import org.apache.sis.storage.event.ChangeListener;
 
+/**
+ * A {@link Table} feature set on which a query has been applied.
+ * TODO: Override {@link #subset(Query)} method to allow stacking of SQL filter and sorting.
+ */
 public class TableSubset implements FeatureSet {
 
     final Table parent;
diff --git a/storage/sis-sqlstore/src/main/java/org/apache/sis/internal/sql/feature/package-info.java
b/storage/sis-sqlstore/src/main/java/org/apache/sis/internal/sql/feature/package-info.java
index 19c3151..e531ce5 100644
--- a/storage/sis-sqlstore/src/main/java/org/apache/sis/internal/sql/feature/package-info.java
+++ b/storage/sis-sqlstore/src/main/java/org/apache/sis/internal/sql/feature/package-info.java
@@ -29,6 +29,16 @@
  * It relies on internal {@link org.apache.sis.internal.sql.feature.SQLTypeSpecification}
API to fetch SQL schema
  * information, and build {@link org.apache.sis.internal.sql.feature.FeatureAdapter an adapter
to feature model from it}.
  *
+ * This package provides two main {@link org.apache.sis.storage.FeatureSet feature set} implementations:
+ * <ul>
+ *     <li>{@link org.apache.sis.internal.sql.feature.QueryFeatureSet}: execute a prepared
SQL query, then interpret its result as Simple Feature collection.</li>
+ *     <li>{@link org.apache.sis.internal.sql.feature.Table}: Analysis of SQL Table
to provide a complex feature type modeling associations.</li>
+ * </ul>
+ *
+ * TODO: a lot of code could be factorized to reduce splitting of code base for both use
cases above. Notably, all
+ * association management is done specifically in table implementation, but should be deported
in {@link org.apache.sis.internal.sql.feature.FeatureAdapter}.
+ * With that, we could reduce feature set implementations to only QueryFeatureSet, and delegating
model analysis upstream.
+ *
  * @author  Johann Sorel (Geomatys)
  * @author  Martin Desruisseaux (Geomatys)
  * @author  Alexis Manin (Geomatys)
diff --git a/storage/sis-sqlstore/src/test/java/org/apache/sis/storage/sql/SQLStoreTest.java
b/storage/sis-sqlstore/src/test/java/org/apache/sis/storage/sql/SQLStoreTest.java
index 5142fa1..d1bcd97 100644
--- a/storage/sis-sqlstore/src/test/java/org/apache/sis/storage/sql/SQLStoreTest.java
+++ b/storage/sis-sqlstore/src/test/java/org/apache/sis/storage/sql/SQLStoreTest.java
@@ -20,6 +20,7 @@ import java.sql.Connection;
 import java.sql.SQLException;
 import java.util.Arrays;
 import java.util.Collection;
+import java.util.Collections;
 import java.util.HashMap;
 import java.util.HashSet;
 import java.util.Iterator;
@@ -157,8 +158,8 @@ public final strictfp class SQLStoreTest extends TestCase {
                         new Object[] {null,             String.class, String.class,  String.class,
  Integer.class, "Countries", "Parks"});
 
                 verifyFeatureType(((FeatureSet) store.findResource("Countries")).getType(),
-                        new String[] {"sis:identifier", "code",       "native_name",  "sis:Cities"},
-                        new Object[] {null,             String.class, String.class, "Cities"});
+                        new String[] {"sis:identifier", "code",       "native_name"},
+                        new Object[] {null,             String.class, String.class});
 
                 verifyFeatureType(((FeatureSet) store.findResource("Parks")).getType(),
                         new String[] {"sis:identifier", "country", "city",       "native_name",
"english_name", "sis:FK_City"},
@@ -234,6 +235,26 @@ public final strictfp class SQLStoreTest extends TestCase {
         verifyFetchCityTableAsQuery(source);
         verifyLimitOffsetAndColumnSelectionFromQuery(source);
         verifyDistinctQuery(source);
+        verifyNestedSQLQuery(source);
+    }
+
+    private void verifyNestedSQLQuery(DataSource source) throws Exception {
+        final QueryFeatureSet qfs;
+        try (Connection c = source.getConnection()) {
+            qfs = new QueryFeatureSet("SELECT * FROM features.\"Parks\"", source, c);
+        }
+
+        final SimpleQuery sq = new SimpleQuery();
+        sq.setSortBy(FF.sort("native_name", SortOrder.DESCENDING));
+        sq.setFilter(FF.equals(FF.property("country"), FF.literal("FRA")));
+        sq.setColumns(new SimpleQuery.Column(FF.property("native_name")));
+        final FeatureSet frenchParks = qfs.subset(sq);
+        checkQueryType(Collections.singletonMap("native_name", String.class), frenchParks.getType());
+        try (Stream<Feature> fs = frenchParks.features(false)) {
+            final Object[] queryResult = fs.map(f -> f.getPropertyValue("native_name"))
+                    .toArray(size -> new Object[size]);
+            assertArrayEquals(new String[]{"Jardin du Luxembourg", "Jardin des Tuileries"},
queryResult);
+        }
     }
 
     private void verifyFetchCityTableAsQuery(DataSource source) throws Exception {
@@ -304,7 +325,7 @@ public final strictfp class SQLStoreTest extends TestCase {
             final String pName = p.getName().toString();
             final Class expectedClass = expectedAttrs.get(pName);
             assertNotNull("Unexpected property: "+pName, expectedClass);
-            assertEquals("Unepected type for property: "+pName, expectedClass, ((AttributeType)p).getValueClass());
+            assertEquals("Unexpected type for property: "+pName, expectedClass, ((AttributeType)p).getValueClass());
         }
     }
 


Mime
View raw message