jamesfredley commented on code in PR #16497:
URL: https://github.com/apache/grails-core/pull/16497#discussion_r4179358137
##########
grails-data-hibernate7/dbmigration-core/src/main/java/liquibase/ext/hibernate/snapshot/HibernateColumnSnapshotGenerator.java:
##########
@@ -267,6 +268,41 @@ public Class<? extends SnapshotGenerator>[] replaces() {
return new Class[]
{liquibase.snapshot.jvm.ColumnSnapshotGenerator.class};
}
+ private boolean handleSequenceGenerator(
+ org.hibernate.id.enhanced.SequenceStyleGenerator seqGen,
+ Dialect dialect,
+ HibernateDatabase database,
+ Column column,
+ org.hibernate.mapping.Table hibernateTable,
+ org.hibernate.mapping.Column hibernateColumn) {
+ if (PostgreSQLDialect.class.isAssignableFrom(dialect.getClass())) {
+ String sequenceName = resolveSequenceName(seqGen, hibernateTable,
hibernateColumn);
+ column.setDefaultValue(new DatabaseFunction("nextval('" +
sequenceName + "'::regclass)"));
+ return false;
+ }
+ return database.supportsAutoIncrement();
+ }
+
+ private boolean handleNativeGenerator(
+ org.hibernate.id.NativeGenerator nativeGen,
+ Dialect dialect,
+ HibernateDatabase database,
+ Column column,
+ org.hibernate.mapping.Table hibernateTable,
+ org.hibernate.mapping.Column hibernateColumn) {
+ return switch (nativeGen.getGenerationType()) {
+ case IDENTITY -> true;
+ case SEQUENCE -> {
+ var delegate =
IdentifierGeneratorSupport.nativeDelegate(nativeGen);
+ if (delegate instanceof
org.hibernate.id.enhanced.SequenceStyleGenerator seqGen) {
+ yield handleSequenceGenerator(seqGen, dialect, database,
column, hibernateTable, hibernateColumn);
+ }
+ yield database.supportsAutoIncrement();
+ }
Review Comment:
Fixed in 2a4a40a746. Sequence and table generators are not marked
auto-increment. Only an identity generator is. HSQL AUTO and the Spring bean
native mapping are no longer auto-increment. H2 classic native still is,
because that dialect's native strategy is identity. Oracle native sequence
coverage is in IdentifierGeneratorSnapshotTest.
##########
grails-data-hibernate7/dbmigration-core/src/main/java/liquibase/ext/hibernate/diff/HibernateChangedIndexChangeGenerator.java:
##########
@@ -0,0 +1,41 @@
+package liquibase.ext.hibernate.diff;
+
+import liquibase.change.Change;
+import liquibase.database.Database;
+import liquibase.diff.ObjectDifferences;
+import liquibase.diff.output.DiffOutputControl;
+import liquibase.diff.output.changelog.ChangeGeneratorChain;
+import liquibase.ext.hibernate.database.HibernateDatabase;
+import liquibase.structure.DatabaseObject;
+import liquibase.structure.core.Index;
+
+/**
+ * Hibernate does not know every index attribute ({@code unique}, {@code
using}), so those differences are
+ * suppressed to prevent needless drop and recreate changes on every diff.
+ */
+public class HibernateChangedIndexChangeGenerator
+ extends
liquibase.diff.output.changelog.core.ChangedIndexChangeGenerator {
+
+ @Override
+ public int getPriority(Class<? extends DatabaseObject> objectType,
Database database) {
+ return Index.class.isAssignableFrom(objectType) ? PRIORITY_ADDITIONAL
: PRIORITY_NONE;
+ }
+
+ @Override
+ public Change[] fixChanged(
+ DatabaseObject changedObject,
+ ObjectDifferences differences,
+ DiffOutputControl control,
+ Database referenceDatabase,
+ Database comparisonDatabase,
+ ChangeGeneratorChain chain) {
+ if (referenceDatabase instanceof HibernateDatabase ||
comparisonDatabase instanceof HibernateDatabase) {
+ differences.removeDifference("unique");
Review Comment:
Fixed in 2a4a40a746. Index uniqueness now comes from
hibernateIndex.isUnique(), not Column.isUnique() and not a fabricated false for
composite indexes. A concrete Boolean mismatch is still kept. Unknown
uniqueness and the using attribute are still suppressed when a
HibernateDatabase is in the diff.
##########
grails-data-hibernate7/dbmigration-core/src/main/java/liquibase/ext/hibernate/snapshot/HibernateSequenceSnapshotGenerator.java:
##########
@@ -36,15 +48,69 @@ protected void addTo(DatabaseObject foundObject,
DatabaseSnapshot snapshot)
if (foundObject instanceof Schema schema) {
HibernateDatabase database = (HibernateDatabase)
snapshot.getDatabase();
+ Set<String> addedSequences = new HashSet<>();
+
for (org.hibernate.boot.model.relational.Namespace namespace :
database.getMetadata().getDatabase().getNamespaces()) {
for (org.hibernate.boot.model.relational.Sequence sequence :
namespace.getSequences()) {
+ String name =
sequence.getName().getSequenceName().getText();
schema.addDatabaseObject(new Sequence()
-
.setName(sequence.getName().getSequenceName().getText())
+ .setName(name)
.setSchema(schema)
.setStartValue(BigInteger.valueOf(sequence.getInitialValue()))
.setIncrementBy(BigInteger.valueOf(sequence.getIncrementSize())));
+ addedSequences.add(name.toLowerCase(Locale.ROOT));
+ }
+ }
+
+ addGeneratorSequences(database, schema, addedSequences);
+ }
+ }
+
+ private void addGeneratorSequences(HibernateDatabase database, Schema
schema, Set<String> addedSequences) {
+ MetadataImplementor metadata = (MetadataImplementor)
database.getMetadata();
+ var dialect = database.getDialect();
+
+ for (PersistentClass entityBinding : metadata.getEntityBindings()) {
+ if (!(entityBinding instanceof RootClass rootClass) ||
+ !(rootClass.getIdentifier() instanceof SimpleValue
simpleValue) ||
+
!IdentifierGeneratorSupport.hasGenerationIntent(simpleValue)) {
+ continue;
+ }
+
+ try {
+ var generator = simpleValue.createGenerator(
+ dialect,
+ rootClass,
+ rootClass.getIdentifierProperty(),
+
IdentifierGeneratorSupport.createGeneratorSettings(simpleValue));
+
+ SequenceStyleGenerator seqGen = null;
+ // NativeGenerator may wrap a SequenceStyleGenerator delegate
depending on the dialect.
+ if (generator instanceof NativeGenerator nativeGen) {
+ if (IdentifierGeneratorSupport.nativeDelegate(nativeGen)
instanceof SequenceStyleGenerator s) {
+ seqGen = s;
+ }
+ } else if (generator instanceof SequenceStyleGenerator s) {
+ seqGen = s;
+ }
+
+ if (seqGen != null) {
+ var structure = seqGen.getDatabaseStructure();
+ if (structure != null && structure.getPhysicalName() !=
null) {
Review Comment:
Fixed in 2a4a40a746. Table-backed SequenceStyleGenerator structures are not
recorded as sequences. MySQL emulation of the auction package and an explicit
force_table_use mapping are both covered.
##########
grails-data-hibernate7/dbmigration-core/src/main/java/liquibase/ext/hibernate/snapshot/HibernateSequenceSnapshotGenerator.java:
##########
@@ -36,15 +48,69 @@ protected void addTo(DatabaseObject foundObject,
DatabaseSnapshot snapshot)
if (foundObject instanceof Schema schema) {
HibernateDatabase database = (HibernateDatabase)
snapshot.getDatabase();
+ Set<String> addedSequences = new HashSet<>();
+
for (org.hibernate.boot.model.relational.Namespace namespace :
database.getMetadata().getDatabase().getNamespaces()) {
for (org.hibernate.boot.model.relational.Sequence sequence :
namespace.getSequences()) {
+ String name =
sequence.getName().getSequenceName().getText();
schema.addDatabaseObject(new Sequence()
-
.setName(sequence.getName().getSequenceName().getText())
+ .setName(name)
.setSchema(schema)
.setStartValue(BigInteger.valueOf(sequence.getInitialValue()))
.setIncrementBy(BigInteger.valueOf(sequence.getIncrementSize())));
+ addedSequences.add(name.toLowerCase(Locale.ROOT));
+ }
+ }
+
+ addGeneratorSequences(database, schema, addedSequences);
+ }
+ }
+
+ private void addGeneratorSequences(HibernateDatabase database, Schema
schema, Set<String> addedSequences) {
+ MetadataImplementor metadata = (MetadataImplementor)
database.getMetadata();
+ var dialect = database.getDialect();
+
+ for (PersistentClass entityBinding : metadata.getEntityBindings()) {
+ if (!(entityBinding instanceof RootClass rootClass) ||
+ !(rootClass.getIdentifier() instanceof SimpleValue
simpleValue) ||
+
!IdentifierGeneratorSupport.hasGenerationIntent(simpleValue)) {
+ continue;
+ }
+
+ try {
+ var generator = simpleValue.createGenerator(
+ dialect,
+ rootClass,
+ rootClass.getIdentifierProperty(),
+
IdentifierGeneratorSupport.createGeneratorSettings(simpleValue));
+
+ SequenceStyleGenerator seqGen = null;
+ // NativeGenerator may wrap a SequenceStyleGenerator delegate
depending on the dialect.
+ if (generator instanceof NativeGenerator nativeGen) {
+ if (IdentifierGeneratorSupport.nativeDelegate(nativeGen)
instanceof SequenceStyleGenerator s) {
+ seqGen = s;
+ }
+ } else if (generator instanceof SequenceStyleGenerator s) {
+ seqGen = s;
+ }
+
+ if (seqGen != null) {
+ var structure = seqGen.getDatabaseStructure();
+ if (structure != null && structure.getPhysicalName() !=
null) {
+ String name = structure.getPhysicalName().render();
+ if (addedSequences.add(name.toLowerCase(Locale.ROOT)))
{
Review Comment:
Not fully fixed, so I am leaving this thread open.
The dedup key uses Identifier.getCanonicalName(), which preserves quoted
case and folds unquoted names. Liquibase 4.27 Sequence.equals and hashCode
still compare names case-insensitively, so a public snapshot cannot retain both
Foo and foo even when our key treats them as distinct. I did not add a snapshot
assertion that cannot pass.
##########
grails-data-hibernate7/dbmigration-core/src/main/java/liquibase/ext/hibernate/snapshot/IdentifierGeneratorSupport.java:
##########
@@ -0,0 +1,68 @@
+package liquibase.ext.hibernate.snapshot;
+
+import liquibase.Scope;
+import org.hibernate.boot.model.relational.SqlStringGenerationContext;
+import
org.hibernate.boot.model.relational.internal.SqlStringGenerationContextImpl;
+import org.hibernate.generator.Generator;
+import org.hibernate.id.NativeGenerator;
+import org.hibernate.mapping.GeneratorSettings;
+import org.hibernate.mapping.SimpleValue;
+
+/**
+ * Shared helpers for the snapshot generators that inspect identifier
generators.
+ */
+final class IdentifierGeneratorSupport {
+
+ private IdentifierGeneratorSupport() {
+ }
+
+ /**
+ * For annotation-based entities an identifier without {@code
@GeneratedValue} is application-assigned, so no
+ * generator applies. XML-mapped entities have no member details and
declare their generator in the hbm.xml
+ * mapping, so they always count as having generation intent.
+ */
+ static boolean hasGenerationIntent(SimpleValue simpleValue) {
+ var memberDetails = simpleValue.getMemberDetails();
+ return memberDetails == null ||
+
memberDetails.hasDirectAnnotationUsage(jakarta.persistence.GeneratedValue.class);
+ }
Review Comment:
Fixed in 2a4a40a746. hasGenerationIntent uses GeneratorCreator.isAssigned()
when a creator is present, and a direct @NativeGenerator still counts as
generation intent. NativeGenEntity no longer carries a redundant
@GeneratedValue, and both native tests pass. @SnowflakeId without
@GeneratedValue is still not treated as a database-generated id.
##########
grails-data-hibernate7/dbmigration-core/src/main/java/liquibase/ext/hibernate/snapshot/HibernateColumnSnapshotGenerator.java:
##########
@@ -267,6 +268,41 @@ public Class<? extends SnapshotGenerator>[] replaces() {
return new Class[]
{liquibase.snapshot.jvm.ColumnSnapshotGenerator.class};
}
+ private boolean handleSequenceGenerator(
+ org.hibernate.id.enhanced.SequenceStyleGenerator seqGen,
+ Dialect dialect,
+ HibernateDatabase database,
+ Column column,
+ org.hibernate.mapping.Table hibernateTable,
+ org.hibernate.mapping.Column hibernateColumn) {
+ if (PostgreSQLDialect.class.isAssignableFrom(dialect.getClass())) {
+ String sequenceName = resolveSequenceName(seqGen, hibernateTable,
hibernateColumn);
+ column.setDefaultValue(new DatabaseFunction("nextval('" +
sequenceName + "'::regclass)"));
+ return false;
+ }
+ return database.supportsAutoIncrement();
+ }
+
+ private boolean handleNativeGenerator(
+ org.hibernate.id.NativeGenerator nativeGen,
+ Dialect dialect,
+ HibernateDatabase database,
+ Column column,
+ org.hibernate.mapping.Table hibernateTable,
+ org.hibernate.mapping.Column hibernateColumn) {
+ return switch (nativeGen.getGenerationType()) {
+ case IDENTITY -> true;
+ case SEQUENCE -> {
+ var delegate =
IdentifierGeneratorSupport.nativeDelegate(nativeGen);
+ if (delegate instanceof
org.hibernate.id.enhanced.SequenceStyleGenerator seqGen) {
Review Comment:
Fixed in 2a4a40a746. An unconfigured GrailsNativeGenerator no longer
dereferences a null DatabaseStructure, and it does not invent a nextval
default. GormIdentifierSnapshot16497Spec snapshots native, sequence, and
identity ids through GormDatabase on PostgreSQL: native is auto-increment with
no nextval default, sequence keeps its nextval default, and identity is
auto-increment.
##########
grails-data-hibernate7/dbmigration-core/src/main/java/liquibase/ext/hibernate/snapshot/IdentifierGeneratorSupport.java:
##########
@@ -0,0 +1,68 @@
+package liquibase.ext.hibernate.snapshot;
+
+import liquibase.Scope;
+import org.hibernate.boot.model.relational.SqlStringGenerationContext;
+import
org.hibernate.boot.model.relational.internal.SqlStringGenerationContextImpl;
+import org.hibernate.generator.Generator;
+import org.hibernate.id.NativeGenerator;
+import org.hibernate.mapping.GeneratorSettings;
+import org.hibernate.mapping.SimpleValue;
+
+/**
+ * Shared helpers for the snapshot generators that inspect identifier
generators.
+ */
+final class IdentifierGeneratorSupport {
+
+ private IdentifierGeneratorSupport() {
+ }
+
+ /**
+ * For annotation-based entities an identifier without {@code
@GeneratedValue} is application-assigned, so no
+ * generator applies. XML-mapped entities have no member details and
declare their generator in the hbm.xml
+ * mapping, so they always count as having generation intent.
+ */
+ static boolean hasGenerationIntent(SimpleValue simpleValue) {
+ var memberDetails = simpleValue.getMemberDetails();
+ return memberDetails == null ||
+
memberDetails.hasDirectAnnotationUsage(jakarta.persistence.GeneratedValue.class);
Review Comment:
Fixed in 2a4a40a746. hasGenerationIntent uses GeneratorCreator.isAssigned()
when a creator is present, and a direct @NativeGenerator still counts as
generation intent. NativeGenEntity no longer carries a redundant
@GeneratedValue, and both native tests pass. @SnowflakeId without
@GeneratedValue is still not treated as a database-generated id.
##########
grails-data-hibernate7/dbmigration-core/src/main/java/liquibase/ext/hibernate/snapshot/HibernateSequenceSnapshotGenerator.java:
##########
@@ -36,15 +48,69 @@ protected void addTo(DatabaseObject foundObject,
DatabaseSnapshot snapshot)
if (foundObject instanceof Schema schema) {
HibernateDatabase database = (HibernateDatabase)
snapshot.getDatabase();
+ Set<String> addedSequences = new HashSet<>();
+
for (org.hibernate.boot.model.relational.Namespace namespace :
database.getMetadata().getDatabase().getNamespaces()) {
for (org.hibernate.boot.model.relational.Sequence sequence :
namespace.getSequences()) {
+ String name =
sequence.getName().getSequenceName().getText();
schema.addDatabaseObject(new Sequence()
-
.setName(sequence.getName().getSequenceName().getText())
+ .setName(name)
.setSchema(schema)
.setStartValue(BigInteger.valueOf(sequence.getInitialValue()))
.setIncrementBy(BigInteger.valueOf(sequence.getIncrementSize())));
+ addedSequences.add(name.toLowerCase(Locale.ROOT));
+ }
+ }
+
+ addGeneratorSequences(database, schema, addedSequences);
+ }
+ }
+
+ private void addGeneratorSequences(HibernateDatabase database, Schema
schema, Set<String> addedSequences) {
+ MetadataImplementor metadata = (MetadataImplementor)
database.getMetadata();
+ var dialect = database.getDialect();
+
+ for (PersistentClass entityBinding : metadata.getEntityBindings()) {
+ if (!(entityBinding instanceof RootClass rootClass) ||
+ !(rootClass.getIdentifier() instanceof SimpleValue
simpleValue) ||
+
!IdentifierGeneratorSupport.hasGenerationIntent(simpleValue)) {
+ continue;
+ }
+
+ try {
+ var generator = simpleValue.createGenerator(
+ dialect,
+ rootClass,
+ rootClass.getIdentifierProperty(),
+
IdentifierGeneratorSupport.createGeneratorSettings(simpleValue));
+
+ SequenceStyleGenerator seqGen = null;
+ // NativeGenerator may wrap a SequenceStyleGenerator delegate
depending on the dialect.
+ if (generator instanceof NativeGenerator nativeGen) {
+ if (IdentifierGeneratorSupport.nativeDelegate(nativeGen)
instanceof SequenceStyleGenerator s) {
+ seqGen = s;
+ }
+ } else if (generator instanceof SequenceStyleGenerator s) {
+ seqGen = s;
+ }
+
+ if (seqGen != null) {
+ var structure = seqGen.getDatabaseStructure();
+ if (structure != null && structure.getPhysicalName() !=
null) {
Review Comment:
Fixed in 2a4a40a746. Table-backed SequenceStyleGenerator structures are not
recorded as sequences. MySQL emulation of the auction package and an explicit
force_table_use mapping are both covered.
##########
grails-data-hibernate7/dbmigration-core/src/main/java/liquibase/ext/hibernate/diff/HibernateChangedIndexChangeGenerator.java:
##########
@@ -0,0 +1,41 @@
+package liquibase.ext.hibernate.diff;
+
+import liquibase.change.Change;
+import liquibase.database.Database;
+import liquibase.diff.ObjectDifferences;
+import liquibase.diff.output.DiffOutputControl;
+import liquibase.diff.output.changelog.ChangeGeneratorChain;
+import liquibase.ext.hibernate.database.HibernateDatabase;
+import liquibase.structure.DatabaseObject;
+import liquibase.structure.core.Index;
+
+/**
+ * Hibernate does not know every index attribute ({@code unique}, {@code
using}), so those differences are
+ * suppressed to prevent needless drop and recreate changes on every diff.
+ */
+public class HibernateChangedIndexChangeGenerator
+ extends
liquibase.diff.output.changelog.core.ChangedIndexChangeGenerator {
+
+ @Override
+ public int getPriority(Class<? extends DatabaseObject> objectType,
Database database) {
+ return Index.class.isAssignableFrom(objectType) ? PRIORITY_ADDITIONAL
: PRIORITY_NONE;
+ }
+
+ @Override
+ public Change[] fixChanged(
+ DatabaseObject changedObject,
+ ObjectDifferences differences,
+ DiffOutputControl control,
+ Database referenceDatabase,
+ Database comparisonDatabase,
+ ChangeGeneratorChain chain) {
+ if (referenceDatabase instanceof HibernateDatabase ||
comparisonDatabase instanceof HibernateDatabase) {
+ differences.removeDifference("unique");
Review Comment:
Fixed in 2a4a40a746. Index uniqueness now comes from
hibernateIndex.isUnique(), not Column.isUnique() and not a fabricated false for
composite indexes. A concrete Boolean mismatch is still kept. Unknown
uniqueness and the using attribute are still suppressed when a
HibernateDatabase is in the diff.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
