This is an automated email from the ASF dual-hosted git repository.
desruisseaux pushed a commit to branch geoapi-4.0
in repository https://gitbox.apache.org/repos/asf/sis.git
The following commit(s) were added to refs/heads/geoapi-4.0 by this push:
new 5e694bfb26 Fix a DataStoreException about duplicated identifier when a
table contains 2 foreigner keys referencing the same table.
https://issues.apache.org/jira/browse/SIS-606
5e694bfb26 is described below
commit 5e694bfb2606f921a5344512b0d909724dac8bdc
Author: Martin Desruisseaux <martin.desruisse...@geomatys.com>
AuthorDate: Tue Oct 22 20:30:20 2024 +0200
Fix a DataStoreException about duplicated identifier when a table contains
2 foreigner keys referencing the same table.
https://issues.apache.org/jira/browse/SIS-606
---
.../org/apache/sis/storage/sql/feature/Analyzer.java | 7 +++----
.../apache/sis/storage/sql/feature/FeatureAdapter.java | 11 ++++++-----
.../org/apache/sis/storage/sql/feature/PrimaryKey.java | 12 ++++++++++++
.../apache/sis/storage/sql/feature/QueryAnalyzer.java | 6 +++---
.../org/apache/sis/storage/sql/feature/Relation.java | 17 ++++++++++++-----
.../main/org/apache/sis/storage/sql/feature/Table.java | 4 +---
.../apache/sis/storage/sql/feature/TableAnalyzer.java | 10 ++++------
7 files changed, 41 insertions(+), 26 deletions(-)
diff --git
a/endorsed/src/org.apache.sis.storage.sql/main/org/apache/sis/storage/sql/feature/Analyzer.java
b/endorsed/src/org.apache.sis.storage.sql/main/org/apache/sis/storage/sql/feature/Analyzer.java
index 645ea0bd47..d83976afba 100644
---
a/endorsed/src/org.apache.sis.storage.sql/main/org/apache/sis/storage/sql/feature/Analyzer.java
+++
b/endorsed/src/org.apache.sis.storage.sql/main/org/apache/sis/storage/sql/feature/Analyzer.java
@@ -227,8 +227,7 @@ public final class Analyzer {
* specified name. During this iteration, we may discover new tables
to analyze because of dependencies
* (foreigner keys).
*/
- final List<Table> tableList;
- tableList = new ArrayList<>(tableNames.length);
+ final var tableList = new ArrayList<Table>(tableNames.length);
for (final TableReference reference : declared) {
// Adds only the table explicitly required by the user.
tableList.add(table(reference, reference.getName(this), null));
@@ -246,8 +245,8 @@ public final class Analyzer {
/**
* Reads a string from the given result set and return a unique instance
of that string.
* This method should be invoked only for {@code String} instances that
are going to be
- * stored in {@link Table} or {@link Relation} structures; there is no
point to invoke
- * this method for example before to parse the string as a boolean.
+ * stored in {@link Table} or {@link Relation} structures. There is no
point to invoke
+ * this method for example before to parse the string as a Boolean.
*
* @param reflect the result set from which to read a string.
* @param column the column to read.
diff --git
a/endorsed/src/org.apache.sis.storage.sql/main/org/apache/sis/storage/sql/feature/FeatureAdapter.java
b/endorsed/src/org.apache.sis.storage.sql/main/org/apache/sis/storage/sql/feature/FeatureAdapter.java
index 53c794b92c..5ce00aa6f8 100644
---
a/endorsed/src/org.apache.sis.storage.sql/main/org/apache/sis/storage/sql/feature/FeatureAdapter.java
+++
b/endorsed/src/org.apache.sis.storage.sql/main/org/apache/sis/storage/sql/feature/FeatureAdapter.java
@@ -150,6 +150,7 @@ final class FeatureAdapter {
* @param following the relations that we are following. Used for
avoiding never ending loop.
* @param noFollow relation to not follow, or {@code null} if none.
*/
+ @SuppressWarnings("LocalVariableHidesMemberVariable")
private FeatureAdapter(final Table table, final DatabaseMetaData metadata,
final List<Relation> following, final Relation
noFollow)
throws SQLException, InternalDataStoreException
@@ -161,13 +162,13 @@ final class FeatureAdapter {
} else {
keyComponentClass = null;
}
- final Map<String,Integer> columnIndices = new HashMap<>();
+ final var columnIndices = new HashMap<String,Integer>();
/*
* Create a SELECT clause with all columns that are ordinary
attributes.
* Order matter, because `FeatureIterator` iterator will map the
columns
* to the attributes listed in the `attributes` array in that order.
*/
- final SQLBuilder sql = new
SQLBuilder(table.database).append(SQLBuilder.SELECT);
+ final var sql = new
SQLBuilder(table.database).append(SQLBuilder.SELECT);
for (final Column column : attributes) {
appendColumn(sql, column.label, columnIndices);
}
@@ -185,9 +186,9 @@ final class FeatureAdapter {
deferredAssociation = null;
} else {
String deferredAssociation = null;
- final FeatureAdapter[] dependencies = new
FeatureAdapter[count];
- final String[] associationNames = new String[count];
- final int[][] foreignerKeyIndices = new int[count][];
+ final var dependencies = new FeatureAdapter[count];
+ final var associationNames = new String[count];
+ final var foreignerKeyIndices = new int[count][];
/*
* For each foreigner key to another table, append all columns of
that foreigner key
* and the name of the single feature property where the
association will be stored.
diff --git
a/endorsed/src/org.apache.sis.storage.sql/main/org/apache/sis/storage/sql/feature/PrimaryKey.java
b/endorsed/src/org.apache.sis.storage.sql/main/org/apache/sis/storage/sql/feature/PrimaryKey.java
index 8ace760ca5..5341444b43 100644
---
a/endorsed/src/org.apache.sis.storage.sql/main/org/apache/sis/storage/sql/feature/PrimaryKey.java
+++
b/endorsed/src/org.apache.sis.storage.sql/main/org/apache/sis/storage/sql/feature/PrimaryKey.java
@@ -18,6 +18,8 @@ package org.apache.sis.storage.sql.feature;
import java.util.List;
import java.util.Collection;
+import java.util.StringJoiner;
+import org.apache.sis.util.Classes;
import org.apache.sis.util.privy.CollectionsExt;
import org.apache.sis.util.privy.UnmodifiableArrayList;
@@ -105,4 +107,14 @@ abstract class PrimaryKey {
return columns;
}
}
+
+ /**
+ * Returns a string representation of this primary key for debugging
purposes.
+ */
+ @Override
+ public String toString() {
+ var buffer = new StringJoiner(", ", "(", ")");
+ getColumns().forEach(buffer::add);
+ return buffer + " as " + Classes.getShortName(valueClass);
+ }
}
diff --git
a/endorsed/src/org.apache.sis.storage.sql/main/org/apache/sis/storage/sql/feature/QueryAnalyzer.java
b/endorsed/src/org.apache.sis.storage.sql/main/org/apache/sis/storage/sql/feature/QueryAnalyzer.java
index 73bf3dd419..d2083beeee 100644
---
a/endorsed/src/org.apache.sis.storage.sql/main/org/apache/sis/storage/sql/feature/QueryAnalyzer.java
+++
b/endorsed/src/org.apache.sis.storage.sql/main/org/apache/sis/storage/sql/feature/QueryAnalyzer.java
@@ -107,8 +107,8 @@ final class QueryAnalyzer extends FeatureAnalyzer {
@Override
Relation[] getForeignerKeys(final Relation.Direction direction) throws
SQLException, DataStoreException {
final boolean isImport = (direction == Relation.Direction.IMPORT);
- List<String> primaryKeyColumns = isImport ? new ArrayList<>() : null;
- final List<Relation> relations = new ArrayList<>();
+ var primaryKeyColumns = isImport ? new ArrayList<String>() : null;
+ final var relations = new ArrayList<Relation>();
for (final Map.Entry<TableReference, Map<String,Column>> entry :
columnsPerTable.entrySet()) {
final Set<String> columnNames = entry.getValue().keySet();
final TableReference src = entry.getKey();
@@ -121,7 +121,7 @@ final class QueryAnalyzer extends FeatureAnalyzer {
:
analyzer.metadata.getExportedKeys(src.catalog, src.schema, src.table))
{
if (reflect.next()) do {
- final Relation relation = new Relation(analyzer,
direction, reflect);
+ final var relation = new Relation(analyzer, direction,
reflect);
if (columnNames.containsAll(relation.getOwnerColumns())) {
if (isImport) {
addForeignerKeys(relation);
diff --git
a/endorsed/src/org.apache.sis.storage.sql/main/org/apache/sis/storage/sql/feature/Relation.java
b/endorsed/src/org.apache.sis.storage.sql/main/org/apache/sis/storage/sql/feature/Relation.java
index ae9e7dfd7a..8471b3ccbc 100644
---
a/endorsed/src/org.apache.sis.storage.sql/main/org/apache/sis/storage/sql/feature/Relation.java
+++
b/endorsed/src/org.apache.sis.storage.sql/main/org/apache/sis/storage/sql/feature/Relation.java
@@ -204,7 +204,7 @@ final class Relation extends TableReference {
analyzer.getUniqueString(reflect, dir.table),
analyzer.getUniqueString(reflect, Reflection.FK_NAME));
- final Map<String,String> m = new LinkedHashMap<>();
+ final var m = new LinkedHashMap<String,String>();
do {
final String column = analyzer.getUniqueString(reflect,
dir.column);
if (m.put(column, analyzer.getUniqueString(reflect,
dir.containerColumn)) != null) {
@@ -216,9 +216,16 @@ final class Relation extends TableReference {
break;
}
} while (table.equals(reflect.getString(dir.table)) &&
// Table name is mandatory.
- Objects.equals(schema, reflect.getString(dir.schema)) &&
// Schema and catalog may be null.
- Objects.equals(catalog, reflect.getString(dir.catalog)));
-
+ Objects.equals(schema, reflect.getString(dir.schema)) &&
// Schema and catalog may be null.
+ Objects.equals(catalog, reflect.getString(dir.catalog)) &&
+ Objects.equals(freeText,
reflect.getString(Reflection.FK_NAME)));
+ /*
+ * In above conditions, the comparison of `FK_NAME` is actually the
only mandatory check.
+ * The "catalog.schema.table" comparaison is a paranoiac check added
as a safety in case
+ * the foreigner key is null or empty in some JDBC implementations.
Note that the table
+ * name is not a sufficient condition, because we could have more than
one foreigner key
+ * referencing the same table.
+ */
columns = CollectionsExt.compact(m);
/*
* If the foreigner key uses exactly one column, we can use the name
of that column.
@@ -308,7 +315,7 @@ final class Relation extends TableReference {
* relations declare different column order in their foreigner
key. Note that the 'columns' map
* is unmodifiable if its size is less than 2, and modifiable
otherwise.
*/
- final Map<String,String> copy = new HashMap<>(columns);
+ final var copy = new HashMap<String,String>(columns);
columns.clear();
for (String key : pkColumns) {
String value = key;
diff --git
a/endorsed/src/org.apache.sis.storage.sql/main/org/apache/sis/storage/sql/feature/Table.java
b/endorsed/src/org.apache.sis.storage.sql/main/org/apache/sis/storage/sql/feature/Table.java
index f9f06ef535..7bb776d5cc 100644
---
a/endorsed/src/org.apache.sis.storage.sql/main/org/apache/sis/storage/sql/feature/Table.java
+++
b/endorsed/src/org.apache.sis.storage.sql/main/org/apache/sis/storage/sql/feature/Table.java
@@ -228,9 +228,7 @@ final class Table extends AbstractFeatureSet {
* have been set to association names. If
`ClassCastException` occurs here, it is a bug
* in our object constructions.
*/
- final FeatureAssociationRole association =
- (FeatureAssociationRole)
featureType.getProperty(relation.propertyName);
-
+ final var association = (FeatureAssociationRole)
featureType.getProperty(relation.propertyName);
final Table table =
tables.get(association.getValueType().getName());
if (table == null) {
throw new
InternalDataStoreException(association.toString());
diff --git
a/endorsed/src/org.apache.sis.storage.sql/main/org/apache/sis/storage/sql/feature/TableAnalyzer.java
b/endorsed/src/org.apache.sis.storage.sql/main/org/apache/sis/storage/sql/feature/TableAnalyzer.java
index d31ceddfc2..2571c18b95 100644
---
a/endorsed/src/org.apache.sis.storage.sql/main/org/apache/sis/storage/sql/feature/TableAnalyzer.java
+++
b/endorsed/src/org.apache.sis.storage.sql/main/org/apache/sis/storage/sql/feature/TableAnalyzer.java
@@ -16,8 +16,6 @@
*/
package org.apache.sis.storage.sql.feature;
-import java.util.Map;
-import java.util.List;
import java.util.ArrayList;
import java.util.LinkedHashMap;
import java.sql.SQLException;
@@ -106,7 +104,7 @@ final class TableAnalyzer extends FeatureAnalyzer {
*/
@Override
final Relation[] getForeignerKeys(final Relation.Direction direction)
throws SQLException, DataStoreException {
- final List<Relation> relations = new ArrayList<>();
+ final var relations = new ArrayList<Relation>();
final boolean isImport = (direction == Relation.Direction.IMPORT);
try (ResultSet reflect = isImport ?
analyzer.metadata.getImportedKeys(id.catalog, id.schema, id.table)
:
analyzer.metadata.getExportedKeys(id.catalog, id.schema, id.table))
@@ -153,11 +151,11 @@ final class TableAnalyzer extends FeatureAnalyzer {
* Get all columns in advance because `completeIntrospection(…)`
* needs to be invoked before to invoke `database.getMapping(column)`.
*/
- final Map<String,Column> columns = new LinkedHashMap<>();
+ final var columns = new LinkedHashMap<String,Column>();
final String quote = analyzer.metadata.getIdentifierQuoteString();
try (ResultSet reflect = analyzer.metadata.getColumns(id.catalog,
schemaEsc, tableEsc, null)) {
while (reflect.next()) {
- final Column column = new Column(analyzer, reflect, quote);
+ final var column = new Column(analyzer, reflect, quote);
if (columns.put(column.name, column) != null) {
throw duplicatedColumn(column);
}
@@ -170,7 +168,7 @@ final class TableAnalyzer extends FeatureAnalyzer {
/*
* Analyze the type of each column, which may be geometric as a
consequence of above call.
*/
- final List<Column> attributes = new ArrayList<>();
+ final var attributes = new ArrayList<Column>();
for (final Column column : columns.values()) {
if (createAttribute(column)) {
attributes.add(column);
