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 support for unique constraints (PostgreSQL-only). #46192

Merged
merged 1 commit into from
Mar 12, 2023

Conversation

alpaca-tc
Copy link
Contributor

@alpaca-tc alpaca-tc commented Oct 4, 2022

Motivation / Background

Add support for unique constraints (PostgreSQL-only).

add_unique_key :sections, [:position], deferrable: :deferred, name: "unique_section_position"
remove_unique_key :sections, name: "unique_section_position"

See PostgreSQL's Unique Constraints documentation for more on unique constraints.

By default, unique constraints in PostgreSQL are checked after each statement.
This works for most use cases, but becomes a major limitation when replacing
records with unique column by using multiple statements.

An example of swapping unique columns between records.

# position is unique column
old_item = Item.create!(position: 1)
new_item = Item.create!(position: 2)

Item.transaction do
  old_item.update!(position: 2)
  new_item.update!(position: 1)
end

Using the default behavior, the transaction would fail when executing the
first UPDATE statement.

By passing the :deferrable option to the add_unique_key statement in
migrations, it's possible to defer this check.

add_unique_key :items, [:position], deferrable: :immediate

Passing deferrable: :immediate does not change the behaviour of the previous example,
but allows manually deferring the check using SET CONSTRAINTS ALL DEFERRED within a transaction.
This will cause the unique constraints to be checked after the transaction.

It's also possible to adjust the default behavior from an immediate
check (after the statement), to a deferred check (after the transaction):

add_unique_key :items, [:position], deferrable: :deferred

PostgreSQL allows users to create a unique constraints on top of the unique
index that cannot be deferred. In this case, even if users creates deferrable
unique constraint, the existing unique index does not allow users to violate uniqueness
within the transaction. If you want to change existing unique index to deferrable,
you need execute remove_index before creating deferrable unique constraints.

Detail

Similar to #40224, this extends Active Record's migration/schema dumping to support Unique Constraints.

add_unique_key :sections, [:position], deferrable: :deferred, name: "unique_section_position"
remove_unique_key :sections, name: "unique_section_position"

Additional information

The unique constraint is supported by most DB, but its function is almost equivalent to unique index.
Perhaps only PostgreSQL and Oracle Database with deferrable features would benefit from supporting unique constraint syntax.

Checklist

Before submitting the PR make sure the following are checked:

  • This Pull Request is related to one change. Changes that are unrelated should be opened in separate PRs.
  • There are no typos in commit messages and comments.
  • Commit message has a detailed description of what changed and why. If this PR fixes a related issue include it in the commit message. Ex: [Fix #issue-number]
  • Feature branch is up-to-date with main (if not - rebase it).
  • Pull request only contains one commit for bug fixes and small features. If it's a larger feature, multiple commits are permitted but must be descriptive.
  • Tests are added if you fix a bug or add a feature.
  • CHANGELOG files are updated for the changed libraries if there is a behavior change or additional feature. Minor bug fixes and documentation changes should not be included.
  • PR is not in a draft state.
  • CI is passing.

@sergiopantoja
Copy link

This is something I could definitely use. Ran into this exact issue (needing a deferred unique constraint) implementing a drag-and-drop feature with https://github.com/brendon/acts_as_list. Thank you!

@sergiopantoja
Copy link

If the Rails team doesn't want to add the new methods, another possible syntax we could use is:

add_index :items, [:position], unique: true
add_index :items, [:position], unique: { deferrable: true }
add_index :items, [:position], unique: { deferrable: :deferred }

@yahonda
Copy link
Member

yahonda commented Oct 19, 2022

Thanks for the information. Let me confirm that brendon/acts_as_list#378 is the issue you are referring to?

@yahonda
Copy link
Member

yahonda commented Oct 19, 2022

The question above is for @sergiopantoja

@alpaca-tc
Copy link
Contributor Author

alpaca-tc commented Oct 19, 2022

If the Rails team doesn't want to add the new methods, another possible syntax we could use is:
add_index :items, [:position], unique: { deferrable: true }

@sergiopantoja The syntax of add_index(... , unique: true, deferrable: :true) is confusing and I don't want to support it.
Since UNIQUE INDEX is a restriction on the structure of the index, there is no such thing as a DEFERRABLE unique index. Therefore, CREATE UNIQUE INDEX ~ DEFERRABLE is a syntax error.

The UNIQUE CONSTRAINT added this PR is a unique constraint. And it can define as DEFERRABLE.
When adding a unique constraint implicitly creates an index, but this is to speed up the DB's internal processing.

@yahonda
Copy link
Member

yahonda commented Oct 25, 2022

Let me leave some comments.

  • Method name add_unique_constraint

How about add_unique_key because there is add_foreign_key method to create foreign keys?

add_*_key looks consistent to me because these methods create database-level unique/foreign key constraints rather than following add_check_constrainit.

  • deferrable: true and deferrable: :deferred are hard to understand to me. Both true and :deferred are truthy values.

  • Unique constraints can be created on the same column as the unique index created.

As far as I understand, PostgreSQL allows users to create a unique constraints on top of the unique index
that cannot be deferred. I assume most of the users who need to create deferrable unique constraints must have already created unique index by using add_index with unique: true.

In this case, even if users creates deferrable unique constraint, the existing unique index does not allow users
to violate uniqueness within the transaction. So I think some documentation needs to be updated like remove_index is suggested before creating deferrable unique constraints.

@alpaca-tc
Copy link
Contributor Author

@yahonda Thank you for reviewing.

Method name add_unique_constraint
How about add_unique_key because there is add_foreign_key method to create foreign keys?

add_*_key looks consistent to me because these methods create database-level unique/foreign key constraints rather than following add_check_constrainit.

Okay, I'm going to rename those methods.

deferrable: true and deferrable: :deferred are hard to understand to me. Both true and :deferred are truthy values.

I think the deferrable should be the same option as add_foreign_key, don't you? Adds support for deferrable foreign key constraints in PostgreSQL #41487

Unique constraints can be created on the same column as the unique index created.

As far as I understand, PostgreSQL allows users to create a unique constraints on top of the unique index
that cannot be deferred. I assume most of the users who need to create deferrable unique constraints must have already created unique index by using add_index with unique: true.

In this case, even if users creates deferrable unique constraint, the existing unique index does not allow users
to violate uniqueness within the transaction. So I think some documentation needs to be updated like remove_index is suggested before creating deferrable unique constraints.

👍

@alpaca-tc alpaca-tc force-pushed the support_unique_constraints branch 5 times, most recently from 24cf203 to 7cfe0e8 Compare October 27, 2022 03:36
@alpaca-tc
Copy link
Contributor Author

@yahonda
Copy link
Member

yahonda commented Feb 14, 2023

I have one question.

The deferrable: true leaves the default behavior,

Let me confirm this is correct. Adding deferrable: true in the migration adds DEFERRABLE at the end of the SQL statement which allows unique constraints to check deferred.

  • Migration
add_unique_key :users, [:name], deferrable: true
  • SQL generated by this migration
ALTER TABLE "users" ADD CONSTRAINT uniq_rails_4a6f7b0ccc UNIQUE ("name") DEFERRABLE

Here is my example using psql and raw SQL statements.

  • SQL without DEFERRABLE
CREATE TABLE example(
    row integer NOT NULL,
    col integer NOT NULL,
    UNIQUE (row, col)
);

begin;
INSERT INTO example (row, col) VALUES (1,1),(2,2),(3,3);
commit;

SELECT * FROM example;


begin;
UPDATE example SET row = row + 1, col = col + 1; 
# This statement raises `ERROR:  duplicate key value violates unique constraint "example_row_col_key"
# DETAIL:  Key ("row", col)=(2, 2) already exists.` error
commit;

SELECT * FROM example;
  • Result without DEFERRABLE
CREATE TABLE
BEGIN
INSERT 0 3
COMMIT
 row | col
-----+-----
   1 |   1
   2 |   2
   3 |   3
(3 rows)

BEGIN
ERROR:  duplicate key value violates unique constraint "example_row_col_key"
DETAIL:  Key ("row", col)=(2, 2) already exists.
ROLLBACK
 row | col
-----+-----
   1 |   1
   2 |   2
   3 |   3
(3 rows)

test46192=#
  • SQL with DEFERRABLE
CREATE TABLE example(
    row integer NOT NULL,
    col integer NOT NULL,
    UNIQUE (row, col) DEFERRABLE
);

begin;
INSERT INTO example (row, col) VALUES (1,1),(2,2),(3,3);
commit;

SELECT * FROM example;


begin;
UPDATE example SET row = row + 1, col = col + 1; # This statement should update the rows
commit;

SELECT * FROM example;
  • Result with DEFERRABLE
CREATE TABLE
BEGIN
INSERT 0 3
COMMIT
 row | col
-----+-----
   1 |   1
   2 |   2
   3 |   3
(3 rows)

BEGIN
UPDATE 3
COMMIT
 row | col
-----+-----
   2 |   2
   3 |   3
   4 |   4
(3 rows)

test46192=#

@yahonda
Copy link
Member

yahonda commented Feb 14, 2023

My previous question is more like confirming PostgreSQL behavior, so let me answer the question.

I think the deferrable should be the same option as add_foreign_key,
don't you? Adds support for deferrable foreign key constraints in PostgreSQL #41487

Now I understand this API design comes from add_foreign_key. Do you have any other ideas for API design without considering current add_foreign_key? If the new API design is better, we may consider deprecating the current behavior. API compatibility is important. it would be difficult to change the API of add_foreign_key, though. I'd like to know.

@alpaca-tc
Copy link
Contributor Author

@yahonda

Let me confirm this is correct. Adding deferrable: true in the migration adds DEFERRABLE at the end of the SQL statement which allows unique constraints to check deferred.

I didn't know there was a no conflict with bulk update without specifying SET CONSTRAINTS ALL DEFERRED. Thanks for the confirmation.

Now I understand this API design comes from add_foreign_key. Do you have any other ideas for API design without considering current add_foreign_key? If the new API design is better, we may consider deprecating the current behavior. API > compatibility is important. it would be difficult to change the API of add_foreign_key, though. I'd like to know.

I like the current add_foreign_key API.

Hmmm...
If I had to change it, I think it would be possible to get rid of deferrable: true and change it to only allow deferrable: :immediate | :deferred symbols.

@yahonda
Copy link
Member

yahonda commented Feb 17, 2023

If I had to change it, I think it would be possible to get rid of deferrable: true and change it to only allow deferrable: :immediate | :deferred symbols.

If just setting DEFERRABLE without INITIALLY DEFERRED or INITIALLY IMMEDIATE clause
means one of INITIALLY DEFERRED or INITIALLY IMMEDIATE, I think we would remove true value from deferrable: arguments.

Reading PostgreSQL document however, I'm not clear what is an expected behavior only when DEFERRABLE specified.

https://www.postgresql.org/docs/15/sql-set-constraints.html

Upon creation, a constraint is given one of three characteristics: DEFERRABLE INITIALLY DEFERRED, DEFERRABLE INITIALLY IMMEDIATE, or NOT DEFERRABLE. The third class is always IMMEDIATE and is not affected by the SET CONSTRAINTS command. The first two classes start every transaction in the indicated mode, but their behavior can be changed within a transaction by SET CONSTRAINTS.

@yahonda
Copy link
Member

yahonda commented Feb 20, 2023

Reading PostgreSQL document however, I'm not clear what is an expected behavior only when DEFERRABLE specified.

Investigated how DEFERRABLE without any option works and found it means DEFERRABLE INITIALLY IMMEDIATE.
This behavior is documented and consistent between PostgreSQL 9.3, the oldest version supported by the Rails main branch, and the latest PostgreSQL 15.

https://www.postgresql.org/docs/9.3/sql-createtable.html

If a constraint is deferrable, this clause specifies the default time to check the constraint. If the constraint is INITIALLY IMMEDIATE, it is checked after each statement. This is the default. If the constraint is INITIALLY DEFERRED, it is checked only at the end of the transaction. The constraint check time can be altered with the SET CONSTRAINTS command.

https://www.postgresql.org/docs/15/sql-createtable.html

If a constraint is deferrable, this clause specifies the default time to check the constraint. If the constraint is INITIALLY IMMEDIATE, it is checked after each statement. This is the default. If the constraint is INITIALLY DEFERRED, it is checked only at the end of the transaction. The constraint check time can be altered with the SET CONSTRAINTS command.

Here are my test details. https://gist.github.com/yahonda/1adcf1a776a385b8ec47c9cdf5dcf08b

@yahonda
Copy link
Member

yahonda commented Feb 21, 2023

Based on how PostgreSQL works, investigated at #46192 (comment)

I'd like to propose that deferrable: only accepts :immediate or :deferred.

  • Example
add_unique_key :items, [:position], deferrable: :immediate
add_unique_key :items, [:position], deferrable: :deferred

If deferrable: true or other invalid option name is given, it should raise an error or suggest the correct argument name.

@alpaca-tc alpaca-tc force-pushed the support_unique_constraints branch 2 times, most recently from bd90559 to 12a2721 Compare February 22, 2023 16:27
@alpaca-tc
Copy link
Contributor Author

@yahonda Thank you for your detailed investigation 🙏

I have updated some fixes and rebased 😄
Since force-push and rebase make it difficult to display changes, I also provide a URL to view the diff.
diff

The changes are as follows

  • CHANGELOG
    • Now breaks lines after a certain length.
    • Corrected explanation of option values and default behavior.
  • The variable unique_constraint is renamed to unique_key when changing from add_unique_constraint to add_unique_key, which was forgotten to be renamed.
  • Changed deferrable: to only accept :immediate and :deferred.
    • It no longer shares the conversion logic with add_foreign_key.
      • NOTE: To unify the behavior of deferrable: true for add_foreign_key with add_unique_key, we need to deprecate deferred: true for add_foreign_key in another PR. I will try to fix this in the next PR.
  • Fixed a bug in the condition that throws an ActiveRecord::IrreversibleMigration exception when remove_unique_key cannot be rolled back.
  • Added some tests

alpaca-tc added a commit to alpaca-tc/rails that referenced this pull request Apr 26, 2023
`deferrable: true` is deprecated in favor of `deferrable: :immediate`, and
will be removed in Rails 7.2.

Because `deferrable: true` and `deferrable: :deferred` are hard to understand.
Both true and :deferred are truthy values.
This behavior is the same as the deferrable option of the add_unique_key method, added in rails#46192.

*Hiroyuki Ishii*
danielvdao pushed a commit to danielvdao/rails that referenced this pull request May 1, 2023
`deferrable: true` is deprecated in favor of `deferrable: :immediate`, and
will be removed in Rails 7.2.

Because `deferrable: true` and `deferrable: :deferred` are hard to understand.
Both true and :deferred are truthy values.
This behavior is the same as the deferrable option of the add_unique_key method, added in rails#46192.

*Hiroyuki Ishii*
danielvdao pushed a commit to danielvdao/rails that referenced this pull request May 1, 2023
`deferrable: true` is deprecated in favor of `deferrable: :immediate`, and
will be removed in Rails 7.2.

Because `deferrable: true` and `deferrable: :deferred` are hard to understand.
Both true and :deferred are truthy values.
This behavior is the same as the deferrable option of the add_unique_key method, added in rails#46192.

*Hiroyuki Ishii*
danielvdao pushed a commit to danielvdao/rails that referenced this pull request May 1, 2023
`deferrable: true` is deprecated in favor of `deferrable: :immediate`, and
will be removed in Rails 7.2.

Because `deferrable: true` and `deferrable: :deferred` are hard to understand.
Both true and :deferred are truthy values.
This behavior is the same as the deferrable option of the add_unique_key method, added in rails#46192.

*Hiroyuki Ishii*
briu pushed a commit to briu/rails that referenced this pull request May 4, 2023
`deferrable: true` is deprecated in favor of `deferrable: :immediate`, and
will be removed in Rails 7.2.

Because `deferrable: true` and `deferrable: :deferred` are hard to understand.
Both true and :deferred are truthy values.
This behavior is the same as the deferrable option of the add_unique_key method, added in rails#46192.

*Hiroyuki Ishii*
alpaca-tc added a commit to alpaca-tc/rails that referenced this pull request Sep 19, 2023
follow-up rails#46192

Fixed a bug where `unique_keys` returned the old column name after the column specified in add_unique_key was renamed.
Since `pg_attribute.attname` may return the old column name after renaming a column, match `attrelid, attnum` in the process of getting the list of column names.
@searls
Copy link
Contributor

searls commented Jan 4, 2024

Update to anyone landing here from Google or this post, this was renamed back to add_unique_constraint (t.unique_constraint also works)

@chaadow
Copy link
Contributor

chaadow commented Mar 11, 2024

@searls FYI : here is the changelog link locked with SHA by pressing "Y" on the page.

This link will work "forever"

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

Successfully merging this pull request may close these issues.

6 participants