Fix types handling

[filipelautert]

Restore previous behavior and handle diff when the size/length of a field changes.

[github-actions]

Testing These Changes

To test this PR, use the artifacts attached to the latest CI build

Artifacts Available:

• liquibase-hibernate-artifacts: Zip containing the .jar file to test
• test-reports-*: Detailed automated test results

Download with liquibase-sdk-maven-plugin

Alternately, you can use the Liquibase SDK Maven Plugin

Download the artifacts

mvn org.liquibase.ext:liquibase-sdk-maven-plugin:0.10.20:download-snapshot-artifacts -Dliquibase.sdk.repo=liquibase/liquibase-hibernate -Dliquibase.sdk.branchSearch=liquibase:fix_types_handling -Dliquibase.sdk.downloadDirectory=download -Dliquibase.sdk.artifactPattern=*-artifacts -Dliquibase.sdk.unzipArtifacts=true

Install to your local maven cache

mvn org.liquibase.ext:liquibase-sdk-maven-plugin:0.10.20:install-snapshot -Dliquibase.sdk.repo=liquibase/liquibase-hibernate -Dliquibase.sdk.branchSearch=liquibase:fix_types_handling

[github-actions]

Test Results

36 files  ±0  36 suites  ±0   53s ⏱️ +2s
21 tests ±0  21 ✔️ ±0  0 💤 ±0  0 ❌ ±0 
84 runs  ±0  84 ✔️ ±0  0 💤 ±0  0 ❌ ±0 

Results for commit c4c391c. ± Comparison against base commit 76151df.

♻️ This comment has been updated with latest results.

[javaboy79]

@filipelautert My test produces an NPE
Caused by: java.lang.NullPointerException: Cannot invoke "java.lang.Integer.intValue()" because the return value of "liquibase.structure.core.DataType.getColumnSize()" is null at liquibase.ext.hibernate.diff.ChangedColumnChangeGenerator.handleSizeChange(ChangedColumnChangeGenerator.java:47) at liquibase.ext.hibernate.diff.ChangedColumnChangeGenerator.handleTypeDifferences(ChangedColumnChangeGenerator.java:33) at liquibase.diff.output.changelog.core.ChangedColumnChangeGenerator.fixChanged(ChangedColumnChangeGenerator.java:66) at liquibase.diff.output.changelog.ChangeGeneratorChain.fixChanged(ChangeGeneratorChain.java:137) at liquibase.diff.output.changelog.ChangeGeneratorFactory.fixChanged(ChangeGeneratorFactory.java:117) at liquibase.diff.output.changelog.DiffToChangeLog.generateChangeSets(DiffToChangeLog.java:282) at liquibase.diff.output.changelog.DiffToChangeLog.printNew(DiffToChangeLog.java:230) at liquibase.diff.output.changelog.DiffToChangeLog$1.run(DiffToChangeLog.java:149) ... 38 more

[filipelautert]

I should have tested with fields that don't have sizes. Well, fixed now! Can you give it another try @javaboy79 ? Thank you!

[javaboy79]

This is mostly working now. However, I am still ending up with items in my diff that should not be there. timestamp types are always listed for modify. I tried applying the change, and still, they are listed again on the next diff. So, clearly, something is still wrong with the diff.

All other reported types are now cleared from the diff.

[javaboy79]

@filipelautert I feel like you were making good progress on this. Do you have any feedback or fix for my last comment?

[filipelautert]

Hello @javaboy79 ! I think there is no other way but creating a list of type exclusions for the size checks.
In the end this is good, as before it was excluding everything, but now we can choose what to exclude.
I went ahead and added timestamp. If there is anything else let me know and I can add to the list. I want to finish this in time for the next release.

[javaboy79]

Another NPE:
Caused by: java.lang.RuntimeException: java.lang.RuntimeException: java.lang.NullPointerException: Cannot invoke "java.lang.Integer.intValue()" because the return value of "liquibase.structure.core.DataType.getColumnSize()" is null at liquibase.diff.output.changelog.DiffToChangeLog.print(DiffToChangeLog.java:207) at liquibase.diff.output.changelog.DiffToChangeLog.print(DiffToChangeLog.java:96) at liquibase.diff.output.changelog.DiffToChangeLog.print(DiffToChangeLog.java:90) at liquibase.command.core.InternalDiffChangelogCommandStep.run(InternalDiffChangelogCommandStep.java:63) at liquibase.command.CommandScope.execute(CommandScope.java:172) ... 29 more Caused by: java.lang.RuntimeException: java.lang.NullPointerException: Cannot invoke "java.lang.Integer.intValue()" because the return value of "liquibase.structure.core.DataType.getColumnSize()" is null at liquibase.diff.output.changelog.DiffToChangeLog$1.run(DiffToChangeLog.java:190) at liquibase.Scope.lambda$child$0(Scope.java:180) at liquibase.Scope.child(Scope.java:189) at liquibase.Scope.child(Scope.java:179) at liquibase.Scope.child(Scope.java:158) at liquibase.diff.output.changelog.DiffToChangeLog.print(DiffToChangeLog.java:143) ... 33 more Caused by: java.lang.NullPointerException: Cannot invoke "java.lang.Integer.intValue()" because the return value of "liquibase.structure.core.DataType.getColumnSize()" is null at liquibase.ext.hibernate.diff.ChangedColumnChangeGenerator.handleSizeChange(ChangedColumnChangeGenerator.java:47) at liquibase.ext.hibernate.diff.ChangedColumnChangeGenerator.handleTypeDifferences(ChangedColumnChangeGenerator.java:33) at liquibase.diff.output.changelog.core.ChangedColumnChangeGenerator.fixChanged(ChangedColumnChangeGenerator.java:66) at liquibase.diff.output.changelog.ChangeGeneratorChain.fixChanged(ChangeGeneratorChain.java:137) at liquibase.diff.output.changelog.ChangeGeneratorFactory.fixChanged(ChangeGeneratorFactory.java:117) at liquibase.diff.output.changelog.DiffToChangeLog.generateChangeSets(DiffToChangeLog.java:282) at liquibase.diff.output.changelog.DiffToChangeLog.printNew(DiffToChangeLog.java:230) at liquibase.diff.output.changelog.DiffToChangeLog$1.run(DiffToChangeLog.java:149)

[filipelautert]

Hm.. this time the problem was down there on liquibase core. Seems I passed something with null that could not be handled .
I believe I fixed it, here we go again...

[filipelautert]

@javaboy79 When you have some time, it's ready for another run. Thanks!

[javaboy79]

Yes, I'm out of town this week. I should be able to give it another run on Monday.

…

On Mon, Apr 3, 2023, 10:35 AM filipe ***@***.***> wrote: @javaboy79 <https://github.com/javaboy79> When you have some time, it's ready for another run. Thanks! — Reply to this email directly, view it on GitHub <#467 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AA2HRC44BGWGMM6L5NYDDNTW7LN23ANCNFSM6AAAAAAVZ23IHE> . You are receiving this because you were mentioned.Message ID: ***@***.***>

[filipelautert]

Hello @javaboy79 - I'm merging this as is for 4.21.0 that is going to be out today. If the problem persists let's continue from here 👍

[javaboy79]

@filipelautert Thank you for your efforts here! This is looking really good. Today I was able to pull 4.21.1 into my project and the diff produced actually found and correctly reported a discrepancy on one of my varchar lengths. This is great! However, as far as I can see, there is one problem left...

The diff is also reporting false positives on my unique indexes. Mind you, this is only happening for unique indexes. Other indexes seem fine.

e.g. (names obfuscated)
@Table( indexes = { @Index(name = "my_id_index", columnList = "id", unique = true), })

Even though the db already shows this as unique, the diff produces the following:
<changeSet author="user (generated)" id="1681760762251-3"> <dropUniqueConstraint constraintName="my_id_index" tableName="xxx"/> </changeSet>

[javaboy79]

@filipelautert This is also happening on unique constraints that were previously generated by the column annotation:
@Column(unique = true)
These are similarly being reported as a drop by the diff.

[filipelautert]

@javaboy79 thanks! I'll create another PR until tomorrow and will tag you there.

[javaboy79]

@filipelautert Looks like it has already been reported. #479