Skip to content

Handle case sensitive primary key names in dropPrimaryKey for postgresql #1974

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 4 commits into from
Oct 29, 2021
Merged

Handle case sensitive primary key names in dropPrimaryKey for postgresql #1974

merged 4 commits into from
Oct 29, 2021

Conversation

nvoxland
Copy link
Contributor

@nvoxland nvoxland commented Jul 9, 2021 •
edited by sync-by-unito bot
Loading

Fixes #1933

Dev Handoff Notes (Internal Use)

Links

Testing

Dev Verification

Liquibase command 'update' was executed successfully.

Test Requirements (Liquibase Internal QA)

Manual Tests

Use the changelog attached to this ticket to run your tests against a Postgres database.

  • Setup: Deploy the changes to create tables
    • liquibase update --changelog-file lb1976-changelog.xml --labels createTable

Verify update of a dropPrimaryKey with an auto-generated, camelCase PK name is successful._

  • liquibase update --changelog-file lb1976-changelog.xml --labels dropAutoNamedPK

Verify update of a addPrimaryKey with a camelCase named PK is successful._

  • liquibase update --changelog-file lb1976-changelog.xml --labels addNamedPK

Verify update of a dropPrimaryKey with a camelCase named PK is successful._

  • liquibase update --changelog-file lb1976-changelog.xml --labels dropNamedPK

Automated Test Requirements

  • No new functional automated tests required.
  • A liquibase/liquibase-test-harness test is required.
    • Add a changelog with changes sets in the attached changelog with changeset author “gjvoosten.”
      • liquibase-test-harness/src/main/resources/liquibase/harness/change/changelogs/postgresql.
    • Add the JSON snapshot for test validation:
      • src/main/resources/liquibase/harness/change/expectedSnapshot/
    • Add the generated SQL for test validation:
      • src/main/resources/liquibase/harness/change/expectedSql/

Validate that dropPrimaryKey does not fail due to camelCasing of the autogenerated PK name.

┆Issue is synchronized with this Jira Bug by Unito

@nvoxland nvoxland requested a review from suryaaki2 July 9, 2021 21:46
@nvoxland nvoxland mentioned this pull request Jul 9, 2021
Closed
@gjvoosten
Copy link
Contributor

@nvoxland @suryaaki2 Any progress on this? It needs this additional update to pass the tests:

diff --git a/liquibase-core/src/test/java/liquibase/verify/saved_state/compareGeneratedSqlWithExpectedSqlForMinimalChangesets/dropPrimaryKey/postgresql.sql b/liquibase-core/src/test/java/liquibase/verify/saved_state/compareGeneratedSqlWithExpectedSqlForMinimalChangesets/dropPrimaryKey/postgresql.sql
index 660ffa303..45aca4267 100644
--- a/liquibase-core/src/test/java/liquibase/verify/saved_state/compareGeneratedSqlWithExpectedSqlForMinimalChangesets/dropPrimaryKey/postgresql.sql
+++ b/liquibase-core/src/test/java/liquibase/verify/saved_state/compareGeneratedSqlWithExpectedSqlForMinimalChangesets/dropPrimaryKey/postgresql.sql
@@ -6,5 +6,5 @@ BEGIN
     FROM INFORMATION_SCHEMA.TABLE_CONSTRAINTS tc
     WHERE CONSTRAINT_TYPE = 'PRIMARY KEY'
       AND TABLE_NAME = 'person' AND TABLE_SCHEMA = 'null';
-    EXECUTE 'alter table null.person drop constraint ' || constraint_name;
+    EXECUTE 'alter table null.person drop constraint "' || constraint_name || '"';
 END $$;;

As I wrote in #1933 (comment), it otherwise works fine.

@gjvoosten
Copy link
Contributor

@nvoxland @suryaaki2 Anything I can do to help speed this up?

@nvoxland nvoxland changed the base branch from 4.4.x to master September 24, 2021 19:57
@gjvoosten
Copy link
Contributor

@nvoxland @suryaaki2 Any chance this will get merged & released soon'ish? It's really blocking us from updating…

@sync-by-unito sync-by-unito bot assigned yodzhubeiskyi and unassigned nvoxland Oct 27, 2021
@nvoxland nvoxland merged commit c65d233 into master Oct 29, 2021
@nvoxland nvoxland deleted the postgresql-drop-pk branch October 29, 2021 16:47
@nvoxland
Copy link
Contributor Author

It will be going out in the upcoming 4.6.0 release

@nvoxland nvoxland mentioned this pull request Nov 3, 2021
Closed
@nvoxland nvoxland added this to the v4.6.0 milestone Nov 5, 2021
@kristyldatical kristyldatical mentioned this pull request Nov 12, 2021
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
1 more reviewer

@suryaaki2 suryaaki2 suryaaki2 approved these changes

Reviewers whose approvals may not affect merge requirements
Assignees

@yodzhubeiskyi yodzhubeiskyi

Labels
None yet
Projects
None yet
Milestone
v4.6.0
Development

Successfully merging this pull request may close these issues.

dropPrimaryKey fails on PostgreSQL
4 participants
@nvoxland @gjvoosten @suryaaki2 @yodzhubeiskyi