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

SQLite3 adapter alter_table method restores foreign keys #33585

Merged
merged 1 commit into from Aug 16, 2018

Conversation

@yahonda
Copy link
Contributor

@yahonda yahonda commented Aug 11, 2018

Summary

SQLite3 adapter alter_table method restores foreign keys.

Rails 6 supports SQLite database 3.8 or higher which allows executing alter_table method for foreign key referenced - "parent" tables. This pull request enables alter_table method for foreign keys referencing - "child" tables.

Related to #33520, this pull request does not support renaming columns for foreign keys.

@rails-bot
Copy link

@rails-bot rails-bot commented Aug 11, 2018

r? @georgeclaghorn

(@rails-bot has picked a reviewer for you, use r? to override)

Copy link
Member

@eileencodes eileencodes left a comment

Looks good. Can you add a change log entry?

options[:foreign_keys].each do |fk|
@definition.foreign_key(fk.to_table, fk.options)
end
end
Copy link
Member

@eileencodes eileencodes Aug 11, 2018

Can you add a space before and after this block? Like this:

end

if options[:foreign_keys]
 ...
end

yield

@eileencodes eileencodes added this to the 6.0.0 milestone Aug 11, 2018
@yahonda
Copy link
Contributor Author

@yahonda yahonda commented Aug 11, 2018

Thanks for the review. Added a new changelog entry and made changes as suggested.

@georgeclaghorn
Copy link
Contributor

@georgeclaghorn georgeclaghorn commented Aug 11, 2018

@kamipo kamipo merged commit 33ce73e into rails:master Aug 16, 2018
2 checks passed
kamipo added a commit that referenced this issue Aug 16, 2018
SQLite3 adapter `alter_table` method restores foreign keys
@yahonda yahonda deleted the diag33520 branch Aug 16, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

None yet

5 participants