Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Updated PrimaryKeyExistsPrecondition to require table name for H2 databases #5323

Conversation

filipelautert
Copy link
Collaborator

@filipelautert filipelautert commented Dec 6, 2023

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)

Description

This PR updates the PrimaryKeyExistsPrecondition in order to throw a friendly message when no table name is provided while using H2 database. This change has been done because the H2 database driver requires a table name to be specified for searching a primary key.

Doc notes

This precondition fails on databases which don't support the null as third parameter Documentation is required on https://docs.liquibase.com/concepts/changelogs/preconditions.html mentioning that due to JDBC driver limitations, is some databases such as H2 the tableName is required.

Fixes #3643

…abases

This commit updates the PrimaryKeyExistsPrecondition class in order to throw an exception when no table name is provided while using H2 database. This change has been done because the H2 database driver requires a table name to be specified for searching a primary key.
…r MySQL when tableName is not provided.

- Update test.
- Update test data.
… is not specified to H2 and MySQL test suites.

- Add integration test to validate no error is returned for others DBs.
- Added integration test for these new applied DBs.
- Update existent abstract integration test.
Copy link
Collaborator

@MalloD12 MalloD12 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code changes looks good to me. I have add some other DBs so precondition returns the same given error and added some integration tests.

@rberezen Would you mind looking at the test cases added and see if they look good enough for you?

Thanks,
Daniel.

@rberezen
Copy link
Contributor

rberezen commented Dec 8, 2023

@MalloD12 all good, thanks!

Code changes looks good to me. I have add some other DBs so precondition returns the same given error and added some integration tests.

@rberezen Would you mind looking at the test cases added and see if they look good enough for you?

Thanks, Daniel.

@filipelautert filipelautert merged commit 04c31c8 into master Dec 11, 2023
30 of 32 checks passed
@filipelautert filipelautert deleted the 3643-primarykeyexists-precondition-crashes-when-used-with-h2-2x branch December 11, 2023 19:33
@filipelautert filipelautert added this to the 1NEXT milestone Dec 11, 2023
@adrian-velonis1
Copy link
Contributor

Docs internal: https://datical.atlassian.net/browse/PD-4392

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
DocNeeded enhancement New non breaking enhancements in LB
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

primaryKeyExists precondition crashes when used with H2 2.x
4 participants