jamesfredley commented on code in PR #16497:
URL: https://github.com/apache/grails-core/pull/16497#discussion_r4179350208
##########
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);
Review Comment:
Fixed in 2a4a40a746. handleSequenceGenerator returns before
resolveSequenceName when DatabaseStructure is null, so the unconfigured GORM
native delegate cannot NPE. The PostgreSQL GORM spec covers the public snapshot
path.
##########
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();
Review Comment:
Fixed in 2a4a40a746. The sequence and unresolved-delegate branches no longer
return database.supportsAutoIncrement(). Sequence and table generators return
false. Only identity is auto-increment. IdentifierGeneratorSnapshotTest covers
Oracle native sequence.
##########
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();
Review Comment:
The duplicate is fixed in 2a4a40a746: a sequence already recorded as qs_seq
is not added again as app.qs_seq. The snapshot has one sequence, named qs_seq,
on the synthetic HIBERNATE schema, so the default diff still emits one
create-sequence change.
I did not attach the physical schema (app) to the Liquibase object.
HibernateDatabase.getDefaultSchemaName() is HIBERNATE, and
StandardDiffGenerator filters to that schema. Putting the sequence in schema
app removes it from the changelog entirely.
Two sequences that share a bare name in different physical schemas still
collapse under that synthetic schema. That is a limitation of this extension's
diff model, not a second copy of the same sequence.
--
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]
