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

Fix Postgresql autoIncrement Information in snapshot #3361

Merged
merged 13 commits into from
Nov 18, 2022

Conversation

filipelautert
Copy link
Collaborator

@filipelautert filipelautert commented Oct 11, 2022

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

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

  • None

Additional Context

  • None

… and encapsulate auto increment details in a new class.

Adds support for Postgresql auto increment details.
@filipelautert filipelautert linked an issue Oct 11, 2022 that may be closed by this pull request
@filipelautert filipelautert self-assigned this Oct 11, 2022
@github-actions
Copy link

github-actions bot commented Oct 11, 2022

Unit Test Results

  4 752 files  ±  0    4 752 suites  ±0   34m 46s ⏱️ - 3m 16s
  4 717 tests +18    4 484 ✔️ +21     233 💤  - 3  0 ±0 
55 620 runs  +12  50 306 ✔️ +17  5 314 💤  - 5  0 ±0 

Results for commit 0cb90c8. ± Comparison against base commit 9272da2.

♻️ This comment has been updated with latest results.

Copy link
Contributor

@XDelphiGrl XDelphiGrl left a 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

@XDelphiGrl
Copy link
Contributor

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!

liquibase/liquibase-test-harness#377

@filipelautert
Copy link
Collaborator Author

@XDelphiGrl finally tests are fixed, sorry for the delay.

@suryaaki2
Copy link
Contributor

@filipelautert Do you want to add javadoc comments?

@filipelautert
Copy link
Collaborator Author

@filipelautert Do you want to add javadoc comments?

@suryaaki2 just did it, thanks for point out.

@filipelautert filipelautert merged commit bc51733 into master Nov 18, 2022
@filipelautert filipelautert deleted the 2867-autoincrement-information-fix branch November 18, 2022 13:33
@filipelautert filipelautert added this to the 4.18.0 milestone Nov 29, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

Postgres autoIncrementInformation in snapshot is not correct
5 participants