change at 4.19.1 makes includeAll is finding duplicates #3881

bmoregeo · 2023-03-01T21:32:44Z

Search first

I searched and no similar issues were found

Description

Granted our change log is pretty janky due to some not great decisions when we started out with this repo, but it was working with 4.19.0. The update with 4.19.1 is now raising an exception that we have duplicate changelogs defined. Previously the final changelog lookup would skip over the existing changelogs that were defined.

<?xml version="1.0" encoding="UTF-8"?>
<databaseChangeLog
   xmlns="http://www.liquibase.org/xml/ns/dbchangelog"
   xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"
   xmlns:pro="http://www.liquibase.org/xml/ns/pro"
   xsi:schemaLocation="http://www.liquibase.org/xml/ns/dbchangelog
      http://www.liquibase.org/xml/ns/dbchangelog/dbchangelog-4.1.xsd
      http://www.liquibase.org/xml/ns/pro
      http://www.liquibase.org/xml/ns/pro/liquibase-pro-4.1.xsd">
    <includeAll path="changelog/0.0/"/>
    <includeAll path="changelog/0.1/"/>
    <includeAll path="changelog/0.2/"/>
    <includeAll path="changelog/0.3/"/>
    <includeAll path="changelog/0.4/"/>
    <includeAll path="changelog/0.5/"/>
    <includeAll path="changelog/0.6/"/>
    <includeAll path="changelog/0.7/"/>
    <includeAll path="changelog/0.8/"/>
    <includeAll path="changelog/0.9/"/>
    <includeAll path="changelog/1.0/"/>
    <includeAll path="changelog/1.1/"/>
    <includeAll path="changelog/1.2/"/>
    <includeAll path="changelog/1.3/"/>
    <includeAll path="changelog/1.4/"/>
    <includeAll path="changelog/1.5/"/>
    <includeAll path="changelog/1.6/"/>
    <includeAll path="changelog/1.7/"/>
    <includeAll path="changelog/1.8/"/>
    <includeAll path="changelog/1.9/"/>
    <includeAll path="changelog/"/>

</databaseChangeLog>

Steps To Reproduce

Create a change log with a specific folder defined
Create a change log that also includes that folder and at least one other folder

<?xml version="1.0" encoding="UTF-8"?>
<databaseChangeLog
   xmlns="http://www.liquibase.org/xml/ns/dbchangelog"
   xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"
   xmlns:pro="http://www.liquibase.org/xml/ns/pro"
   xsi:schemaLocation="http://www.liquibase.org/xml/ns/dbchangelog
      http://www.liquibase.org/xml/ns/dbchangelog/dbchangelog-4.1.xsd
      http://www.liquibase.org/xml/ns/pro
      http://www.liquibase.org/xml/ns/pro/liquibase-pro-4.1.xsd">

    <includeAll path="changelog/1.0/"/>
    <includeAll path="changelog/"/>

</databaseChangeLog>

Actual Behavior

####################################################
##   _     _             _ _                      ##
##  | |   (_)           (_) |                     ##
##  | |    _  __ _ _   _ _| |__   __ _ ___  ___   ##
##  | |   | |/ _` | | | | | '_ \ / _` / __|/ _ \  ##
##  | |___| | (_| | |_| | | |_) | (_| \__ \  __/  ##
##  \_____/_|\__, |\__,_|_|_.__/ \__,_|___/\___|  ##
##              | |                               ##
##              |_|                               ##
##                                                ## 
##  Get documentation at docs.liquibase.com       ##
##  Get certified courses at learn.liquibase.com  ## 
##  Free schema change activity reports at        ##
##      https://hub.liquibase.com/                 ##
##                                                ##
####################################################
Starting Liquibase at 20:12:35 (version 4.19.1 #7641 built at 2023-02-28 18:08+0000)
Liquibase Version: 4.19.1
Liquibase Open Source 4.19.1 by Liquibase
Validation Error: 
     85 changesets had duplicate identifiers
          changelog/1.8/changelog-00.sql::1.8.0::joe.shmo
          changelog/0.8/changelog-1.sql::raw::includeAll

Expected/Desired Behavior

Go back to how it was or document a fix moving forward?

Liquibase Version

4.19.1

Database Vendor & Version

postgresql

Liquibase Integration

Github Action

Liquibase Extensions

None

OS and/or Infrastructure Type/Provider

Ubuntu

Additional Context

No response

Are you willing to submit a PR?

I'm willing to submit a PR (Thank you!)

The text was updated successfully, but these errors were encountered:

hylkevds · 2023-03-02T08:07:27Z

It's not caused by includeAll. We're not using includeAll, but some of our change sets are also broken

In our case it was actually a mistake in an included changelog file that re-included the same files again. Apparently there used to be duplicate-detection that removed these duplicates.

filipelautert · 2023-03-02T20:38:46Z

Hello @bmoregeo , I believe that @hylkevds comment is right, it's not related to include all but to a performance fix introduced to how we load changesets. In fact allowing duplicated changesets to be processed was a bug and not a feature, but the bug lived so long that seems it became a feature - and as such it should be on a major release and with better documentation, sorry for that.
Anyway, just to confirm that we are hadling the correct issue before assuming more stuff: if you remove <includeAll path="changelog/"/> does it work? Of even better, if you remove all the other includes and keep just <includeAll path="changelog/"/> , does it fixes the duplicate warnings?

bmoregeo · 2023-03-03T18:21:39Z

@filipelautert, yes I'm sure it will work if I remove the includeAll.

My issue is that we originally named the changelogs poorly and this resulted in changesets being executed out of sequence. 1.10.0 was executing before 1.2.0 etc. The fix was to name all of the poorly named changesets into the root xml in the correct order and then use the catchall includeAll to get all the correctly formatted stuff.

I'll play around with the it and see if I can come up with a better solution given the new constraints. Thanks!

filipelautert · 2023-03-03T18:58:02Z

@bmoregeo thanks for the feedback! If you are willing to skip 4.19.1 , we are discussing about using "strict" flag on 4.20.0 to bypass it, as we were not expecting this issue with includes (ie none of our tests were addressing this case yet). The change may be: if strict is set then don't allow duplicates; otherwise use the old behavior.

bodewig · 2023-03-13T14:09:28Z

@filipelautert we just stumbled over this while trying to update from 4.19.0. We've had duplicated includes by accident in our case. The discussed strict option you talked about has not landed in 4.20.0 or has it?

In our case it is easy to fix by removing the redundant include, so it's not a big issue, but it took some time to figure out why it used to work in 4.19.0.

filipelautert · 2023-03-13T14:32:42Z

@bodewig not yet, it still in our dev queue. Hopefully it will be released in the next version.

XDelphiGrl · 2023-03-15T16:51:37Z

This ticket requires manual testing. Depending on developer tests and implementation, we may also need to add a functional test.

famod · 2023-07-26T11:40:21Z

Same here with Quarkus 3.2.2 that uses Liquibase 4.20.0, also on empty database (e.g. fresh mariadb testcontainer).
I don't see an actual duplication (still checking though).
Going back to 4.19.0 works around the problem.

famod · 2023-07-26T14:18:06Z

Turns out that #3790 clashes with includeAll always being fully recursive until 4.21.0, which introduced a maxDepth attribute: #3620

So if you have a changelog in folder x which includes other changelogs in subfolders of x and you say includeAll x from some other changelog, then there are of course duplicates.
The workaround until 4.21.0 is to move out those other included changelogs from the folder that is passed to includeAll.

nils-christian · 2023-10-24T07:26:20Z

Hi,

As far as I can tell, this issue does not only apply to includeAll, but also to the regular include.

Up to 4.19.0 the following was allowed:

changelog-main.xml includes changelog-1.xml and changelog-2.xml
changelog-1.xml defines a changeset (let's call it 001)
changelog-2.xml includes changelog-1.xml

From 4.20.0 on the startup fails, as 001 is recognized as a forbidden duplicated entry.

So I wonder if the fix of this issue will also restore the old behavior in regard of this situation. In my opinion this is a regression.

Best regards

Nils

nils-christian · 2023-11-25T08:13:35Z

Hi,

Any news or plans on this topic? With the current Spring Boot version (3.2.0) a downgrade of Liquibase to 4.19.0 is no longer possible, due to a ClassNotFoundException (e.g., liquibase.UpdateSummaryEnum). So this issue became a blocker for us.

Best regards

Nils

filipelautert · 2023-11-28T16:01:27Z

@nils-christian @famod thanks for the examples, let me see if I manage to reproduce it and find a fix.

filipelautert · 2023-11-29T14:55:01Z

@nils-christian @famod I created PR #5296 as a possible solution for this problem.
adding --allowDuplicatedChangesetIdentifiers=true to command line orliquibase.allowDuplicatedChangesetIdentifiers=trueto properties file would restore the old behavior. What do you think about it?

nils-christian · 2023-11-30T09:44:03Z

Sounds good to me, @filipelautert. Thank you.

Who would be responsible for adding this property to org.springframework.boot.autoconfigure.liquibase.LiquibaseProperties as well?

filipelautert · 2023-11-30T12:45:20Z

@nils-christian as this is a liquibase global parameter, it should work if you start springboot with parameter -Dliquibase.allowDuplicatedChangesetIdentifiers=true . It is possible to test it using the following package for this build:

<dependency>
  <groupId>org.liquibase</groupId>
  <artifactId>liquibase-core</artifactId>
  <version>fix-3881-by-adding-a-new-flag-SNAPSHOT</version>
</dependency>

And you will need to add github repository to your maven settings as explained here -> https://docs.github.com/en/packages/working-with-a-github-packages-registry/working-with-the-apache-maven-registry .

<activeProfiles>
    <activeProfile>github</activeProfile>
  </activeProfiles>

  <profiles>
    <profile>
      <id>github</id>
      <repositories>
        <repository>
          <id>central</id>
          <url>https://repo1.maven.org/maven2</url>
        </repository>
        <repository>
          <id>github</id>
          <url>https://maven.pkg.github.com/liquibase/liquibase</url>
          <snapshots>
            <enabled>true</enabled>
          </snapshots>
        </repository>
        <repository>
          <id>github</id>
          <url>https://maven.pkg.github.com/liquibase/liquibase-pro</url>
          <snapshots>
            <enabled>true</enabled>
          </snapshots>
        </repository>
      </repositories>
    </profile>
  </profiles>

  <servers>
    <server>
      <id>github</id>
      <username>your_gh_yusername</username>
      <password>the_token</password>
    </server>
  </servers>
</settings>

nils-christian · 2023-11-30T15:56:09Z

Hi @filipelautert,

Unfortunately, I just realized that the issue is a little bit deeper than we originally thought and that the fix does not solve it. If we modify my above example and let the changeset create a table named "A" we get an error during the execution, as the table exists already. This means the regression does not only affect the duplication check but also the actual execution.

4.19.0: Example works.
4.19.1: ValidationFailedException due to duplicates.
fix-3881-by-adding-a-new-flag-SNAPSHOT with -Dliquibase.allowDuplicatedChangesetIdentifiers=true: Changesets are executed multiple times.

So I think the regression is a little bit more serious than I originally thought.

Best regards,

Nils

filipelautert · 2023-12-04T18:03:34Z

@nils-christian ah, good catch. I found out the issue: I was ignoring the error and adding the duplicated changeset; now I fixed it and I'm ignoring the duplicated changeset thus not triggering the error.
I even managed to rollback the unit tests that we have prior to this change. Can you give it another try?

nils-christian · 2023-12-05T07:35:20Z

Hi @filipelautert,

Yes, this looks better. The example works now as expected, thank you very much.

mdimond · 2023-12-06T23:15:53Z

@filipelautert So has this truly been fixed? If not what is the workaround?

filipelautert · 2023-12-07T00:28:50Z

Hi @mdimond - we added a new flag that enables the old behavior. For command line you can use
New global flag: --allow-duplicated-changeset-identifiers=true , or -Dliquibase.allowDuplicatedChangesetIdentifiers=true as a command line parameter for Springboot .
Right now it's on master branch and it will be out in the next release (in 1-2 weeks).

mdimond · 2023-12-07T00:32:33Z

@filipelautert Just so I am clear I can add the following to my properties file:
liquibase.allowDuplicatedChangesetIdentifiers: true then the standard command line call liquibase.sh --defaults-file foo.properties? Outside of this my only other option is the remove the duplicates from the file?

filipelautert · 2023-12-07T12:51:29Z

@filipelautert Just so I am clear I can add the following to my properties file:

@mdimond yes, you are right.

RiVogel · 2023-12-18T10:58:49Z

Hi @mdimond - we added a new flag that enables the old behavior. For command line you can use New global flag: --allow-duplicated-changeset-identifiers=true , or -Dliquibase.allowDuplicatedChangesetIdentifiers=true as a command line parameter for Springboot . Right now it's on master branch and it will be out in the next release (in 1-2 weeks).

Hi @filipelautert: Can you estimate when this release will happen? :)

filipelautert · 2023-12-18T14:04:53Z

@RiVogel hopefully by today or tomorrow!

muzuro · 2023-12-20T14:35:03Z

Hello! Was this fix included in 4.25.1? I tried today to execute h2 tests with liqubase migrations and -Dliquibase.allowDuplicatedChangesetIdentifiers=true command line option. It failed with "Validation Failed: changesets had duplicate identifiers"

filipelautert · 2023-12-20T14:59:35Z

Hi @muzuro - yes, this has been included in 4.25.1 . If you are executing liquibase from the command line please try to use flag --allow-duplicated-changeset-identifiers=true instead of -D...

muzuro · 2023-12-21T12:40:49Z

@filipelautert I am actually executing spring boot tests with gradle. So command line looks like:
gradle -Dliquibase.allowDuplicatedChangesetIdentifiers=true :project:test.

I have tried to create reproduce project but it works even without new property, so I am little confused why it don't works at main project

UPD: I updated reproduce project with README and added test

Laurens-W · 2024-01-05T13:42:31Z

@filipelautert I don't think a global parameter is a great idea for those who are using the springboot integration of liquibase.

This means I have to add a command line param in 3 places: On my local machine, in my CI/CD environment and my runtime environment. Unless I made a mistake it seems setting liquibase.allowDuplicatedChangesetIdentifiers in application.properties or yaml equivalent doesn't work.

on a side note:

Why was this breaking behavior introduced in a minor version?
I've tried getting rid of the duplicate changeset identifiers by changing the structure of my changelog.

I have this folder structure:

- changesets
  - init

I'm including the init folder first:

  - includeAll:
      relativeToChangelogFile: true
      path: changesets/init

and after that I want to introduce the changesets directory:

  - includeAll:
      relativeToChangelogFile: true
      path: changesets

Obviously we can see how recursion would cause duplicate identifiers here, I expected setting min and maxDepth to 0 would fix this but instead it causes an error to be thrown regarding the changesets directory supposedly being empty.

muzuro · 2024-02-02T13:54:53Z

We can also set LIQUIBASE_ALLOW_DUPLICATED_CHANGESET_IDENTIFIERS env variable to true to enable this fix. My gradle tests worked only with env variable, -D option didn't worked ¯_(ツ)_/¯

filipelautert self-assigned this Mar 2, 2023

filipelautert added the sprint2023-46 label Mar 2, 2023

nvoxland mentioned this issue Mar 7, 2023

Performance Improvement: optimized DatabaseChangeLog.normalizePath() #3853

Merged

3 tasks

XDelphiGrl added the ManualTest label Mar 15, 2023

kevin-atx added the TypeBug label Mar 24, 2023

filipelautert mentioned this issue Nov 29, 2023

Global flag to restore behaviour of not failing on duplicated changeset identifiers #5296

Merged

3 tasks

filipelautert closed this as completed in #5296 Dec 6, 2023

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

change at 4.19.1 makes includeAll is finding duplicates #3881

change at 4.19.1 makes includeAll is finding duplicates #3881

bmoregeo commented Mar 1, 2023 •

edited by nvoxland

hylkevds commented Mar 2, 2023 •

edited

filipelautert commented Mar 2, 2023

bmoregeo commented Mar 3, 2023

filipelautert commented Mar 3, 2023

bodewig commented Mar 13, 2023

filipelautert commented Mar 13, 2023

XDelphiGrl commented Mar 15, 2023

famod commented Jul 26, 2023

famod commented Jul 26, 2023 •

edited

nils-christian commented Oct 24, 2023

nils-christian commented Nov 25, 2023

filipelautert commented Nov 28, 2023

filipelautert commented Nov 29, 2023

nils-christian commented Nov 30, 2023

filipelautert commented Nov 30, 2023

nils-christian commented Nov 30, 2023

filipelautert commented Dec 4, 2023

nils-christian commented Dec 5, 2023

mdimond commented Dec 6, 2023 •

edited

filipelautert commented Dec 7, 2023

mdimond commented Dec 7, 2023 •

edited

filipelautert commented Dec 7, 2023

RiVogel commented Dec 18, 2023

filipelautert commented Dec 18, 2023

muzuro commented Dec 20, 2023 •

edited

filipelautert commented Dec 20, 2023

muzuro commented Dec 21, 2023 •

edited

Laurens-W commented Jan 5, 2024 •

edited

muzuro commented Feb 2, 2024 •

edited

change at 4.19.1 makes includeAll is finding duplicates #3881

change at 4.19.1 makes includeAll is finding duplicates #3881

Comments

bmoregeo commented Mar 1, 2023 • edited by nvoxland

Search first

Description

Steps To Reproduce

Actual Behavior

Expected/Desired Behavior

Liquibase Version

Database Vendor & Version

Liquibase Integration

Liquibase Extensions

OS and/or Infrastructure Type/Provider

Additional Context

Are you willing to submit a PR?

hylkevds commented Mar 2, 2023 • edited

filipelautert commented Mar 2, 2023

bmoregeo commented Mar 3, 2023

filipelautert commented Mar 3, 2023

bodewig commented Mar 13, 2023

filipelautert commented Mar 13, 2023

XDelphiGrl commented Mar 15, 2023

famod commented Jul 26, 2023

famod commented Jul 26, 2023 • edited

nils-christian commented Oct 24, 2023

nils-christian commented Nov 25, 2023

filipelautert commented Nov 28, 2023

filipelautert commented Nov 29, 2023

nils-christian commented Nov 30, 2023

filipelautert commented Nov 30, 2023

nils-christian commented Nov 30, 2023

filipelautert commented Dec 4, 2023

nils-christian commented Dec 5, 2023

mdimond commented Dec 6, 2023 • edited

filipelautert commented Dec 7, 2023

mdimond commented Dec 7, 2023 • edited

filipelautert commented Dec 7, 2023

RiVogel commented Dec 18, 2023

filipelautert commented Dec 18, 2023

muzuro commented Dec 20, 2023 • edited

filipelautert commented Dec 20, 2023

muzuro commented Dec 21, 2023 • edited

Laurens-W commented Jan 5, 2024 • edited

muzuro commented Feb 2, 2024 • edited

bmoregeo commented Mar 1, 2023 •

edited by nvoxland

hylkevds commented Mar 2, 2023 •

edited

famod commented Jul 26, 2023 •

edited

mdimond commented Dec 6, 2023 •

edited

mdimond commented Dec 7, 2023 •

edited

muzuro commented Dec 20, 2023 •

edited

muzuro commented Dec 21, 2023 •

edited

Laurens-W commented Jan 5, 2024 •

edited

muzuro commented Feb 2, 2024 •

edited