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 7acadb85587d32cb8ad725e3f66fc5a7bc0170ba Author: Alexis Manin <[email protected]> AuthorDate: Wed Oct 16 18:14:29 2019 +0200 feat(SQLStore): improve query feature set to delegate count and subqueries to target database. --- .../java/org/apache/sis/filter/DefaultBBOX.java | 2 +- .../java/org/apache/sis/internal/feature/ESRI.java | 8 ++- .../java/org/apache/sis/internal/feature/JTS.java | 33 ++++++++++- .../org/apache/sis/internal/feature/Java2D.java | 12 +++- .../org/apache/sis/internal/feature/jts/JTS.java | 38 ++++++++++-- .../sis/internal/sql/feature/ANSIInterpreter.java | 32 ++++------- .../apache/sis/internal/sql/feature/Analyzer.java | 3 +- .../apache/sis/internal/sql/feature/Features.java | 28 ++++----- .../sis/internal/sql/feature/QueryFeatureSet.java | 67 +++++++++++++++++----- .../sis/internal/sql/feature/SQLQueryAdapter.java | 31 ++++++---- .../apache/sis/internal/sql/feature/StreamSQL.java | 6 +- .../org/apache/sis/internal/sql/feature/Table.java | 6 +- .../sis/internal/sql/feature/TableSubset.java | 5 ++ .../sis/internal/sql/feature/package-info.java | 10 ++++ .../sql/feature/FilterInterpreterTest.java | 9 +-- .../org/apache/sis/storage/sql/SQLStoreTest.java | 27 ++++++++- 16 files changed, 232 insertions(+), 85 deletions(-) diff --git a/core/sis-feature/src/main/java/org/apache/sis/filter/DefaultBBOX.java b/core/sis-feature/src/main/java/org/apache/sis/filter/DefaultBBOX.java index a597e45..6444070 100644 --- a/core/sis-feature/src/main/java/org/apache/sis/filter/DefaultBBOX.java +++ b/core/sis-feature/src/main/java/org/apache/sis/filter/DefaultBBOX.java @@ -79,7 +79,7 @@ final class DefaultBBOX implements BBOX { private boolean nonOptimizedIntersect(Object candidate) { Envelope leftEval = left == null ? null : asEnvelope(left, candidate); - Envelope rightEval = left == null ? null : asEnvelope(right, candidate); + Envelope rightEval = right == null ? null : asEnvelope(right, candidate); if (left == null) { return multiIntersect(candidate, rightEval); } else if (right == null) { diff --git a/core/sis-feature/src/main/java/org/apache/sis/internal/feature/ESRI.java b/core/sis-feature/src/main/java/org/apache/sis/internal/feature/ESRI.java index 80a71dab..9659549 100644 --- a/core/sis-feature/src/main/java/org/apache/sis/internal/feature/ESRI.java +++ b/core/sis-feature/src/main/java/org/apache/sis/internal/feature/ESRI.java @@ -146,6 +146,12 @@ final class ESRI extends Geometries<Geometry> { return path; } + @Override + public Geometry toPolygon(Geometry polyline) throws IllegalArgumentException { + if (polyline instanceof Polygon) return polyline; + return createMultiPolygonImpl(polyline); + } + /** * Merges a sequence of points or paths if the first instance is an implementation of this library. * @@ -206,7 +212,7 @@ add: for (;;) { } @Override - Object createMultiPolygonImpl(Object... polygonsOrLinearRings) { + Polygon createMultiPolygonImpl(Object... polygonsOrLinearRings) { final Polygon poly = new Polygon(); for (final Object polr : polygonsOrLinearRings) { if (polr instanceof MultiPath) { diff --git a/core/sis-feature/src/main/java/org/apache/sis/internal/feature/JTS.java b/core/sis-feature/src/main/java/org/apache/sis/internal/feature/JTS.java index 2e9b184..34c5ba0 100644 --- a/core/sis-feature/src/main/java/org/apache/sis/internal/feature/JTS.java +++ b/core/sis-feature/src/main/java/org/apache/sis/internal/feature/JTS.java @@ -21,10 +21,14 @@ import java.util.Arrays; import java.util.Iterator; import java.util.List; +import org.opengis.referencing.crs.CoordinateReferenceSystem; +import org.opengis.util.FactoryException; + import org.apache.sis.geometry.GeneralEnvelope; import org.apache.sis.math.Vector; import org.apache.sis.setup.GeometryLibrary; import org.apache.sis.util.Classes; +import org.apache.sis.util.collection.BackingStoreException; import org.locationtech.jts.geom.Coordinate; import org.locationtech.jts.geom.Envelope; @@ -33,6 +37,7 @@ import org.locationtech.jts.geom.GeometryFactory; import org.locationtech.jts.geom.LineString; import org.locationtech.jts.geom.LinearRing; import org.locationtech.jts.geom.MultiLineString; +import org.locationtech.jts.geom.MultiPolygon; import org.locationtech.jts.geom.Point; import org.locationtech.jts.geom.Polygon; import org.locationtech.jts.io.ParseException; @@ -191,6 +196,32 @@ final class JTS extends Geometries<Geometry> { return toGeometry(lines); } + @Override + public Geometry toPolygon(Geometry polyline) throws IllegalArgumentException { + if (polyline instanceof Polygon) return polyline; + + Polygon result = null; + if (polyline instanceof LinearRing) { + result = factory.createPolygon((LinearRing) polyline); + } else if (polyline instanceof LineString) { + final LineString myLine = (LineString) polyline; + if (myLine.getEndPoint().equals(myLine.getStartPoint())) { + result = factory.createPolygon(myLine.getCoordinateSequence()); + } + } + + if (result == null) throw new IllegalArgumentException("Input is not a closed line."); + + try { + final CoordinateReferenceSystem crs = org.apache.sis.internal.feature.jts.JTS.getCoordinateReferenceSystem(polyline); + org.apache.sis.internal.feature.jts.JTS.setCoordinateReferenceSystem(result, crs); + } catch (FactoryException e) { + throw new BackingStoreException("Cannot extract CRS from geometry", e); + } + + return result; + } + /** * Makes a line string or linear ring from the given coordinates, and add the line string to the given list. * If the given coordinates array is empty, then this method does nothing. @@ -291,7 +322,7 @@ add: for (;;) { } @Override - Object createMultiPolygonImpl(Object... polygonsOrLinearRings) { + MultiPolygon createMultiPolygonImpl(Object... polygonsOrLinearRings) { final Polygon[] polys = new Polygon[polygonsOrLinearRings.length]; for (int i = 0 ; i < polys.length ; i++) { Object o = polygonsOrLinearRings[i]; diff --git a/core/sis-feature/src/main/java/org/apache/sis/internal/feature/Java2D.java b/core/sis-feature/src/main/java/org/apache/sis/internal/feature/Java2D.java index 991a723..35f9a04 100644 --- a/core/sis-feature/src/main/java/org/apache/sis/internal/feature/Java2D.java +++ b/core/sis-feature/src/main/java/org/apache/sis/internal/feature/Java2D.java @@ -178,7 +178,6 @@ final class Java2D extends Geometries<Shape> { } if (Double.isNaN(x) || Double.isNaN(y)) { if (lastX == startX && lastY == startY) path.closePath(); - startX = Double.NaN; lineTo = false; startX = startY = Double.NaN; } else if (lineTo) { @@ -258,13 +257,20 @@ add: for (;;) { } @Override - Object createMultiPolygonImpl(Object... polygonsOrLinearRings) { + Shape createMultiPolygonImpl(Object... polygonsOrLinearRings) { ensureNonEmpty("Polygons or linear rings to merge", polygonsOrLinearRings); - if (polygonsOrLinearRings.length == 1) return polygonsOrLinearRings[0]; + if (polygonsOrLinearRings.length == 1 && polygonsOrLinearRings[0] instanceof Shape) + return (Shape) polygonsOrLinearRings[0]; final Iterator<Object> it = Arrays.asList(polygonsOrLinearRings).iterator(); return tryMergePolylines(it.next(), it); } + @Override + public Shape toPolygon(Shape polyline) throws IllegalArgumentException { + // TODO: check that path ends with close. + return polyline; + } + /** * If the given object is a Java2D shape, builds its WKT representation. * Current implementation assumes that all closed shapes are polygons and that polygons have no hole diff --git a/core/sis-feature/src/main/java/org/apache/sis/internal/feature/jts/JTS.java b/core/sis-feature/src/main/java/org/apache/sis/internal/feature/jts/JTS.java index af9d29a..d3e1174 100644 --- a/core/sis-feature/src/main/java/org/apache/sis/internal/feature/jts/JTS.java +++ b/core/sis-feature/src/main/java/org/apache/sis/internal/feature/jts/JTS.java @@ -17,23 +17,29 @@ package org.apache.sis.internal.feature.jts; import java.util.Map; -import org.opengis.util.FactoryException; +import java.util.Optional; + 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.FactoryException; + +import org.apache.sis.geometry.Envelope2D; +import org.apache.sis.internal.system.Loggers; import org.apache.sis.internal.util.Constants; +import org.apache.sis.metadata.iso.extent.DefaultGeographicBoundingBox; import org.apache.sis.referencing.CRS; import org.apache.sis.util.Static; import org.apache.sis.util.Utilities; import org.apache.sis.util.logging.Logging; import org.apache.sis.util.resources.Errors; -import org.apache.sis.metadata.iso.extent.DefaultGeographicBoundingBox; -import org.apache.sis.geometry.Envelope2D; -import org.apache.sis.internal.system.Loggers; + import org.locationtech.jts.geom.Envelope; import org.locationtech.jts.geom.Geometry; +import static org.apache.sis.util.ArgumentChecks.ensureNonNull; + /** * Utilities for Java Topology Suite (JTS) objects. @@ -101,6 +107,30 @@ public final class JTS extends Static { return null; } + public static Optional<CoordinateReferenceSystem> setCoordinateReferenceSystem(final Geometry target, final CoordinateReferenceSystem toSet) { + ensureNonNull("Target geometry", target); + final Object ud = target.getUserData(); + if (ud == null) { + // By security, we reset SRID in case old CRS was defined this way. + target.setSRID(0); + target.setUserData(toSet); + return Optional.empty(); + } else if (ud instanceof CoordinateReferenceSystem) { + target.setUserData(toSet); + return Optional.of((CoordinateReferenceSystem)ud); + } else if (ud instanceof Map) { + final Map asMap = (Map) ud; + // In case user-data contains other useful data, we don't switch from map to CRS. We also reset SRID. + final Object oldVal = asMap.put(CRS_KEY, toSet); + // By security, we reset SRID in case old CRS was defined this way. + if (oldVal == null) { + target.setSRID(0); + } + } + + throw new IllegalArgumentException("Cannot modify input geometry, because user-data does not comply with SIS convention (should be a map or null, but was "+ud.getClass().getCanonicalName()+")."); + } + /** * Transforms the given geometry to the specified Coordinate Reference System (CRS). * If the given CRS or the given geometry is null, the geometry is returned unchanged. 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 676fa1a..943df9e 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 @@ -182,26 +182,9 @@ public class ANSIInterpreter implements FilterVisitor, ExpressionVisitor { @Override public Object visit(BBOX filter, Object extraData) { - final CharSequence left = evaluateMandatory(filter.getExpression1(), extraData); - final CharSequence right = evaluateMandatory(filter.getExpression2(), extraData); - - // TODO: In case source expression is already an envelope, we do not need to force envelope conversion. It would - // only be micro-optimisation however. - boolean leftToEnvelope = true; - boolean rightToEnvelope = true; - - final StringBuilder sb = new StringBuilder("ST_Intersects("); - if (leftToEnvelope) { - sb.append("ST_Envelope(").append(left).append(')'); - } else sb.append(left); - - sb.append(", "); - - if (rightToEnvelope) { - sb.append("ST_Envelope(").append(right).append(')'); - } else sb.append(right); - - return sb.append(')'); + // TODO: This is a wrong interpretation, but sqlmm has no equivalent of filter encoding bbox, so we'll + // fallback on a standard intersection. However, PostGIS, H2, etc. have their own versions of such filters. + return function("ST_Intersects(", filter, extraData); } @Override @@ -442,6 +425,11 @@ public class ANSIInterpreter implements FilterVisitor, ExpressionVisitor { return join(candidate::getExpression1, candidate::getExpression2, operator, extraData); } + + protected CharSequence join(BinarySpatialOperator candidate, String operator, Object extraData) { + return join(candidate::getExpression1, candidate::getExpression2, operator, extraData); + } + protected CharSequence join( Supplier<Expression> leftOperand, Supplier<Expression> rightOperand, @@ -526,9 +514,9 @@ public class ANSIInterpreter implements FilterVisitor, ExpressionVisitor { .mapToDouble(env::getSpan) .average() .orElseThrow(() -> new IllegalArgumentException("Given geometry envelope dimension is 0")); - return new StringBuilder("ST_GeomFromText(") + return new StringBuilder("ST_GeomFromText('") .append(Geometries.formatWKT(source, flatness)) - .append(')'); + .append("')"); } protected static double clampInfinity(final double candidate) { 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..dd97df9 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 @@ -151,6 +151,8 @@ final class Features implements Spliterator<Feature> { */ private final long estimatedSize; + private final FeatureAdapter adapter; + /** * Creates a new iterator over the feature instances. * @@ -180,6 +182,7 @@ final class Features implements Spliterator<Feature> { attributeNames[i++] = column.getAttributeName(); } this.featureType = table.featureType; + this.adapter = table.adapter; final DatabaseMetaData metadata = connection.getMetaData(); estimatedSize = following.isEmpty() ? table.countRows(metadata, true) : 0; /* @@ -414,13 +417,8 @@ final class Features implements Spliterator<Feature> { */ private boolean fetch(final Consumer<? super Feature> action, final boolean all) throws SQLException { while (result.next()) { - final Feature feature = featureType.newInstance(); - for (int i=0; i < attributeNames.length; i++) { - final Object value = result.getObject(i+1); - if (!result.wasNull()) { - feature.setPropertyValue(attributeNames[i], value); - } - } + // TODO: give connection to adapter. + final Feature feature = adapter.read(result, null); for (int i=0; i < dependencies.length; i++) { final Features dependency = dependencies[i]; final int[] columnIndices = foreignerKeyIndices[i]; @@ -667,12 +665,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 +684,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..e5d991c 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 { @@ -248,7 +289,7 @@ public class QueryFeatureSet extends AbstractFeatureSet { nativeOffset = originOffset + additionalOffset; } - if (originLimit < 0) { + if (originLimit <= 0) { javaLimit = this.additionalLimit; } else if (originLimit > 0 || additionalLimit > 0) { nativeLimit = Math.min(originLimit, additionalLimit); @@ -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/StreamSQL.java b/storage/sis-sqlstore/src/main/java/org/apache/sis/internal/sql/feature/StreamSQL.java index 30820a9..7291a04 100644 --- a/storage/sis-sqlstore/src/main/java/org/apache/sis/internal/sql/feature/StreamSQL.java +++ b/storage/sis-sqlstore/src/main/java/org/apache/sis/internal/sql/feature/StreamSQL.java @@ -55,9 +55,9 @@ import static org.apache.sis.util.ArgumentChecks.ensurePositive; * Manages query lifecycle and optimizations. Operations like {@link #count()}, {@link #distinct()}, {@link #skip(long)} * and {@link #limit(long)} are delegated to underlying SQL database. This class consistently propagate optimisation * strategies through streams obtained using {@link #map(Function)}, {@link #mapToDouble(ToDoubleFunction)} and - * {@link #peek(Consumer)} operations. However, for result consistency, no optimization is stacked once either - * {@link #filter(Predicate)} or {@link #flatMap(Function)} operations are called, as they modify browing flow (the - * count of stream elements is not bound 1 to 1 to query result rows). + * {@link #peek(Consumer)} operations. However, for result consistency, no optimization is stacked anymore after either + * {@link #filter(Predicate)} or {@link #flatMap(Function)} operations are called, as they modify volumetry (the count + * of stream elements is not bound 1 to 1 to query result rows). * * @since 1.0 * 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..7205f70 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 @@ -145,7 +145,7 @@ final class Table extends AbstractFeatureSet { */ private final SQLBuilder sqlTemplate; - private final FeatureAdapter adapter; + protected final FeatureAdapter adapter; /** * Creates a description of the table of the given name. @@ -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/internal/sql/feature/FilterInterpreterTest.java b/storage/sis-sqlstore/src/test/java/org/apache/sis/internal/sql/feature/FilterInterpreterTest.java index 7c73016..b9f59d7 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 @@ -18,15 +18,12 @@ public class FilterInterpreterTest extends TestCase { @Test public void testGeometricFilter() { - final ANSIInterpreter interpreter = new ANSIInterpreter(); final BBOX filter = FF.bbox(FF.property("Toto"), new GeneralEnvelope(new DefaultGeographicBoundingBox(-12.3, 2.1, 43.3, 51.7))); assertConversion(filter, "ST_Intersects(" + - "ST_Envelope(\"Toto\"), " + - "ST_Envelope(" + - "ST_GeomFromText(" + - "POLYGON ((-12.3 43.3, -12.3 51.7, 2.1 51.7, 2.1 43.3, -12.3 43.3))" + - ")" + + "\"Toto\", " + + "ST_GeomFromText(" + + "'POLYGON ((-12.3 43.3, -12.3 51.7, 2.1 51.7, 2.1 43.3, -12.3 43.3))'" + ")" + ")" ); 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()); } }
