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

deferrable foreign key can be passed to references #47671

Merged

Conversation

alpaca-tc
Copy link
Contributor

Motivation / Background

Fixed a bug in where deferrable foreign_key was ignored when passed to t.references

Detail

This PR is a follow-up to #41487 .
#41487 supports the deferrable option for add_foreign_key but t.references does not yet support the deferrable option yet.

I think this is just an omission in the implementation.

create_table(:testings) do |t|
  t.references(:parent, foreign_key: { deferrable: :immediate }) #=> deferrable must not be ignored!
end

Additional information

This PR needs to be merged after #47659 because the return value of ForeignKeyDefinition#deferrable will be changed.

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.
  • 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]
  • Tests are added or updated 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.

@alpaca-tc alpaca-tc force-pushed the references_with_deferrable_foreign_key branch 2 times, most recently from 38010c9 to 512b8b2 Compare March 14, 2023 12:35
@@ -84,6 +84,8 @@ def visit_ForeignKeyDefinition(o)
FOREIGN KEY (#{quote_column_name(o.column)})
REFERENCES #{quote_table_name(o.to_table)} (#{quote_column_name(o.primary_key)})
SQL

sql << " DEFERRABLE INITIALLY #{o.deferrable.to_s.upcase}" if o.deferrable
Copy link
Contributor Author

Choose a reason for hiding this comment

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

NOTE: This PR needs to be merged after #47659 because the return value of ForeignKeyDefinition#deferrable will be changed.

@yahonda
Copy link
Member

yahonda commented Mar 14, 2023

Let's consider this pull request adds a feature with changelog.

@alpaca-tc alpaca-tc force-pushed the references_with_deferrable_foreign_key branch from 512b8b2 to 7dd0607 Compare April 27, 2023 01:32
activerecord/CHANGELOG.md Outdated Show resolved Hide resolved
Fixed a bug in which deferrable foreign_key was ignored when passed to `t.references`.
@alpaca-tc alpaca-tc force-pushed the references_with_deferrable_foreign_key branch from 7dd0607 to ec0a2a9 Compare April 27, 2023 01:44
@alpaca-tc
Copy link
Contributor Author

@yahonda fixed 💛

@yahonda yahonda merged commit 53df1e1 into rails:main Apr 27, 2023
9 checks passed
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.

None yet

2 participants