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
Adds support for deferrable foreign key constraints in PostgreSQL #41487
Conversation
This pull request has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. |
@benedikt I will have to do some more research on this but is this something you're still interested in picking up? |
@zzak Yes, absolutely! Let me know how I can help with research. |
@benedikt First thing I notice is that CI failed, but I'm not entirely sure it is related yet.
Will have to take a closer look at this, thank you for your patience 🙇 |
@zzak Just merged the current main into this branch to resolve conflicts which caused the CI to fail. I don't think this failure is related, but I can look into it more. |
@benedikt Thanks! I will check back on this issue shortly but feel free to ping me if you have any new ideas 🙇 |
@zzak Tried to reproduce this locally but didn't see the failure there. After rebasing, the CI also didn't complain didn't complain about it again. As the failure was in the Sqlite3 tests and this pull request only touches PostgreSQL, I think it's save to assume it was an unrelated failure. There now seems to be a failure related to memcache timeout, but I guess that's also not related to the changes of this pull request. |
@zzak It's a miracle, but all tests are passing. Even the unrelated ones. 😄 |
Thanks for your review, @ghiculescu. As mentioned in the individual comments, I adjusted the pull request accordingly. Unfortunately, the build once again isn't green because of an HTTP timeout in an unrelated part. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This looks great to me, I will wait for someone from the core team to review. Thanks for your work on this!
@benedikt You will have to squash the commits but maybe better to wait until another committer reviews this 🙏 |
@zzak Yup, it's already just one commit. |
This pull request has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. |
@zzak @ghiculescu Anything I can do to move this forward? 😅 |
There’s a merge conflict, other than that it’s waiting on the core team to review. |
@ghiculescu Yeah, I keep fixing that particular merge conflict for the last couple of months. How do we get the core team to review this? 😄 |
By default, foreign key constraints in PostgreSQL are checked after each statement. This works for most use cases, but becomes a major limitation when creating related records before the parent record is inserted into the database. One example of this is looking up / creating a person via one or more unique alias. ```ruby Person.transaction do alias = Alias .create_with(user_id: SecureRandom.uuid) .create_or_find_by(name: "DHH") person = Person .create_with(name: "David Heinemeier Hansson") .create_or_find_by(id: alias.user_id) end ``` Using the default behavior, the transaction would fail when executing the first `INSERT` statement. This pull request adds support for deferrable foreign key constraints by adding a new option to the `add_foreign_key` statement in migrations: ```ruby add_foreign_key :aliases, :person, deferrable: true ``` The `deferrable: true` leaves the default behavior, but allows manually deferring the checks using `SET CONSTRAINTS ALL DEFERRED` within a transaction. This will cause the foreign keys 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). ```ruby add_foreign_key :aliases, :person, deferrable: :deferred ```
Patience is the name of the game :) There's lots of open PRs and they're all pretty busy with Rails + real jobs. They do eventually get to most PRs but you just have to wait it out. The best advice I've heard is "make the PR easy to merge", which I think you have done a good job of here. |
@rafaelfranca Thank you for merging this 👍 |
Summary
By default, foreign key constraints in PostgreSQL are checked after each statement. This works for most use cases, but becomes a major limitation when creating related records before the parent record is inserted into the database.
One example of this is looking up / creating a person via one or more unique alias.
Using the default behavior, the transaction would fail when executing the first
INSERT
statement.This pull request adds support for deferrable foreign key constraints by adding a new option to the
add_foreign_key
statement in migrations:The
deferrable: true
leaves the default behavior, but allows manually deferring the checks usingSET CONSTRAINTS ALL DEFERRED
within a transaction. This will cause the foreign keys 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).
Other Information
There is a pull request #17094 from 2014 that implements generic SQL options on
add_foreign_key
. Pull request #40192 recently implemented the validate aspect of that.