Support Flyway file ordering in DDLDatabase

[yuecelm]

Expected behavior and actual behavior:

Flyway SQL scripts are imported in the wrong order into DDLDatabase (with sort=semantic)

excepted:
src/main/resources/db/migration/V1__init_schema.sql
src/main/resources/db/migration/V1.1__add_indexes.sql

actual:
src/main/resources/db/migration/V1.1__add_indexes.sql <- triggers sql error due to missing table
src/main/resources/db/migration/V1__init_schema.sql

Steps to reproduce the problem (if possible, create an MCVE: https://github.com/jOOQ/jOOQ-mcve):

src/main/resources/db/migration/V1__init_schema.sql:

CREATE TABLE company (
  id UUID PRIMARY KEY,
  name VARCHAR NOT NULL,
  created_at TIMESTAMP,
  updated_at TIMESTAMP
);

src/main/resources/db/migration/V1.1__add_indexes.sql:

CREATE INDEX company_index_name ON company (name);

pom.xml:

                <database>
                  <name>org.jooq.meta.extensions.ddl.DDLDatabase</name>
                  <inputCatalog></inputCatalog>
                  <inputSchema>PUBLIC</inputSchema>
                  <outputSchemaToDefault>true</outputSchemaToDefault>
                  <outputCatalogToDefault>true</outputCatalogToDefault>
                  <properties>
                    <property>
                      <key>sort</key>
                      <value>semantic</value>
                    </property>
                    <property>
                      <key>scripts</key>
                      <value>src/main/resources/db/migration/*</value>
                    </property>
                  </properties>
                </database>

Versions:

• jOOQ: 3.11.11
• Java: OpenJDK 1.8.0_212
• Database (include vendor): org.jooq.meta.extensions.ddl.DDLDatabase
• OS: Ubuntu 18.04.2 LTS

[yuecelm]

I would suggest to introduce sort=flyway for org.jooq.meta.extensions.ddl.DDLDatabase if the actual order is intended with sort=semantic

[yuecelm]

I am also willing to implement int compare(String s1, String s2) for the new sort

[yuecelm]

https://flywaydb.org/documentation/migrations#naming

[lukaseder]

Thank you very much for your suggestion.

I thought there was an existing feature request for this, but I cannot find it. I definitely agree Flyway file ordering should be supported, although I'd say that's a new feature, not a bug :-)

This new ordering would be implemented when sort=flyway, so it could be implemented completely in org.jooq.meta.tools.FilePattern.

I am also willing to implement int compare(String s1, String s2) for the new sort

That's greatly appreciated. I think this is a very local change, so a PR is definitely feasible. May I ask you to review our contribution guidelines: https://github.com/jOOQ/jOOQ/blob/master/CONTRIBUTING.md

I'll be very happy to assist you with this change.

[yuecelm]

I can do a PR. First question: what shall we do with SQL scripts that are not fitting in the flyway naming pattern (e.g. not containing a version)? throw an exception, return null, or ignore the file completely?

[yuecelm]

My first implementation would look like this: ignore first character, then I expect the version until 2 underscores. In the version I would interpret any non-number as dot.

[lukaseder]

I can do a PR. First question: what shall we do with SQL scripts that are not fitting in the flyway naming pattern (e.g. not containing a version)? throw an exception, return null, or ignore the file completely?

I think that we don't have to implement this functionality, as we're not going to replace Flyway. Flyway will have validated the migration files already in the build pipeline, so this is only about producing the same order.

My first implementation would look like this: ignore first character, then I expect the version until 2 underscores. In the version I would interpret any non-number as dot.

Can we reuse some Flyway code? Flyway's Open Source version is ASL 2.0 licensed, which is compatible with jOOQ's ASL 2.0. With proper attribution and if retaining Flyway copyright, we should be able to reuse their logic.

[yuecelm]

Can we reuse some Flyway code? Flyway's Open Source version is ASL 2.0 licensed, which is compatible with jOOQ's ASL 2.0. With proper attribution and if retaining Flyway copyright, we should be able to reuse their logic.

I think we can use this as starting point: https://github.com/flyway/flyway/blob/master/flyway-core/src/main/java/org/flywaydb/core/api/MigrationVersion.java
is a maven dependancy against flyway-core also acceptable?

[lukaseder]

I think we can use this as starting point

That does look very useful, indeed

is a maven dependancy against flyway-core also acceptable?

I'd prefer not to add one. Instead, we can manually shade the class into a jOOQ package (which seems to have no other dependencies apart from FlywayException, which we don't need)

[yuecelm]

Just wondering, I wrote a test with Flyway/DDLDatabase and the following sql files:

src/main/resources/db/migration/V1__initialise_database.sql
src/main/resources/db/migration/V1.2__create_author_table.sql
src/main/resources/db/migration/V1_3__create_book_table_and_add_records.sql

with sort=semantic it fails as expected, but when I do sort=none it succeed IMO unexpected. Do you have an idea why?

[lukaseder]

sort=none just means that DDLDatabase will use the order produced by your operating system when doing File.list() and similar calls. This may accidentally match a specific ordering.

You cannot rely on sort=none orderings in tests as they can be reasonably be expected to be arbitrary, which is difficult to test.

[yuecelm]

PR: #8976

[lukaseder]

Done. Thanks again for your contribution!