sis-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From ama...@apache.org
Subject [sis] 12/45: WIP(SQLStore): still abstracting feature-type building to ease maintenance and evolutions.
Date Tue, 12 Nov 2019 16:44:39 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 60b0cbbd443366b867726dd5a9c44430f15ba3a2
Author: Alexis Manin <amanin@apache.org>
AuthorDate: Mon Sep 23 18:15:38 2019 +0200

    WIP(SQLStore): still abstracting feature-type building to ease maintenance and evolutions.
---
 .../apache/sis/internal/sql/feature/Analyzer.java  | 275 ++++++++-------------
 .../sql/feature/{metamodel => }/ColumnRef.java     |   6 +-
 .../apache/sis/internal/sql/feature/Features.java  |  18 +-
 .../sql/feature/{metamodel => }/PrimaryKey.java    |   6 +-
 .../sis/internal/sql/feature/QueryFeatureSet.java  |   4 +-
 .../apache/sis/internal/sql/feature/Relation.java  |  12 +-
 .../apache/sis/internal/sql/feature/SQLColumn.java |  58 +++++
 .../internal/sql/feature/SQLTypeSpecification.java |  35 +++
 .../org/apache/sis/internal/sql/feature/Table.java |  73 ++----
 .../org/apache/sis/storage/sql/SQLStoreTest.java   |  23 +-
 10 files changed, 255 insertions(+), 255 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 c91ddbb..157a823 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
@@ -21,7 +21,18 @@ import java.sql.PreparedStatement;
 import java.sql.ResultSet;
 import java.sql.ResultSetMetaData;
 import java.sql.SQLException;
-import java.util.*;
+import java.util.ArrayList;
+import java.util.Collection;
+import java.util.Collections;
+import java.util.HashMap;
+import java.util.HashSet;
+import java.util.LinkedHashSet;
+import java.util.List;
+import java.util.Locale;
+import java.util.Map;
+import java.util.Objects;
+import java.util.Optional;
+import java.util.Set;
 import java.util.logging.Level;
 import java.util.logging.LogRecord;
 import javax.sql.DataSource;
@@ -29,7 +40,6 @@ import javax.sql.DataSource;
 import org.opengis.feature.Feature;
 import org.opengis.feature.FeatureType;
 import org.opengis.feature.PropertyType;
-import org.opengis.referencing.crs.CoordinateReferenceSystem;
 import org.opengis.util.GenericName;
 import org.opengis.util.NameFactory;
 import org.opengis.util.NameSpace;
@@ -41,8 +51,6 @@ import org.apache.sis.feature.builder.FeatureTypeBuilder;
 import org.apache.sis.internal.metadata.sql.Dialect;
 import org.apache.sis.internal.metadata.sql.Reflection;
 import org.apache.sis.internal.metadata.sql.SQLUtilities;
-import org.apache.sis.internal.sql.feature.metamodel.ColumnRef;
-import org.apache.sis.internal.sql.feature.metamodel.PrimaryKey;
 import org.apache.sis.internal.system.DefaultFactories;
 import org.apache.sis.storage.DataStore;
 import org.apache.sis.storage.DataStoreContentException;
@@ -50,6 +58,7 @@ import org.apache.sis.storage.DataStoreException;
 import org.apache.sis.storage.InternalDataStoreException;
 import org.apache.sis.storage.sql.SQLStore;
 import org.apache.sis.util.collection.BackingStoreException;
+import org.apache.sis.util.iso.Names;
 import org.apache.sis.util.logging.WarningListeners;
 import org.apache.sis.util.resources.ResourceInternationalString;
 
@@ -326,24 +335,21 @@ final class Analyzer {
         return tables.values();
     }
 
-    public FeatureType buildFeatureType(final TableReference table, final TableReference
importedBy) throws SQLException {
-        try (TableMetadata metadata = new TableMetadata(table, importedBy)) {
-            return build(metadata);
-        }
+    public SQLTypeSpecification create(final TableReference table, final TableReference importedBy)
throws SQLException {
+        return new TableMetadata(table, importedBy);
     }
 
-    public FeatureType buildFeatureType(final PreparedStatement target, final String sourceQuery,
final GenericName optName) throws SQLException {
-        return build(new QuerySpecification(target, sourceQuery, optName));
+    public SQLTypeSpecification create(final PreparedStatement target, final String sourceQuery,
final GenericName optName) throws SQLException {
+        return new QuerySpecification(target, sourceQuery, optName);
     }
 
-    private FeatureType build(final SQLTypeSpecification spec) throws SQLException {
+    public FeatureType buildFeatureType(final SQLTypeSpecification spec) throws SQLException
{
         final FeatureTypeBuilder builder = new FeatureTypeBuilder(nameFactory, functions.library,
locale);
         builder.setName(spec.getName());
         builder.setDefinition(spec.getDefinition());
         final String geomCol = spec.getPrimaryGeometryColumn().orElse("");
         final List pkCols = spec.getPK().map(PrimaryKey::getColumns).orElse(Collections.EMPTY_LIST);
-        while  (spec.hasNext()) {
-            final SQLColumn col = spec.next();
+        for (SQLColumn col : spec.getColumns()) {
             Class<?> type = functions.toJavaType(col.getType(), col.getTypeName());
             final String colName = col.getName().getColumnName();
             final String attrName = col.getName().getAttributeName();
@@ -355,7 +361,7 @@ final class Analyzer {
             final AttributeTypeBuilder<?> attribute = builder
                     .addAttribute(type)
                     .setName(attrName);
-            if (col.isNullable) attribute.setMinimumOccurs(0);
+            if (col.isNullable()) attribute.setMinimumOccurs(0);
             final int precision = col.getPrecision();
             /* TODO: we should check column type. Precision for numbers or blobs is meaningfull,
but the convention
              * exposed by SIS does not allow to distinguish such cases.
@@ -384,15 +390,8 @@ final class Analyzer {
         }
 
         for (final Relation r : exports) {
-            final GenericName foreignTypeName = r.getName(Analyzer.this);
-            String propertyName = foreignTypeName.tip().toString();
-            final String base = propertyName;
-            int count = 0;
-            while (builder.isNameUsed("sis:"+base)) {
-                propertyName = base + '-' + ++count;
-            }
-            r.propertyName = propertyName;
             try {
+                final GenericName foreignTypeName = r.getName(Analyzer.this);
                 final Table foreignTable = table(r, foreignTypeName, null); // 'null' because
exported, not imported.
                 final AssociationRoleBuilder association;
                 if (foreignTable != null) {
@@ -401,7 +400,7 @@ final class Analyzer {
                 } else {
                     association = builder.addAssociation(foreignTypeName);     // May happen
in case of cyclic dependency.
                 }
-                association.setName("sis", r.propertyName)
+                association.setName(r.propertyName)
                         .setMinimumOccurs(0)
                         .setMaximumOccurs(Integer.MAX_VALUE);
             } catch (DataStoreException e) {
@@ -411,13 +410,7 @@ final class Analyzer {
     }
 
     private void addImports(SQLTypeSpecification spec, FeatureTypeBuilder target) throws
SQLException {
-        final List<Relation> imports;
-        try {
-            imports = spec.getImports();
-        } catch (DataStoreContentException e) {
-            throw new BackingStoreException(e);
-        }
-
+        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);
@@ -429,8 +422,7 @@ final class Analyzer {
             }
             final AssociationRoleBuilder association = foreignTable == null?
                     target.addAssociation(foreignTypeName) : target.addAssociation(foreignTable.featureType);
-            r.propertyName = foreignTypeName.tip().toString();
-            association.setName("sis", r.propertyName);
+            association.setName(r.propertyName);
         }
     }
 
@@ -439,48 +431,47 @@ final class Analyzer {
         void fill(ResultSet source, final Feature target);
     }
 
-    private interface SQLTypeSpecification extends Iterator<SQLColumn> {
-        /**
-         *
-         * @return Name for the feature type to build. Nullable.
-         * @throws SQLException If an error occurs while retrieving information from database.
-         */
-        GenericName getName() throws SQLException;
-
-        /**
-         *
-         * @return A succint description of the data source. Nullable.
-         * @throws SQLException If an error occurs while retrieving information from database.
-         */
-        String getDefinition() throws SQLException;
-
-        Optional<PrimaryKey<?>> getPK() throws SQLException;
-
-        List<Relation> getImports() throws SQLException, DataStoreContentException;
-
-        List<Relation> getExports() throws SQLException, DataStoreContentException;
-
-        default Optional<String> getPrimaryGeometryColumn() {return Optional.empty();}
-    }
-
-    private class TableMetadata implements SQLTypeSpecification, AutoCloseable {
-
+    private class TableMetadata implements SQLTypeSpecification {
         final TableReference id;
-        final ResultSet reflect;
         private final String tableEsc;
         private final String schemaEsc;
 
-        private boolean hasNext;
+        private final Optional<PrimaryKey> pk;
 
         private final TableReference importedBy;
 
+        private final List<SQLColumn> columns;
+
         private TableMetadata(TableReference source, TableReference importedBy) throws SQLException
{
             this.id = source;
             tableEsc = escape(source.table);
             schemaEsc = escape(source.schema);
-            reflect = metadata.getColumns(source.catalog, schemaEsc, tableEsc, null);
-            hasNext = reflect.next();
-            this.importedBy = importedBy;
+
+            try (ResultSet reflect = metadata.getPrimaryKeys(id.catalog, id.schema, id.table))
{
+                final List<String> cols = new ArrayList<>();
+                while (reflect.next()) {
+                    cols.add(getUniqueString(reflect, Reflection.COLUMN_NAME));
+                    // The actual Boolean value will be fetched in the loop on columns later.
+                }
+                pk = PrimaryKey.create(cols);
+            }
+
+            try (ResultSet reflect = metadata.getColumns(source.catalog, schemaEsc, tableEsc,
null)) {
+                this.importedBy = importedBy;
+
+                final ArrayList<SQLColumn> tmpList = new ArrayList<>();
+                while (reflect.next()) {
+                    final int type = reflect.getInt(Reflection.DATA_TYPE);
+                    final String typeName = reflect.getString(Reflection.TYPE_NAME);
+                    final boolean isNullable = Boolean.TRUE.equals(SQLUtilities.parseBoolean(reflect.getString(Reflection.IS_NULLABLE)));
+                    final ColumnRef name = new ColumnRef(getUniqueString(reflect, Reflection.COLUMN_NAME));
+                    final int precision = reflect.getInt(Reflection.COLUMN_SIZE);
+                    final SQLColumn col = new SQLColumn(type, typeName, isNullable, name,
precision);
+                    tmpList.add(col);
+                }
+
+                columns = Collections.unmodifiableList(tmpList);
+            }
         }
 
         @Override
@@ -511,27 +502,33 @@ final class Analyzer {
         }
 
         @Override
-        public Optional<PrimaryKey<?>> getPK() throws SQLException {
-            try (ResultSet reflect = metadata.getPrimaryKeys(id.catalog, id.schema, id.table))
{
-                final List<String> cols = new ArrayList<>();
-                while (reflect.next()) {
-                    cols.add(getUniqueString(reflect, Reflection.COLUMN_NAME));
-                    // The actual Boolean value will be fetched in the loop on columns later.
-                }
-                return PrimaryKey.create(cols);
-            }
+        public Optional<PrimaryKey> getPK() throws SQLException {
+            return pk;
         }
 
         @Override
-        public List<Relation> getImports() throws SQLException, DataStoreContentException
{
+        public List<SQLColumn> getColumns() {
+            return columns;
+        }
+
+        @Override
+        public List<Relation> getImports() throws SQLException {
             try (ResultSet reflect = metadata.getImportedKeys(id.catalog, id.schema, id.table))
{
                 if (!reflect.next()) return Collections.EMPTY_LIST;
-                final List<Relation> fks = new ArrayList<>(2);
+                final List<Relation> imports = new ArrayList<>(2);
                 do {
-                    Relation relation = new Relation(Analyzer.this, Relation.Direction.IMPORT,
reflect);
-                    fks.add(relation);
+                    Relation r = new Relation(Analyzer.this, Relation.Direction.IMPORT, reflect);
+                    final GenericName foreignTypeName = r.getName(Analyzer.this);
+                    final Collection<String> fks = r.getForeignerKeys();
+                    // If the link is composed of a single foreign key, we'll name it after
that name. Otherwise, we'll use
+                    // referenced table name, as it will surely be more explicit than a concatenation
of column names.
+                    if (fks.size() == 1) r.propertyName = Names.createGenericName(null, ":",
"sis", fks.iterator().next());
+                    else r.propertyName = Names.createGenericName(null, ":", "sis", foreignTypeName.tip().toString());
+                    imports.add(r);
                 } while (!reflect.isClosed());
-                return fks;
+                return imports;
+            } catch (DataStoreContentException e) {
+                throw new BackingStoreException(e);
             }
         }
 
@@ -542,6 +539,9 @@ final class Analyzer {
                 final List<Relation> exports = new ArrayList<>(2);
                 do {
                     final Relation export = new Relation(Analyzer.this, Relation.Direction.EXPORT,
reflect);
+                    final GenericName foreignTypeName = export.getName(Analyzer.this);
+                    final String propertyName = foreignTypeName.tip().toString();
+                    export.propertyName = Names.createGenericName(null, ":", "sis", propertyName);
                     if (!export.equals(importedBy)) {
                         exports.add(export);
                     }
@@ -555,32 +555,6 @@ final class Analyzer {
             return Optional.empty();
             //throw new UnsupportedOperationException("Not supported yet"); // "Alexis Manin
(Geomatys)" on 20/09/2019
         }
-
-        @Override
-        public boolean hasNext() {
-            return hasNext;
-        }
-
-        @Override
-        public SQLColumn next() {
-            try {
-                final int type = reflect.getInt(Reflection.DATA_TYPE);
-                final String typeName = reflect.getString(Reflection.TYPE_NAME);
-                final boolean isNullable = Boolean.TRUE.equals(SQLUtilities.parseBoolean(reflect.getString(Reflection.IS_NULLABLE)));
-                final ColumnRef name = new ColumnRef(getUniqueString(reflect, Reflection.COLUMN_NAME));
-                final int precision = reflect.getInt(Reflection.COLUMN_SIZE);
-                final SQLColumn col = new SQLColumn(type, typeName, isNullable, name, precision);
-                hasNext = reflect.next();
-                return col;
-            } catch (SQLException e) {
-                throw new BackingStoreException(e);
-            }
-        }
-
-        @Override
-        public void close() throws SQLException {
-            reflect.close();
-        }
     }
 
     private class QuerySpecification implements SQLTypeSpecification {
@@ -592,12 +566,27 @@ final class Analyzer {
         private final String query;
         private final GenericName name;
 
+        private final List<SQLColumn> columns;
+
         public QuerySpecification(PreparedStatement source, String sourceQuery, GenericName
optName) throws SQLException {
             this.source = source;
             meta = source.getMetaData();
             total = meta.getColumnCount();
             query = sourceQuery;
             name = optName;
+
+            final ArrayList<SQLColumn> tmpCols = new ArrayList<>(total);
+            for (int i = 0 ; i < total ; i++) {
+                tmpCols.add(new SQLColumn(
+                        meta.getColumnType(idx),
+                        meta.getColumnTypeName(idx),
+                        meta.isNullable(idx) == ResultSetMetaData.columnNullable,
+                        new ColumnRef(meta.getColumnName(idx)).as(meta.getColumnLabel(idx)),
+                        meta.getPrecision(idx)
+                ));
+            }
+
+            columns = Collections.unmodifiableList(tmpCols);
         }
 
         @Override
@@ -611,92 +600,24 @@ final class Analyzer {
         }
 
         @Override
-        public Optional<PrimaryKey<?>> getPK() throws SQLException {
+        public Optional<PrimaryKey> getPK() throws SQLException {
             return Optional.empty();
         }
 
         @Override
-        public List<Relation> getImports() throws SQLException {
-            return Collections.EMPTY_LIST;
+        public List<SQLColumn> getColumns() {
+            return columns;
         }
 
         @Override
-        public List<Relation> getExports() throws SQLException, DataStoreContentException
{
+        public List<Relation> getImports() throws SQLException {
             return Collections.EMPTY_LIST;
         }
 
         @Override
-        public boolean hasNext() {
-            return idx < total;
-        }
-
-        @Override
-        public SQLColumn next() {
-            try {
-                final SQLColumn col = new SQLColumn(
-                        meta.getColumnType(idx),
-                        meta.getColumnTypeName(idx),
-                        meta.isNullable(idx) == ResultSetMetaData.columnNullable,
-                        new ColumnRef(meta.getColumnName(idx)).as(meta.getColumnLabel(idx)),
-                        meta.getPrecision(idx)
-                );
-                idx++;
-                return col;
-            } catch (SQLException e) {
-                throw new BackingStoreException(e);
-            }
+        public List<Relation> getExports() throws SQLException, DataStoreContentException
{
+            return Collections.EMPTY_LIST;
         }
     }
 
-    private class SQLColumn {
-        final int type;
-        final String typeName;
-        private final boolean isNullable;
-        private final ColumnRef naming;
-        private final int precision;
-
-        public SQLColumn(int type, String typeName, boolean isNullable, ColumnRef naming,
int precision) {
-            this.type = type;
-            this.typeName = typeName;
-            this.isNullable = isNullable;
-            this.naming = naming;
-            this.precision = precision;
-        }
-
-        public ColumnRef getName() {
-            return naming;
-        }
-
-        public int getType() {
-            return type;
-        }
-
-        public String getTypeName() {
-            return typeName;
-        }
-
-        public boolean isNullable() {
-            return isNullable;
-        }
-
-        /**
-         * Same as {@link ResultSetMetaData#getPrecision(int)}.
-         * @return 0 if unknown. For texts, maximum number of characters allowed. For numerics,
max precision. For blobs,
-         * number of bytes allowed.
-         */
-        public int getPrecision() {
-            return precision;
-        }
-
-        /**
-         * TODO: implement.
-         * Note : This method could be used not only for geometric fields, but also on numeric
ones representing 1D
-         * systems.
-         *
-         * @return null for now, implementation needed.
-         */
-        public Optional<CoordinateReferenceSystem> getCrs() {
-            return Optional.empty();
-        }
-    }
 }
diff --git a/storage/sis-sqlstore/src/main/java/org/apache/sis/internal/sql/feature/metamodel/ColumnRef.java
b/storage/sis-sqlstore/src/main/java/org/apache/sis/internal/sql/feature/ColumnRef.java
similarity index 93%
rename from storage/sis-sqlstore/src/main/java/org/apache/sis/internal/sql/feature/metamodel/ColumnRef.java
rename to storage/sis-sqlstore/src/main/java/org/apache/sis/internal/sql/feature/ColumnRef.java
index a2ff8c2..d02fb8f 100644
--- a/storage/sis-sqlstore/src/main/java/org/apache/sis/internal/sql/feature/metamodel/ColumnRef.java
+++ b/storage/sis-sqlstore/src/main/java/org/apache/sis/internal/sql/feature/ColumnRef.java
@@ -1,4 +1,4 @@
-package org.apache.sis.internal.sql.feature.metamodel;
+package org.apache.sis.internal.sql.feature;
 
 import java.util.Objects;
 
@@ -10,12 +10,12 @@ import static org.apache.sis.util.ArgumentChecks.ensureNonNull;
  * A column reference. Specify name of the column, and optionally an alias to use for public
visibility.
  * By default, column has no alias. To create a column with an alias, use {@code ColumnRef
myCol = new ColumnRef("colName).as("myAlias");}
  */
-public final class ColumnRef {
+final class ColumnRef {
     private final String name;
     private final String alias;
     private final String attrName;
 
-    public ColumnRef(String name) {
+    ColumnRef(String name) {
         ensureNonNull("Column name", name);
         this.name = this.attrName = name;
         alias = null;
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 c47f65e..204cadd 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
@@ -38,9 +38,9 @@ import org.opengis.feature.Feature;
 import org.opengis.feature.FeatureType;
 import org.opengis.filter.Filter;
 import org.opengis.filter.sort.SortBy;
+import org.opengis.util.GenericName;
 
 import org.apache.sis.internal.metadata.sql.SQLBuilder;
-import org.apache.sis.internal.sql.feature.metamodel.ColumnRef;
 import org.apache.sis.storage.DataStoreException;
 import org.apache.sis.storage.InternalDataStoreException;
 import org.apache.sis.util.ArraysExt;
@@ -82,13 +82,13 @@ final class Features implements Spliterator<Feature> {
      * Imported or exported features read by {@code dependencies[i]} will be stored in
      * the association named {@code associationNames[i]}.
      */
-    private final String[] associationNames;
+    private final GenericName[] associationNames;
 
     /**
      * Name of the property where to store the association that we can not handle with other
{@link #dependencies}.
      * This deferred association may exist because of circular dependency.
      */
-    private final String deferredAssociation;
+    private final GenericName deferredAssociation;
 
     /**
      * The feature sets referenced through foreigner keys, or {@link #EMPTY} if none.
@@ -212,9 +212,9 @@ final class Features implements Spliterator<Feature> {
             foreignerKeyIndices = null;
             deferredAssociation = null;
         } else {
-            String deferredAssociation = null;
+            GenericName deferredAssociation = null;
             final Features[]     dependencies = new Features[totalCount];
-            final String[]   associationNames = new String  [totalCount];
+            final GenericName[]  associationNames = new GenericName[totalCount];
             final int[][] foreignerKeyIndices = new int     [totalCount][];
             /*
              * For each foreigner key to another table, append all columns of that foreigner
key
@@ -453,7 +453,7 @@ final class Features implements Spliterator<Feature> {
                     }
                     value = dependency.fetchReferenced(null, feature);
                 }
-                feature.setPropertyValue(associationNames[i], value);
+                feature.setPropertyValue(associationNames[i].toString(), value);
             }
             action.accept(feature);
             if (!all) return true;
@@ -486,7 +486,7 @@ final class Features implements Spliterator<Feature> {
         }
         if (owner != null && deferredAssociation != null) {
             for (final Feature feature : features) {
-                feature.setPropertyValue(deferredAssociation, owner);
+                feature.setPropertyValue(deferredAssociation.toString(), owner);
             }
         }
         Object feature;
@@ -604,13 +604,13 @@ final class Features implements Spliterator<Feature> {
             this.source = source;
             this.distinct = distinct;
             this.columns = columns;
-            this.sort = Arrays.copyOf(source.sort, source.sort.length);
+            this.sort = source.sort == null ? null : Arrays.copyOf(source.sort, source.sort.length);
         }
 
         public Features connect(final Connection conn) throws SQLException, DataStoreException
{
             return new Features(
                     source.parent, conn,
-                    columns == null || columns.length < 1? source.parent.attributes :
Arrays.asList(columns),
+                    columns == null || columns.length < 1 ? source.parent.attributes :
Arrays.asList(columns),
                     new ArrayList<>(), null, distinct, source.offset, source.limit
             );
         }
diff --git a/storage/sis-sqlstore/src/main/java/org/apache/sis/internal/sql/feature/metamodel/PrimaryKey.java
b/storage/sis-sqlstore/src/main/java/org/apache/sis/internal/sql/feature/PrimaryKey.java
similarity index 88%
rename from storage/sis-sqlstore/src/main/java/org/apache/sis/internal/sql/feature/metamodel/PrimaryKey.java
rename to storage/sis-sqlstore/src/main/java/org/apache/sis/internal/sql/feature/PrimaryKey.java
index 2633960..25ee889 100644
--- a/storage/sis-sqlstore/src/main/java/org/apache/sis/internal/sql/feature/metamodel/PrimaryKey.java
+++ b/storage/sis-sqlstore/src/main/java/org/apache/sis/internal/sql/feature/PrimaryKey.java
@@ -1,4 +1,4 @@
-package org.apache.sis.internal.sql.feature.metamodel;
+package org.apache.sis.internal.sql.feature;
 
 import java.util.ArrayList;
 import java.util.Collections;
@@ -7,9 +7,9 @@ import java.util.Optional;
 
 import org.apache.sis.util.ArgumentChecks;
 
-public interface PrimaryKey<T> {
+interface PrimaryKey {
 
-    static Optional<PrimaryKey<?>> create(List<String> cols) {
+    static Optional<PrimaryKey> create(List<String> cols) {
         if (cols == null || cols.isEmpty()) return Optional.empty();
         if (cols.size() == 1) return Optional.of(new Simple(cols.get(0)));
         return Optional.of(new Composite(cols));
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 b814271..b45c2dc 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
@@ -34,7 +34,8 @@ public class QueryFeatureSet extends AbstractFeatureSet {
         try (Connection conn = connectReadOnly(source)) {
             final String sql = queryBuilder.toString();
             final PreparedStatement statement = conn.prepareStatement(sql);
-            resultType = analyzer.buildFeatureType(statement, sql, null); // TODO: allow
user to give a name ?
+            final SQLTypeSpecification spec = analyzer.create(statement, sql, null);
+            resultType = analyzer.buildFeatureType(spec); // TODO: allow user to give a name
?
         } catch (SQLException e) {
             throw new DataStoreException("Cannot analyze query metadata (feature type determination)",
e);
         }
@@ -55,7 +56,6 @@ public class QueryFeatureSet extends AbstractFeatureSet {
     public static Connection connectReadOnly(final DataSource source) throws SQLException
{
         final Connection c = source.getConnection();
         try {
-            c.setTransactionIsolation(Connection.TRANSACTION_NONE);
             c.setAutoCommit(false);
             c.setReadOnly(true);
         } catch (SQLException e) {
diff --git a/storage/sis-sqlstore/src/main/java/org/apache/sis/internal/sql/feature/Relation.java
b/storage/sis-sqlstore/src/main/java/org/apache/sis/internal/sql/feature/Relation.java
index 140326e..3b7180b 100644
--- a/storage/sis-sqlstore/src/main/java/org/apache/sis/internal/sql/feature/Relation.java
+++ b/storage/sis-sqlstore/src/main/java/org/apache/sis/internal/sql/feature/Relation.java
@@ -27,12 +27,16 @@ import java.util.Objects;
 import java.sql.ResultSet;
 import java.sql.SQLException;
 import java.sql.DatabaseMetaData;
+
+import org.opengis.util.GenericName;
+
 import org.apache.sis.internal.util.CollectionsExt;
 import org.apache.sis.internal.metadata.sql.Reflection;
 import org.apache.sis.storage.DataStoreException;
 import org.apache.sis.storage.DataStoreContentException;
 import org.apache.sis.storage.InternalDataStoreException;
 import org.apache.sis.util.collection.TreeTable;
+import org.apache.sis.util.iso.Names;
 import org.apache.sis.util.resources.Errors;
 import org.apache.sis.util.Debug;
 
@@ -58,7 +62,7 @@ import org.apache.sis.util.Debug;
  * @since   1.0
  * @module
  */
-final class Relation extends TableReference {
+public final class Relation extends TableReference {
     /**
      * Whether another table is <em>using</em> or is <em>used by</em>
the table containing the {@link Relation}.
      */
@@ -152,7 +156,7 @@ final class Relation extends TableReference {
      * The name of the feature property where the association to {@link #searchTable} table
will be stored.
      * Shall be set exactly once.
      */
-    String propertyName;
+    GenericName propertyName;
 
     /**
      * Whether the {@link #columns} map include all primary key columns. This field is set
to {@code false}
@@ -216,10 +220,10 @@ final class Relation extends TableReference {
      */
     final void setPropertyName(final String column, final int count) {
         if (columns.size() > 1) {
-            propertyName = freeText;        // Foreigner key name (may be null).
+            propertyName = Names.createGenericName(null, ":", "sis", freeText);        //
Foreigner key name (may be null).
         }
         if (propertyName == null) {
-            propertyName = (count == 0) ? column : column + '-' + count;
+            propertyName = Names.createGenericName(null, ":", "sis", (count == 0) ? column
: column + '-' + count);
         }
     }
 
diff --git a/storage/sis-sqlstore/src/main/java/org/apache/sis/internal/sql/feature/SQLColumn.java
b/storage/sis-sqlstore/src/main/java/org/apache/sis/internal/sql/feature/SQLColumn.java
new file mode 100644
index 0000000..d92fe8c
--- /dev/null
+++ b/storage/sis-sqlstore/src/main/java/org/apache/sis/internal/sql/feature/SQLColumn.java
@@ -0,0 +1,58 @@
+package org.apache.sis.internal.sql.feature;
+
+import java.sql.ResultSetMetaData;
+import java.util.Optional;
+
+import org.opengis.referencing.crs.CoordinateReferenceSystem;
+
+class SQLColumn {
+    final int type;
+    final String typeName;
+    private final boolean isNullable;
+    private final ColumnRef naming;
+    private final int precision;
+
+    SQLColumn(int type, String typeName, boolean isNullable, ColumnRef naming, int precision)
{
+        this.type = type;
+        this.typeName = typeName;
+        this.isNullable = isNullable;
+        this.naming = naming;
+        this.precision = precision;
+    }
+
+    public ColumnRef getName() {
+        return naming;
+    }
+
+    public int getType() {
+        return type;
+    }
+
+    public String getTypeName() {
+        return typeName;
+    }
+
+    public boolean isNullable() {
+        return isNullable;
+    }
+
+    /**
+     * Same as {@link ResultSetMetaData#getPrecision(int)}.
+     * @return 0 if unknown. For texts, maximum number of characters allowed. For numerics,
max precision. For blobs,
+     * number of bytes allowed.
+     */
+    public int getPrecision() {
+        return precision;
+    }
+
+    /**
+     * TODO: implement.
+     * Note : This method could be used not only for geometric fields, but also on numeric
ones representing 1D
+     * systems.
+     *
+     * @return null for now, implementation needed.
+     */
+    public Optional<CoordinateReferenceSystem> getCrs() {
+        return Optional.empty();
+    }
+}
diff --git a/storage/sis-sqlstore/src/main/java/org/apache/sis/internal/sql/feature/SQLTypeSpecification.java
b/storage/sis-sqlstore/src/main/java/org/apache/sis/internal/sql/feature/SQLTypeSpecification.java
new file mode 100644
index 0000000..0e35edf
--- /dev/null
+++ b/storage/sis-sqlstore/src/main/java/org/apache/sis/internal/sql/feature/SQLTypeSpecification.java
@@ -0,0 +1,35 @@
+package org.apache.sis.internal.sql.feature;
+
+import java.sql.SQLException;
+import java.util.List;
+import java.util.Optional;
+
+import org.opengis.util.GenericName;
+
+import org.apache.sis.storage.DataStoreContentException;
+
+interface SQLTypeSpecification {
+    /**
+     *
+     * @return Name for the feature type to build. Nullable.
+     * @throws SQLException If an error occurs while retrieving information from database.
+     */
+    GenericName getName() throws SQLException;
+
+    /**
+     *
+     * @return A succint description of the data source. Nullable.
+     * @throws SQLException If an error occurs while retrieving information from database.
+     */
+    String getDefinition() throws SQLException;
+
+    Optional<PrimaryKey> getPK() throws SQLException;
+
+    List<SQLColumn> getColumns();
+
+    List<Relation> getImports() throws SQLException;
+
+    List<Relation> getExports() throws SQLException, DataStoreContentException;
+
+    default Optional<String> getPrimaryGeometryColumn() {return Optional.empty();}
+}
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 a66add7..54df4f9 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
@@ -20,14 +20,13 @@ import java.sql.Connection;
 import java.sql.DatabaseMetaData;
 import java.sql.ResultSet;
 import java.sql.SQLException;
-import java.util.ArrayList;
 import java.util.Collection;
 import java.util.Collections;
-import java.util.LinkedHashMap;
 import java.util.List;
 import java.util.Map;
 import java.util.Optional;
 import java.util.concurrent.atomic.AtomicReference;
+import java.util.stream.Collectors;
 import java.util.stream.Stream;
 import javax.sql.DataSource;
 
@@ -35,11 +34,11 @@ import org.opengis.feature.AttributeType;
 import org.opengis.feature.Feature;
 import org.opengis.feature.FeatureAssociationRole;
 import org.opengis.feature.FeatureType;
+import org.opengis.feature.PropertyType;
 import org.opengis.util.GenericName;
 
 import org.apache.sis.internal.metadata.sql.Reflection;
 import org.apache.sis.internal.metadata.sql.SQLBuilder;
-import org.apache.sis.internal.sql.feature.metamodel.ColumnRef;
 import org.apache.sis.internal.storage.AbstractFeatureSet;
 import org.apache.sis.internal.storage.query.SimpleQuery;
 import org.apache.sis.storage.DataStoreException;
@@ -47,9 +46,7 @@ import org.apache.sis.storage.FeatureSet;
 import org.apache.sis.storage.InternalDataStoreException;
 import org.apache.sis.storage.Query;
 import org.apache.sis.storage.UnsupportedQueryException;
-import org.apache.sis.util.Classes;
 import org.apache.sis.util.Debug;
-import org.apache.sis.util.Numbers;
 import org.apache.sis.util.collection.TreeTable;
 import org.apache.sis.util.collection.WeakValueHashMap;
 
@@ -117,6 +114,8 @@ final class Table extends AbstractFeatureSet {
      */
     final Class<?> primaryKeyClass;
 
+    private final SQLTypeSpecification specification;
+
     /**
      * Feature instances already created for given primary keys. This map is used only when
requesting feature
      * instances by identifiers (not for iterating over all features) and those identifiers
are primary keys.
@@ -162,27 +161,6 @@ final class Table extends AbstractFeatureSet {
         this.sqlTemplate = new SQLBuilder(this.dbMeta, true);
         this.source = analyzer.source;
         this.name   = id;
-        final String tableEsc  = analyzer.escape(id.table);
-        final String schemaEsc = analyzer.escape(id.schema);
-        /*
-         * Get a list of primary keys. We need to know them before to create the attributes,
-         * in order to detect which attributes are used as components of Feature identifiers.
-         * In the 'primaryKeys' map, the boolean tells whether the column uses auto-increment,
-         * with null value meaning that we don't know.
-         *
-         * Note: when a table contains no primary keys, we could still look for index columns
-         * with unique constraint using metadata.getIndexInfo(catalog, schema, table, true).
-         * We don't do that for now because of uncertainties (which index to use if there
is
-         * many? If they are suitable as identifiers why they are not primary keys?).
-         */
-        final Map<String,Boolean> primaryKeys = new LinkedHashMap<>();
-        try (ResultSet reflect = analyzer.metadata.getPrimaryKeys(id.catalog, id.schema,
id.table)) {
-            while (reflect.next()) {
-                primaryKeys.put(analyzer.getUniqueString(reflect, Reflection.COLUMN_NAME),
null);
-                // The actual Boolean value will be fetched in the loop on columns later.
-            }
-        }
-        this.primaryKeys = primaryKeys.isEmpty() ? null : primaryKeys.keySet().toArray(new
String[primaryKeys.size()]);
         /*
          * Creates a list of associations between the table read by this method and other
tables.
          * The associations are defined by the foreigner keys referencing primary keys. Note
that
@@ -194,36 +172,32 @@ final class Table extends AbstractFeatureSet {
          * navigability because the database designer's choice may be driven by the need
to support
          * multi-occurrences.
          */
-        final List<Relation> importedKeys = new ArrayList<>();
-        final List<Relation> exportedKeys = new ArrayList<>();
         /*
          * For each column in the table that is not a foreigner key, create an AttributeType
of the same name.
          * The Java type is inferred from the SQL type, and the attribute multiplicity in
inferred from the SQL
          * nullability. Attribute names are added in the 'attributeNames' and 'attributeColumns'
list. Those
          * names are usually the same, except when a column is used both as a primary key
and as foreigner key.
          */
-        Class<?> primaryKeyClass   = null;
-        boolean  primaryKeyNonNull = true;
-        boolean  hasGeometry       = false;
-        final List<ColumnRef> attributes = new ArrayList<>();
 
         /*
          * If the primary keys uses more than one column, we will need an array to store
it.
          * If all columns are non-null numbers, use primitive arrays instead than array of
wrappers.
          */
-        if (primaryKeys.size() > 1) {
-            if (primaryKeyNonNull) {
-                primaryKeyClass = Numbers.wrapperToPrimitive(primaryKeyClass);
-            }
-            primaryKeyClass = Classes.changeArrayDimension(primaryKeyClass, 1);
-        }
-
-        this.featureType      = analyzer.buildFeatureType(id, importedBy);
-        this.importedKeys     = toArray(importedKeys);
-        this.exportedKeys     = toArray(exportedKeys);
-        this.primaryKeyClass  = primaryKeyClass;
-        this.hasGeometry      = hasGeometry;
-        this.attributes = Collections.unmodifiableList(attributes);
+        this.specification = analyzer.create(id, importedBy);
+        primaryKeys = (String[]) specification.getPK()
+                .map(PrimaryKey::getColumns)
+                .orElse(Collections.EMPTY_LIST)
+                .toArray(new String[0]);
+        this.featureType      = analyzer.buildFeatureType(specification);
+        this.importedKeys     = toArray(specification.getImports());
+        this.exportedKeys     = toArray(specification.getExports());
+        this.primaryKeyClass  = primaryKeys.length < 2? Object.class : Object[].class;
+        this.hasGeometry      = specification.getPrimaryGeometryColumn().isPresent();
+        this.attributes = Collections.unmodifiableList(
+                specification.getColumns().stream()
+                        .map(SQLColumn::getName)
+                        .collect(Collectors.toList())
+        );
     }
 
     @Override
@@ -273,7 +247,14 @@ final class Table extends AbstractFeatureSet {
                 for (final Relation relation : relations) {
                     if (!relation.isSearchTableDefined()) {
                         // A ClassCastException below would be a bug since 'relation.propertyName'
shall be for an association.
-                        FeatureAssociationRole association = (FeatureAssociationRole) featureType.getProperty(relation.propertyName);
+                        final PropertyType property = featureType.getProperty(relation.propertyName.toString());
+                        if (!(property instanceof FeatureAssociationRole)) {
+                            throw new IllegalStateException(String.format(
+                                    "We expect a feature association for %s relation %s.
Duplicate key ?",
+                                    direction.name(), relation.propertyName
+                            ));
+                        }
+                        FeatureAssociationRole association = (FeatureAssociationRole) property;
                         final Table table = tables.get(association.getValueType().getName());
                         if (table == null) {
                             throw new InternalDataStoreException(association.toString());
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 271bdb7..0ab5ccc 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
@@ -134,16 +134,16 @@ public final strictfp class SQLStoreTest extends TestCase {
             {
                 final FeatureSet cities = (FeatureSet) store.findResource("Cities");
                 verifyFeatureType(cities.getType(),
-                        new String[] {"sis:identifier", "pk:country", "country",   "native_name",
"english_name", "population",  "parks"},
-                        new Object[] {null,             String.class, "Countries", String.class,
 String.class,   Integer.class, "Parks"});
+                        new String[] {"sis:identifier", "country",   "native_name", "english_name",
"population", "sis:country", "sis:Parks"},
+                        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"},
-                        new Object[] {null,             String.class, String.class});
+                        new String[] {"sis:identifier", "code",       "native_name",  "sis:Cities"},
+                        new Object[] {null,             String.class, String.class, "Cities"});
 
                 verifyFeatureType(((FeatureSet) store.findResource("Parks")).getType(),
-                        new String[] {"sis:identifier", "pk:country", "FK_City", "city",
      "native_name", "english_name"},
-                        new Object[] {null,             String.class, "Cities",  String.class,
String.class,  String.class});
+                        new String[] {"sis:identifier", "country", "city",       "native_name",
"english_name", "sis:Cities"},
+                        new Object[] {null,             String.class,  String.class, String.class,
 String.class, "Cities"});
 
                 try (Stream<Feature> features = cities.features(false)) {
                     features.forEach((f) -> verifyContent(f));
@@ -213,6 +213,7 @@ public final strictfp class SQLStoreTest extends TestCase {
     private static void verifyFeatureType(final FeatureType type, final String[] expectedNames,
final Object[] expectedTypes) {
         int i = 0;
         for (PropertyType pt : type.getProperties(false)) {
+            if (i >= expectedNames.length) fail("Returned feature-type contains more properties
than expected. Example: "+pt.getName());
             assertEquals("name", expectedNames[i], pt.getName().toString());
             final Object expectedType = expectedTypes[i];
             if (expectedType != null) {
@@ -285,7 +286,7 @@ public final strictfp class SQLStoreTest extends TestCase {
         /*
          * Verify attributes. They are the easiest properties to read.
          */
-        assertEquals("pk:country",     country,              feature.getPropertyValue("pk:country"));
+        assertEquals("country",        country,              feature.getPropertyValue("country"));
         assertEquals("sis:identifier", country + ':' + city, feature.getPropertyValue("sis:identifier"));
         assertEquals("english_name",   englishName,          feature.getPropertyValue("english_name"));
         assertEquals("population",     population,           feature.getPropertyValue("population"));
@@ -293,9 +294,9 @@ public final strictfp class SQLStoreTest extends TestCase {
          * Associations using Relation.Direction.IMPORT.
          * Those associations should be cached; we verify with "Canada" case.
          */
-        assertEquals("country", countryName, getIndirectPropertyValue(feature, "country",
"native_name"));
+        assertEquals("country", countryName, getIndirectPropertyValue(feature, "sis:country",
"native_name"));
         if (isCanada) {
-            final Feature f = (Feature) feature.getPropertyValue("country");
+            final Feature f = (Feature) feature.getPropertyValue("sis:country");
             if (canada == null) {
                 canada = f;
             } else {
@@ -307,7 +308,7 @@ public final strictfp class SQLStoreTest extends TestCase {
          * Associations using Relation.Direction.EXPORT.
          * Contrarily to the IMPORT case, those associations can contain many values.
          */
-        final Collection<?> actualParks = (Collection<?>) feature.getPropertyValue("parks");
+        final Collection<?> actualParks = (Collection<?>) feature.getPropertyValue("sis:Parks");
         assertNotNull("parks", actualParks);
         assertEquals("parks.length", parks.length, actualParks.size());
         final Collection<String> expectedParks = new HashSet<>(Arrays.asList(parks));
@@ -323,7 +324,7 @@ public final strictfp class SQLStoreTest extends TestCase {
              * Verify the reverse association form Parks to Cities.
              * This create a cyclic graph, but SQLStore is capable to handle it.
              */
-            assertSame("City → Park → City", feature, pf.getPropertyValue("FK_City"));
+            assertSame("City → Park → City", feature, pf.getPropertyValue("sis:Cities"));
         }
     }
 


Mime
View raw message