-
Notifications
You must be signed in to change notification settings - Fork 1.8k
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
Fix Postgresql autoIncrement Information in snapshot #3361
Conversation
… and encapsulate auto increment details in a new class. Adds support for Postgresql auto increment details.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
PR fixes a bug where Liquibase did not capture the start value and increment by parameters for Postgres autoincrement columns. The bug fixes the underlying issue in snapshot
as well as snapshot-based commands like generate-changelog
and diff-changelog
.
- Code refactored to be more readable.
- Associated liquibase-test-harness PR to add automated test case PR
- No additional testing required.
APPROVED
I am leaving this ticket in code review until the test-harness PR passes. Please take a look at the test failures when you have a moment, @filipelautert. Thanks! |
@XDelphiGrl finally tests are fixed, sorry for the delay. |
@filipelautert Do you want to add javadoc comments? |
@suryaaki2 just did it, thanks for point out. |
Impact
Description
Fix Postgresql auto increment details. Extracted code from ColumnSnapshotGenerator into new class AutoIncrementSequencesCache, and by doing this get rid of a deprecated method usage and creates a standard way to implement new sequence queries.
The AutoIncrementSequencesCache will run a query once against the database and store a cache of all sequences that are related to auto increment fields. To add support to anotehr database , a new query needs to be created returing the same 5 fields that are provided by the existing queries.
Things to be aware of
Things to worry about
Additional Context