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

Sequence owned by a table field must be included in database snapshot #3335

Conversation

filipelautert
Copy link
Collaborator

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

When a sequence is owned by a table field it looks like very much an auto increment sequence in postgres catalogs. Because of that liquibase was skiping this sequence when doing a snapshot of the databse.

This PR makes sure that any sequence that is owned by a table field and is not used as auto increment (serial) is snapshoted as an ordinary sequence.

Things to be aware of

  • A sequence owned by a table field looks like very much as a sequence created by a serial field.

Things to worry about

Additional Context

…d is not detected as SERIAL is created as an ordinary sequence.
@filipelautert filipelautert linked an issue Oct 3, 2022 that may be closed by this pull request
@filipelautert filipelautert self-assigned this Oct 3, 2022
@github-actions
Copy link

github-actions bot commented Oct 3, 2022

Unit Test Results

  4 752 files  ±  0    4 752 suites  ±0   32m 32s ⏱️ - 3m 57s
  4 717 tests +  1    4 484 ✔️ +1     233 💤 ±  0  0 ±0 
55 620 runs  +12  50 306 ✔️ +1  5 314 💤 +11  0 ±0 

Results for commit b0793ea. ± Comparison against base commit 0a4de08.

♻️ This comment has been updated with latest results.

…d is not detected as SERIAL is created as an ordinary sequence.
@nvoxland nvoxland removed their assignment Oct 4, 2022
Copy link
Contributor

@nvoxland nvoxland left a comment

Choose a reason for hiding this comment

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

Code review and test results:

Things to be aware of:

  • Failing test harness test in unrelated
  • Just impacts postgresql
  • Fixes postgres < 10 and 10+
  • Adds integration test for issue

Things to worry about:

  • Nothing

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.

This PR address a bug in Liquibase handling of the Postgres sequence OWNED BY clause. With the fix, Liquibase diff-changelog creates an ALTER SEQUENCE instead of a DROP SEQUENCE changeset for a Postgres sequence that differs only due to a change in the OWNED BY clause.

  • New integration tests added.
  • No additional testing required.

APPROVED

…fchangelog-dropping-sequences-erroneously

# Conflicts:
#	liquibase-integration-tests/src/test/java/liquibase/dbtest/pgsql/PostgreSQLIntegrationTest.java
@filipelautert filipelautert merged commit db8424d into master Nov 17, 2022
@filipelautert filipelautert deleted the 2555-postgresql-diffchangelog-dropping-sequences-erroneously branch November 17, 2022 01:03
@filipelautert filipelautert added this to the 4.18.0 milestone Dec 7, 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.

postgresql diffChangelog dropping sequences erroneously
4 participants