Improve snapshot extensibility for NoSQL databases #5619

fbiville · 2024-02-21T16:05:45Z

Impact

Bug fix (non-breaking change which fixes expected existing functionality)
Enhancement/New feature (adds functionality without impacting existing logic)
Breaking change (fix or feature that would cause existing functionality to change)

Pro companion PR: https://github.com/liquibase/liquibase-pro/pull/1605
Pro-tests companion PR: https://github.com/liquibase/liquibase-pro-tests/pull/1322

Things to be aware of

Fixes SnapshotGeneratorChain
Improves Database.supports methods

Having a generic method as

 default boolean supports(Class<? extends DatabaseObject> object) {

allows extensions (specially NoSql ones) to implement validations as

supports(Table.class) && supports(Catalog.class)

without changing core to add a new "supportsTables" method.

Things to worry about

TODO

liquibase-standard/src/main/java/liquibase/snapshot/jvm/ViewSnapshotGenerator.java

liquibase-standard/src/main/java/liquibase/snapshot/SnapshotGeneratorChain.java

liquibase-integration-tests/src/test/java/liquibase/dbtest/AbstractIntegrationTest.java

liquibase-standard/src/main/java/liquibase/snapshot/SnapshotControl.java

liquibase-standard/src/main/java/liquibase/snapshot/SnapshotGeneratorChain.java

liquibase-standard/src/test/groovy/liquibase/snapshot/jvm/ViewSnapshotGeneratorTest.groovy

liquibase-standard/src/main/java/liquibase/database/core/HsqlDatabase.java

That way, snapshot generators are strongly advised to add to existing snapshot instances, instead of creating new ones. For built-in generators that do not comply to this, the only option is to replace them, as suggested by the error message.

The only difference with before is that Schema gets filtered out since MySQL does not support it.

This reverts commit 317c141.

jasonlyle88 · 2024-05-03T14:14:03Z

Hello @fbiville and @filipelautert ,

As of this PR, I am no longer able to run mvn clean install on the master branch of the project. I did some testing, and the commit right before this (58e9c3d09c3db57cd149bb27dba19539327248d8) allows me to run mvn clean install just fine on the project. As of this commit (2b1c363e7f998bcdb1e4a6c66906a0e2c2226f77), it no longer works.

When running mvn clean install I get an error on liquibase-integration-tests for the hsqldb database type. Deleting the liquibase-integration-tests/src/test/java/liquibase/dbtest/hsqldb/HsqlIntegrationTest.java file allows mvn clean install to run correctly... but obviously that isn't a real solution. Attached is the liquibase.dbtest.hsqldb.HsqlIntegrationTest.txt file that is generated and contains the failures. Below is also the end of the output of the maven run if it is also helpful:

[INFO]
[INFO] Results:
[INFO]
[ERROR] Errors:
[ERROR]   HsqlIntegrationTest>AbstractIntegrationTest.testClearChecksums:843 » CommandExecution liquibase.exception.LiquibaseException: liquibase.exception.MigrationFailedException: Migration failed for changeset changelogs/hsqldb/complete/root.changelog.xml::1.1::nvoxland:
     Reason: liquibase.exception.DatabaseException: object name already exists: SEQ_PERSON in statement [CREATE SEQUENCE PUBLIC.seq_person AS BIGINT] [Failed SQL: (-5504) CREATE SEQUENCE PUBLIC.seq_person AS BIGINT]
[ERROR]   HsqlIntegrationTest>AbstractIntegrationTest.testDbDoc:926 » CommandExecution liquibase.exception.LiquibaseException: liquibase.exception.MigrationFailedException: Migration failed for changeset changelogs/hsqldb/complete/root.changelog.xml::1.1::nvoxland:
     Reason: liquibase.exception.DatabaseException: object name already exists: SEQ_PERSON in statement [CREATE SEQUENCE PUBLIC.seq_person AS BIGINT] [Failed SQL: (-5504) CREATE SEQUENCE PUBLIC.seq_person AS BIGINT]
[ERROR]   HsqlIntegrationTest>AbstractIntegrationTest.testDiff:638->AbstractIntegrationTest.runCompleteChangeLog:356->AbstractIntegrationTest.runChangeLogFile:368 » CommandExecution liquibase.exception.LiquibaseException: liquibase.exception.MigrationFailedException: Migration failed for changeset changelogs/hsqldb/complete/root.changelog.xml::1.1::nvoxland:
     Reason: liquibase.exception.DatabaseException: object name already exists: SEQ_PERSON in statement [CREATE SEQUENCE PUBLIC.seq_person AS BIGINT] [Failed SQL: (-5504) CREATE SEQUENCE PUBLIC.seq_person AS BIGINT]
[ERROR]   HsqlIntegrationTest>AbstractIntegrationTest.testGenerateChangeLogWithNoChanges:1066->AbstractIntegrationTest.runCompleteChangeLog:356->AbstractIntegrationTest.runChangeLogFile:368 » CommandExecution liquibase.exception.LiquibaseException: liquibase.exception.MigrationFailedException: Migration failed for changeset changelogs/hsqldb/complete/root.changelog.xml::1.1::nvoxland:
     Reason: liquibase.exception.DatabaseException: object name already exists: SEQ_PERSON in statement [CREATE SEQUENCE PUBLIC.seq_person AS BIGINT] [Failed SQL: (-5504) CREATE SEQUENCE PUBLIC.seq_person AS BIGINT]
[ERROR]   HsqlIntegrationTest>AbstractIntegrationTest.testRerunDiffChangeLog:660->AbstractIntegrationTest.runCompleteChangeLog:356->AbstractIntegrationTest.runChangeLogFile:368 » CommandExecution liquibase.exception.LiquibaseException: liquibase.exception.MigrationFailedException: Migration failed for changeset changelogs/hsqldb/complete/root.changelog.xml::1.1::nvoxland:
     Reason: liquibase.exception.DatabaseException: object name already exists: SEQ_PERSON in statement [CREATE SEQUENCE PUBLIC.seq_person AS BIGINT] [Failed SQL: (-5504) CREATE SEQUENCE PUBLIC.seq_person AS BIGINT]
[ERROR]   HsqlIntegrationTest>AbstractIntegrationTest.testRollbackToChange:902 » CommandExecution liquibase.exception.LiquibaseException: liquibase.exception.MigrationFailedException: Migration failed for changeset changelogs/hsqldb/rollback/rollbackable.changelog.xml::8a::nvoxland:
     Reason: liquibase.exception.DatabaseException: object name already exists: SEQ_PUBLICATION_ID in statement [CREATE SEQUENCE PUBLIC.seq_publication_id AS BIGINT] [Failed SQL: (-5504) CREATE SEQUENCE PUBLIC.seq_publication_id AS BIGINT]
[ERROR]   HsqlIntegrationTest>AbstractIntegrationTest.testRowCountPrecondition:1145->AbstractIntegrationTest.runChangeLogFile:368 » CommandExecution liquibase.exception.LiquibaseException: liquibase.exception.MigrationFailedException: Migration failed for changeset changelogs/common/common.tests.changelog.xml::seq-test::nvoxland:
     Reason: liquibase.exception.DatabaseException: object name already exists: SEQTESTONALL in statement [CREATE SEQUENCE PUBLIC.seqtestonall AS BIGINT] [Failed SQL: (-5504) CREATE SEQUENCE PUBLIC.seqtestonall AS BIGINT]
[ERROR]   HsqlIntegrationTest>AbstractIntegrationTest.testRunUpdateOnOldChangelogTableFormat:423 » CommandExecution liquibase.exception.LiquibaseException: liquibase.exception.MigrationFailedException: Migration failed for changeset changelogs/hsqldb/complete/root.changelog.xml::1.1::nvoxland:
     Reason: liquibase.exception.DatabaseException: object name already exists: SEQ_PERSON in statement [CREATE SEQUENCE PUBLIC.seq_person AS BIGINT] [Failed SQL: (-5504) CREATE SEQUENCE PUBLIC.seq_person AS BIGINT]
[ERROR]   HsqlIntegrationTest>AbstractIntegrationTest.testSnapshotReportsAllObjectTypes:329->AbstractIntegrationTest.runCompleteChangeLog:356->AbstractIntegrationTest.runChangeLogFile:368 » CommandExecution liquibase.exception.LiquibaseException: liquibase.exception.MigrationFailedException: Migration failed for changeset changelogs/hsqldb/complete/root.changelog.xml::1.1::nvoxland:
     Reason: liquibase.exception.DatabaseException: object name already exists: SEQ_PERSON in statement [CREATE SEQUENCE PUBLIC.seq_person AS BIGINT] [Failed SQL: (-5504) CREATE SEQUENCE PUBLIC.seq_person AS BIGINT]
[ERROR]   HsqlIntegrationTest>AbstractIntegrationTest.testTableExistsPreconditionTableNameMatch:1111->AbstractIntegrationTest.runChangeLogFile:368 » CommandExecution liquibase.exception.LiquibaseException: liquibase.exception.MigrationFailedException: Migration failed for changeset changelogs/common/common.tests.changelog.xml::seq-test::nvoxland:
     Reason: liquibase.exception.DatabaseException: object name already exists: SEQTESTONALL in statement [CREATE SEQUENCE PUBLIC.seqtestonall AS BIGINT] [Failed SQL: (-5504) CREATE SEQUENCE PUBLIC.seqtestonall AS BIGINT]
[ERROR]   HsqlIntegrationTest>AbstractIntegrationTest.testTableIsEmptyPrecondition:1121->AbstractIntegrationTest.runChangeLogFile:368 » CommandExecution liquibase.exception.LiquibaseException: liquibase.exception.MigrationFailedException: Migration failed for changeset changelogs/common/common.tests.changelog.xml::seq-test::nvoxland:
     Reason: liquibase.exception.DatabaseException: object name already exists: SEQTESTONALL in statement [CREATE SEQUENCE PUBLIC.seqtestonall AS BIGINT] [Failed SQL: (-5504) CREATE SEQUENCE PUBLIC.seqtestonall AS BIGINT]
[ERROR]   HsqlIntegrationTest>AbstractIntegrationTest.testTag:628 » CommandExecution liquibase.exception.LiquibaseException: liquibase.exception.MigrationFailedException: Migration failed for changeset changelogs/hsqldb/complete/root.changelog.xml::1.1::nvoxland:
     Reason: liquibase.exception.DatabaseException: object name already exists: SEQ_PERSON in statement [CREATE SEQUENCE PUBLIC.seq_person AS BIGINT] [Failed SQL: (-5504) CREATE SEQUENCE PUBLIC.seq_person AS BIGINT]
[ERROR]   HsqlIntegrationTest>AbstractIntegrationTest.testTagEmptyDatabase:861 » CommandExecution liquibase.exception.LiquibaseException: liquibase.exception.MigrationFailedException: Migration failed for changeset changelogs/hsqldb/rollback/rollbackable.changelog.xml::8a::nvoxland:
     Reason: liquibase.exception.DatabaseException: object name already exists: SEQ_PUBLICATION_ID in statement [CREATE SEQUENCE PUBLIC.seq_publication_id AS BIGINT] [Failed SQL: (-5504) CREATE SEQUENCE PUBLIC.seq_publication_id AS BIGINT]
[ERROR]   HsqlIntegrationTest>AbstractIntegrationTest.testUpdateChangelogChecksum:1201 » CommandExecution liquibase.exception.LiquibaseException: liquibase.exception.MigrationFailedException: Migration failed for changeset changelogs/hsqldb/complete/included.changelog.xml::1.2::nvoxland:
     Reason: liquibase.exception.DatabaseException: object name already exists: SEQ_NEWS in statement [CREATE SEQUENCE PUBLIC.seq_news AS BIGINT] [Failed SQL: (-5504) CREATE SEQUENCE PUBLIC.seq_news AS BIGINT]
[ERROR]   HsqlIntegrationTest>AbstractIntegrationTest.testUpdateClearUpdate:557 » CommandExecution liquibase.exception.LiquibaseException: liquibase.exception.MigrationFailedException: Migration failed for changeset changelogs/hsqldb/complete/root.changelog.xml::1.1::nvoxland:
     Reason: liquibase.exception.DatabaseException: object name already exists: SEQ_PERSON in statement [CREATE SEQUENCE PUBLIC.seq_person AS BIGINT] [Failed SQL: (-5504) CREATE SEQUENCE PUBLIC.seq_person AS BIGINT]
[ERROR]   HsqlIntegrationTest>AbstractIntegrationTest.testUpdateTwice:543 » CommandExecution liquibase.exception.LiquibaseException: liquibase.exception.MigrationFailedException: Migration failed for changeset changelogs/hsqldb/complete/root.changelog.xml::1.1::nvoxland:
     Reason: liquibase.exception.DatabaseException: object name already exists: SEQ_PERSON in statement [CREATE SEQUENCE PUBLIC.seq_person AS BIGINT] [Failed SQL: (-5504) CREATE SEQUENCE PUBLIC.seq_person AS BIGINT]
[INFO]
[ERROR] Tests run: 1137, Failures: 0, Errors: 16, Skipped: 602
[INFO]
[INFO] ------------------------------------------------------------------------
[INFO] Reactor Summary for Liquibase Parent 0-SNAPSHOT:
[INFO]
[INFO] Liquibase Parent ................................... SUCCESS [ 42.811 s]
[INFO] liquibase-standard ................................. SUCCESS [02:00 min]
[INFO] liquibase-cli ...................................... SUCCESS [  9.873 s]
[INFO] liquibase-snowflake ................................ SUCCESS [  9.132 s]
[INFO] Liquibase .......................................... SUCCESS [  1.644 s]
[INFO] liquibase-maven-plugin ............................. SUCCESS [ 21.132 s]
[INFO] liquibase-cdi-jakarta .............................. SUCCESS [ 11.239 s]
[INFO] liquibase-cdi ...................................... SUCCESS [  9.956 s]
[INFO] liquibase-extension-testing ........................ SUCCESS [ 11.557 s]
[INFO] liquibase-integration-tests ........................ FAILURE [01:28 min]
[INFO] liquibase-dist ..................................... SKIPPED
[INFO] ------------------------------------------------------------------------
[INFO] BUILD FAILURE
[INFO] ------------------------------------------------------------------------
[INFO] Total time:  05:26 min
[INFO] Finished at: 2024-05-02T17:58:29-04:00
[INFO] ------------------------------------------------------------------------
[ERROR] Failed to execute goal org.apache.maven.plugins:maven-surefire-plugin:3.2.5:test (default-test) on project liquibase-integration-tests:
[ERROR]
[ERROR] Please refer to /Users/jlyle/temp/liquibase/liquibase-integration-tests/target/surefire-reports for the individual test results.
[ERROR] Please refer to dump files (if any exist) [date].dump, [date]-jvmRun[N].dump and [date].dumpstream.
[ERROR] -> [Help 1]
[ERROR]
[ERROR] To see the full stack trace of the errors, re-run Maven with the -e switch.
[ERROR] Re-run Maven using the -X switch to enable full debug logging.
[ERROR]
[ERROR] For more information about the errors and possible solutions, please read the following articles:
[ERROR] [Help 1] http://cwiki.apache.org/confluence/display/MAVEN/MojoFailureException
[ERROR]
[ERROR] After correcting the problems, you can resume the build with the command
[ERROR]   mvn <args> -rf :liquibase-integration-tests

Any thoughts on what is going on here?

fbiville · 2024-05-06T08:20:58Z

@jasonlyle88 I initially thought I missed porting supportsSequences to supports(Class<? extends DatabaseObject>) in liquibase.database.core.HsqlDatabase, but I don't think that's the issue.

supports is implemented like this:

    @Override
    public boolean supports(Class<? extends DatabaseObject> object) {
        if (Catalog.class.isAssignableFrom(object)) { // this is the same impl as supportsCatalog
            try {
                return getDatabaseMajorVersion() >= 2;
            } catch (DatabaseException e) {
                return true;
            }
        }
        return super.supports(object);
    }

This PR changed all supportsSequences calls to supports(Sequence.class).
In that case, super.supports is going to be called, and it's going to return true which is 100% equivalent to calling supportsSequences. 🤔

fbiville requested a review from filipelautert as a code owner February 21, 2024 16:05

fbiville temporarily deployed to external February 21, 2024 16:05 — with GitHub Actions Inactive

fbiville commented Feb 21, 2024

View reviewed changes

liquibase-standard/src/main/java/liquibase/snapshot/jvm/ViewSnapshotGenerator.java Show resolved Hide resolved

fbiville commented Feb 21, 2024

View reviewed changes

liquibase-standard/src/main/java/liquibase/snapshot/jvm/ViewSnapshotGenerator.java Outdated Show resolved Hide resolved

filipelautert self-assigned this Feb 21, 2024

filipelautert added the enhancement label Feb 21, 2024

filipelautert approved these changes Feb 21, 2024

View reviewed changes

filipelautert requested review from rberezen and suryaaki2 February 21, 2024 21:37

fbiville marked this pull request as draft February 22, 2024 09:43

fbiville had a problem deploying to external February 22, 2024 15:13 — with GitHub Actions Error

fbiville had a problem deploying to external February 22, 2024 15:22 — with GitHub Actions Error

fbiville force-pushed the database_view_support branch from 3731fe6 to 7ff5885 Compare February 22, 2024 15:28

fbiville had a problem deploying to external February 22, 2024 15:28 — with GitHub Actions Error

fbiville changed the title ~~Defer view support to database~~ Improve snapshot extensibility for NoSQL databases Feb 22, 2024

fbiville had a problem deploying to external February 22, 2024 16:01 — with GitHub Actions Error

fbiville had a problem deploying to external February 22, 2024 16:30 — with GitHub Actions Error

fbiville requested a review from filipelautert February 22, 2024 16:33

fbiville force-pushed the database_view_support branch from c86bbdc to cb89af7 Compare February 22, 2024 16:38

fbiville temporarily deployed to external February 22, 2024 16:38 — with GitHub Actions Inactive

filipelautert added the TypeEnhancement label Feb 22, 2024

github-advanced-security bot found potential problems Feb 22, 2024

View reviewed changes

liquibase-standard/src/main/java/liquibase/snapshot/SnapshotGeneratorChain.java Fixed Show fixed Hide fixed

fbiville commented Feb 22, 2024

View reviewed changes

fbiville temporarily deployed to external February 23, 2024 10:00 — with GitHub Actions Inactive

fbiville commented Feb 23, 2024

View reviewed changes

liquibase-standard/src/main/java/liquibase/database/core/HsqlDatabase.java Outdated Show resolved Hide resolved

fbiville had a problem deploying to external February 23, 2024 12:01 — with GitHub Actions Error

fbiville temporarily deployed to external February 23, 2024 12:07 — with GitHub Actions Inactive

filipelautert removed request for rberezen and suryaaki2 February 23, 2024 12:15

fbiville temporarily deployed to external February 23, 2024 13:15 — with GitHub Actions Inactive

fbiville and others added 14 commits April 2, 2024 16:36

Bypass generators when database does not support the capability

0417bd2

Do not include unsupported types of snapshot objects

664e61d

Rewrite chain snapshot to actually invoke the whole chain

5f03e5f

Fix HSQL schema support

5a3cddb

Delegate to super for MySQL

ce87fa9

Rewrite test with more accurate assertions

288d29a

The only difference with before is that Schema gets filtered out since MySQL does not support it.

Add coverage for snapshot generator chain

93ff721

Allow null database

433b57e

Reinstate null check

6483a4b

Add missing super calls for supports

a255a61

fix: firebird workaround + sonar

412e64e

fix: this validation breaks function snapshot capabilities

11bc22b

Revert "fix: this validation breaks function snapshot capabilities"

26a27e4

This reverts commit 317c141.

filipelautert force-pushed the database_view_support branch from d906185 to 26a27e4 Compare April 2, 2024 19:36

filipelautert temporarily deployed to external April 2, 2024 19:36 — with GitHub Actions Inactive

filipelautert requested a deployment to external April 3, 2024 14:03 — with GitHub Actions Abandoned

Merge branch 'master' into database_view_support

47742b2

rberezen temporarily deployed to external April 3, 2024 16:48 — with GitHub Actions Inactive

rberezen requested a deployment to external April 3, 2024 17:03 — with GitHub Actions Abandoned

rberezen requested a deployment to external April 3, 2024 17:21 — with GitHub Actions Abandoned

filipelautert added this to the 1NEXT milestone Apr 4, 2024

filipelautert merged commit 2b1c363 into liquibase:master Apr 4, 2024

fbiville deleted the database_view_support branch April 4, 2024 16:35

filipelautert mentioned this pull request Apr 5, 2024

feat: improve snapshot generation for NOSQL databases #5775

Merged

jasonlyle88 mentioned this pull request May 3, 2024

Changeset execution information properties #5598

Merged

3 tasks

filipelautert added the breakingChanges label May 10, 2024

filipelautert mentioned this pull request May 10, 2024

chore: invoke deprecated methods in default supports method for backwards compatibility #5897

Merged

3 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Improve snapshot extensibility for NoSQL databases #5619

Improve snapshot extensibility for NoSQL databases #5619

Uh oh!

fbiville commented Feb 21, 2024 •

edited by filipelautert

Loading

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

jasonlyle88 commented May 3, 2024

Uh oh!

fbiville commented May 6, 2024 •

edited

Loading

Uh oh!

Uh oh!

Improve snapshot extensibility for NoSQL databases #5619

Improve snapshot extensibility for NoSQL databases #5619

Uh oh!

Conversation

fbiville commented Feb 21, 2024 • edited by filipelautert Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Impact

Things to be aware of

Things to worry about

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

jasonlyle88 commented May 3, 2024

Uh oh!

fbiville commented May 6, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

fbiville commented Feb 21, 2024 •

edited by filipelautert

Loading

fbiville commented May 6, 2024 •

edited

Loading