Skip to content

fix: address review of the dialect resolution changes #362

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
zantvoort merged 1 commit into from
Aug 3, 2026
+155 −82
Merged

fix: address review of the dialect resolution changes #362

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
104 changes: 61 additions & 43 deletions storm-core/src/main/java/st/orm/core/template/impl/DatabaseSchema.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -23,12 +23,16 @@
import java.sql.ResultSet;
import java.sql.SQLException;
import java.util.ArrayList;
import java.util.EnumSet;
import java.util.Collection;
import java.util.Collections;
import java.util.EnumMap;
import java.util.List;
import java.util.Map;
import java.util.Optional;
import java.util.Set;
import java.util.SortedMap;
import java.util.SortedSet;
import java.util.TreeMap;
import java.util.TreeSet;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
import st.orm.core.template.SqlDialect.ConstraintDiscoveryStrategy;
Expand Down Expand Up @@ -115,12 +119,15 @@ public record DbForeignKey(
/**
* A kind of constraint a schema read discovers.
*
* <p>Each kind is read by one query (or one set of metadata calls) per strategy, so a failure applies to the
* kind as a whole. See {@link #isDiscovered(ConstraintKind)} for why the outcome is recorded.</p>
* <p>The kinds are separate because a strategy can read one and fail on another: the JDBC metadata strategy
* asks for each with its own call. See {@link #isDiscovered(String, ConstraintKind)} for why the outcome is
* recorded.</p>
*/
public enum ConstraintKind {
/** Primary keys and unique keys, which every strategy reads together. */
KEY,
/** Primary keys. */
PRIMARY_KEY,
/** Unique keys. */
UNIQUE_KEY,
/** Foreign keys. */
FOREIGN_KEY
}
Comment on lines 119 to 133
Expand All @@ -131,36 +138,53 @@ public enum ConstraintKind {
private final SortedMap<String, List<DbUniqueKey>> uniqueKeysByTable;
private final SortedMap<String, List<DbForeignKey>> foreignKeysByTable;
private final SortedMap<String, Boolean> sequences;
private final Set<ConstraintKind> discoveredConstraints;
private final Map<ConstraintKind, SortedSet<String>> discoveredByKind;

private DatabaseSchema(
@Nonnull SortedMap<String, List<DbColumn>> columnsByTable,
@Nonnull SortedMap<String, List<DbPrimaryKey>> primaryKeysByTable,
@Nonnull SortedMap<String, List<DbUniqueKey>> uniqueKeysByTable,
@Nonnull SortedMap<String, List<DbForeignKey>> foreignKeysByTable,
@Nonnull SortedMap<String, Boolean> sequences,
@Nonnull Set<ConstraintKind> discoveredConstraints
@Nonnull Map<ConstraintKind, SortedSet<String>> discoveredByKind
) {
this.columnsByTable = columnsByTable;
this.primaryKeysByTable = primaryKeysByTable;
this.uniqueKeysByTable = uniqueKeysByTable;
this.foreignKeysByTable = foreignKeysByTable;
this.sequences = sequences;
this.discoveredConstraints = discoveredConstraints;
this.discoveredByKind = discoveredByKind;
}

/**
* Returns whether constraints of the given kind were actually read from the database.
* Returns whether constraints of the given kind were actually read for the given table.
*
* <p>A metadata query that fails leaves the corresponding map empty, which reads exactly like a schema that has
* <p>A metadata query that fails leaves the corresponding map empty, which reads exactly like a table that has
* no such constraints. Callers that would otherwise report a constraint as missing must check this first, so a
* database that cannot answer the query is reported as unknown rather than as wrong.</p>
*
* <p>The answer is per table and per kind, because a read can fail for one table while succeeding for the rest,
* and for one kind while succeeding for another. A failure that costs the schema its foreign keys leaves its
* primary keys, and every other table, still worth validating.</p>
*
* @param tableName the table to check, case-insensitively.
* @param kind the constraint kind to check.
* @return {@code true} if the read succeeded, {@code false} if it failed and the constraints are unknown.
*/
public boolean isDiscovered(@Nonnull ConstraintKind kind) {
return discoveredConstraints.contains(kind);
public boolean isDiscovered(@Nonnull String tableName, @Nonnull ConstraintKind kind) {
return discoveredByKind.getOrDefault(kind, EMPTY_TABLES).contains(tableName);
}

private static final SortedSet<String> EMPTY_TABLES = Collections.emptySortedSet();

/** Records that the given kind was read successfully for the given tables. */
private static void discovered(
@Nonnull Map<ConstraintKind, SortedSet<String>> discoveredByKind,
@Nonnull ConstraintKind kind,
@Nonnull Collection<String> tableNames
) {
discoveredByKind.computeIfAbsent(kind, k -> new TreeSet<>(String.CASE_INSENSITIVE_ORDER))
.addAll(tableNames);
}

/**
Expand Down Expand Up @@ -239,14 +263,14 @@ public static DatabaseSchema read(
}
}
// Discover primary keys, unique keys, and foreign keys using the dialect-provided strategy.
Set<ConstraintKind> discoveredConstraints = EnumSet.noneOf(ConstraintKind.class);
Map<ConstraintKind, SortedSet<String>> discoveredByKind = new EnumMap<>(ConstraintKind.class);
readConstraints(connection, metadata, catalog, schemaPattern, columnsByTable,
primaryKeysByTable, uniqueKeysByTable, foreignKeysByTable, constraintDiscoveryStrategy,
discoveredConstraints);
discoveredByKind);
// Discover sequences using the dialect-provided strategy.
readSequences(connection, catalog, schemaPattern, sequences, sequenceDiscoveryStrategy);
return new DatabaseSchema(columnsByTable, primaryKeysByTable, uniqueKeysByTable, foreignKeysByTable, sequences,
discoveredConstraints);
discoveredByKind);
}

// ------------------------------------------------------------------------------------------------------------------
Expand All @@ -266,7 +290,7 @@ private static void readConstraints(
@Nonnull SortedMap<String, List<DbUniqueKey>> uniqueKeysByTable,
@Nonnull SortedMap<String, List<DbForeignKey>> foreignKeysByTable,
@Nonnull ConstraintDiscoveryStrategy strategy,
@Nonnull Set<ConstraintKind> discovered
@Nonnull Map<ConstraintKind, SortedSet<String>> discovered
) throws SQLException {
switch (strategy) {
case JDBC_METADATA -> readConstraintsFromJdbcMetadata(
Expand Down Expand Up @@ -298,10 +322,8 @@ private static void readConstraintsFromJdbcMetadata(
@Nonnull SortedMap<String, List<DbPrimaryKey>> primaryKeysByTable,
@Nonnull SortedMap<String, List<DbUniqueKey>> uniqueKeysByTable,
@Nonnull SortedMap<String, List<DbForeignKey>> foreignKeysByTable,
@Nonnull Set<ConstraintKind> discovered
@Nonnull Map<ConstraintKind, SortedSet<String>> discovered
) throws SQLException {
boolean keysRead = true;
boolean foreignKeysRead = true;
for (String tableName : new ArrayList<>(columnsByTable.keySet())) {
try (ResultSet primaryKeys = metadata.getPrimaryKeys(catalog, schemaPattern, tableName)) {
while (primaryKeys.next()) {
Expand All @@ -311,10 +333,10 @@ private static void readConstraintsFromJdbcMetadata(
primaryKeysByTable.computeIfAbsent(pkTableName, k -> new ArrayList<>())
.add(new DbPrimaryKey(pkTableName, columnName, keySeq));
}
discovered(discovered, ConstraintKind.PRIMARY_KEY, List.of(tableName));
} catch (SQLException e) {
// Some databases/views may not support getPrimaryKeys; the keys stay unknown.
// Some databases/views may not support getPrimaryKeys; this table's primary key stays unknown.
LOGGER.debug("Failed to read primary keys for table '{}'.", tableName, e);
keysRead = false;
}
}
for (String tableName : new ArrayList<>(columnsByTable.keySet())) {
Expand All @@ -332,10 +354,10 @@ private static void readConstraintsFromJdbcMetadata(
uniqueKeysByTable.computeIfAbsent(tableName, k -> new ArrayList<>())
.add(new DbUniqueKey(tableName, indexName, columnName, ordinalPosition));
}
discovered(discovered, ConstraintKind.UNIQUE_KEY, List.of(tableName));
} catch (SQLException e) {
// Some databases/views may not support getIndexInfo; the keys stay unknown.
// Some databases/views may not support getIndexInfo; this table's unique keys stay unknown.
LOGGER.debug("Failed to read unique indexes for table '{}'.", tableName, e);
keysRead = false;
}
}
for (String tableName : new ArrayList<>(columnsByTable.keySet())) {
Expand All @@ -348,18 +370,12 @@ private static void readConstraintsFromJdbcMetadata(
foreignKeysByTable.computeIfAbsent(fkTableName, k -> new ArrayList<>())
.add(new DbForeignKey(fkTableName, fkColumnName, pkTableName, pkColumnName));
}
discovered(discovered, ConstraintKind.FOREIGN_KEY, List.of(tableName));
} catch (SQLException e) {
// Some databases/views may not support getImportedKeys; the keys stay unknown.
// Some databases/views may not support getImportedKeys; this table's foreign keys stay unknown.
LOGGER.debug("Failed to read foreign keys for table '{}'.", tableName, e);
foreignKeysRead = false;
}
}
if (keysRead) {
discovered.add(ConstraintKind.KEY);
}
if (foreignKeysRead) {
discovered.add(ConstraintKind.FOREIGN_KEY);
}
}

/**
Expand Down Expand Up @@ -419,7 +435,7 @@ private static void readPrimaryAndUniqueKeysFromInformationSchema(
@Nonnull SortedMap<String, List<DbColumn>> columnsByTable,
@Nonnull SortedMap<String, List<DbPrimaryKey>> primaryKeysByTable,
@Nonnull SortedMap<String, List<DbUniqueKey>> uniqueKeysByTable,
@Nonnull Set<ConstraintKind> discovered
@Nonnull Map<ConstraintKind, SortedSet<String>> discovered
) {
try {
StringBuilder sql = new StringBuilder("""
Expand Down Expand Up @@ -454,9 +470,10 @@ private static void readPrimaryAndUniqueKeysFromInformationSchema(
}
}
}
discovered.add(ConstraintKind.KEY);
discovered(discovered, ConstraintKind.PRIMARY_KEY, columnsByTable.keySet());
discovered(discovered, ConstraintKind.UNIQUE_KEY, columnsByTable.keySet());
} catch (SQLException e) {
// INFORMATION_SCHEMA views not available; the keys stay unknown.
// INFORMATION_SCHEMA views not available; the primary and unique keys stay unknown.
LOGGER.debug("Failed to read primary and unique keys from INFORMATION_SCHEMA.", e);
}
}
Expand All @@ -474,7 +491,7 @@ private static void readConstraintsFromInformationSchema(
@Nonnull SortedMap<String, List<DbPrimaryKey>> primaryKeysByTable,
@Nonnull SortedMap<String, List<DbUniqueKey>> uniqueKeysByTable,
@Nonnull SortedMap<String, List<DbForeignKey>> foreignKeysByTable,
@Nonnull Set<ConstraintKind> discovered
@Nonnull Map<ConstraintKind, SortedSet<String>> discovered
) {
readPrimaryAndUniqueKeysFromInformationSchema(
connection, catalog, schemaPattern, columnsByTable, primaryKeysByTable, uniqueKeysByTable, discovered);
Expand Down Expand Up @@ -510,7 +527,7 @@ private static void readConstraintsFromInformationSchema(
.add(new DbForeignKey(fkTableName, fkColumnName, pkTableName, pkColumnName));
}
}
discovered.add(ConstraintKind.FOREIGN_KEY);
discovered(discovered, ConstraintKind.FOREIGN_KEY, columnsByTable.keySet());
} catch (SQLException e) {
// REFERENTIAL_CONSTRAINTS not available; the foreign keys stay unknown.
LOGGER.debug("Failed to read foreign keys from INFORMATION_SCHEMA.REFERENTIAL_CONSTRAINTS.", e);
Expand All @@ -529,7 +546,7 @@ private static void readConstraintsFromInformationSchemaReferencing(
@Nonnull SortedMap<String, List<DbPrimaryKey>> primaryKeysByTable,
@Nonnull SortedMap<String, List<DbUniqueKey>> uniqueKeysByTable,
@Nonnull SortedMap<String, List<DbForeignKey>> foreignKeysByTable,
@Nonnull Set<ConstraintKind> discovered
@Nonnull Map<ConstraintKind, SortedSet<String>> discovered
) {
// For databases that use catalogs as schemas, the catalog value represents the database name and maps to
// TABLE_SCHEMA in INFORMATION_SCHEMA views (not TABLE_CATALOG).
Expand Down Expand Up @@ -558,7 +575,7 @@ private static void readConstraintsFromInformationSchemaReferencing(
.add(new DbForeignKey(fkTableName, fkColumnName, pkTableName, pkColumnName));
}
}
discovered.add(ConstraintKind.FOREIGN_KEY);
discovered(discovered, ConstraintKind.FOREIGN_KEY, columnsByTable.keySet());
} catch (SQLException e) {
// REFERENCED columns not available; the foreign keys stay unknown.
LOGGER.debug("Failed to read foreign keys from INFORMATION_SCHEMA.KEY_COLUMN_USAGE.", e);
Expand All @@ -575,7 +592,7 @@ private static void readConstraintsFromAllConstraints(
@Nonnull SortedMap<String, List<DbPrimaryKey>> primaryKeysByTable,
@Nonnull SortedMap<String, List<DbUniqueKey>> uniqueKeysByTable,
@Nonnull SortedMap<String, List<DbForeignKey>> foreignKeysByTable,
@Nonnull Set<ConstraintKind> discovered
@Nonnull Map<ConstraintKind, SortedSet<String>> discovered
) {
// Primary keys and unique constraints.
try {
Expand Down Expand Up @@ -609,9 +626,10 @@ private static void readConstraintsFromAllConstraints(
}
}
}
discovered.add(ConstraintKind.KEY);
discovered(discovered, ConstraintKind.PRIMARY_KEY, columnsByTable.keySet());
discovered(discovered, ConstraintKind.UNIQUE_KEY, columnsByTable.keySet());
} catch (SQLException e) {
// ALL_CONSTRAINTS not available; the keys stay unknown.
// ALL_CONSTRAINTS not available; the primary and unique keys stay unknown.
LOGGER.debug("Failed to read primary and unique keys from ALL_CONSTRAINTS.", e);
}
// Foreign keys.
Expand Down Expand Up @@ -647,7 +665,7 @@ private static void readConstraintsFromAllConstraints(
.add(new DbForeignKey(fkTableName, fkColumnName, pkTableName, pkColumnName));
}
}
discovered.add(ConstraintKind.FOREIGN_KEY);
discovered(discovered, ConstraintKind.FOREIGN_KEY, columnsByTable.keySet());
} catch (SQLException e) {
// ALL_CONSTRAINTS FK query not available; the foreign keys stay unknown.
LOGGER.debug("Failed to read foreign keys from ALL_CONSTRAINTS.", e);
Expand Down
Loading
Loading