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

Add default catalog condition to SequenceSnapshotGenerator for Oracle #3152

Conversation

gabrielnardes
Copy link
Contributor

@gabrielnardes gabrielnardes commented Aug 5, 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

When verifying if a sequence exists in a Oracle, Liquibase sets the column SEQUENCE_OWNER according to the catalog name provided in the changeset. If the catalog is not set, Liquibase will try to search for a SEQUENCE_OWNER='null'. We can set this value to the default catalog name of the database, because, according to the docs, this is a not null column: https://docs.oracle.com/en/database/oracle/oracle-database/21/refrn/ALL_SEQUENCES.html

Fixes #3138

@nvoxland nvoxland added the SafeToBuild Indicates that a particular PR contains changes which are safe to build using GitHub actions label Aug 31, 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:

  • The changes make sense to me. It's ensuring the catalog name is correctly set in the existing query

Things to worry about:

  • Nothing

@github-actions
Copy link

Unit Test Results

  4 632 files  +12    4 632 suites  +12   36m 9s ⏱️ +37s
  4 622 tests +  3    4 403 ✔️  -   1     219 💤 +4  0 ±0 
54 636 runs  +36  49 612 ✔️ +32  5 024 💤 +4  0 ±0 

Results for commit 527437f. ± Comparison against base commit 4f7e725.

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 prevents Oracle sequence owner being incorrectly set to "null" when catalogName is not specified on a changeset. Oracle defines the ALL_SEQUENCES.SEQUENCE_OWNER as not-null. This means that when a changeset does not include catalogName, it is safe to use the default catalog as the SEQUENCE_OWNER when querying for an Oracle sequence.

APPROVED

@nvoxland nvoxland merged commit 0fc0955 into liquibase:master Sep 19, 2022
@kataggart kataggart added this to the NEXT milestone Sep 20, 2022
@tabbyf00
Copy link

Thanks for your PR submission! We just finished reviewing and merging it into the 4.17.0 release on October 10, 2022. When you get a chance, could you please Star the Liquibase project? The star button is in the upper right corner of the screen.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
autocandidate complexityLocal criticalityBlocker DBOracle featureSnapshot SafeToBuild Indicates that a particular PR contains changes which are safe to build using GitHub actions Severity3 sprint2022-34 TypeBug
Projects
Archived in project
6 participants