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 adding extra unique constraint to columns that are already unique #579

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

kvch
Copy link
Contributor

@kvch kvch commented Jan 9, 2025

Previously, pgroll failed to add unique constraint to columns that were already included in another unique constraint.

Now this problem is fixed.

Related to #227

@kvch kvch requested a review from andrew-farries January 9, 2025 13:12
@kvch kvch force-pushed the fix-add-unique-constraint-to-column-that-is-already-unique branch from 9373551 to 5993661 Compare January 9, 2025 13:21
@kvch kvch enabled auto-merge (squash) January 9, 2025 13:33
Copy link
Collaborator

@andrew-farries andrew-farries left a comment

Choose a reason for hiding this comment

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

Is it possible to add a test case for the problem that this fixes?

Based on the PR description I tried (using the migrations in this Gist) to add a UNIQUE constraint involving a column that was already UNIQUE and it worked as expected:

After completing both migrations the schema was:

+----------+------------------------+-----------------------------------------------------+----------+--------------+-------------+
| Column   | Type                   | Modifiers                                           | Storage  | Stats target | Description |
|----------+------------------------+-----------------------------------------------------+----------+--------------+-------------|
| id       | integer                |  not null default nextval('items_id_seq'::regclass) | plain    | <null>       | <null>      |
| name     | character varying(255) |  not null                                           | extended | <null>       | <null>      |
| producer | character varying(255) |  not null                                           | extended | <null>       | <null>      |
+----------+------------------------+-----------------------------------------------------+----------+--------------+-------------+
Indexes:
    "items_pkey" PRIMARY KEY, btree (id)
    "items_name_key" UNIQUE CONSTRAINT, btree (name)
    "unique_name_producer" UNIQUE CONSTRAINT, btree (name, producer)

This was using commit daba767

@kvch
Copy link
Contributor Author

kvch commented Jan 9, 2025

Haha, yes. I will redo the example. The test case I put together seemed too complex, but apparently I simplified to too much :D

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants