Skip to content

Comments

support for db-specific options to add_foreign_key#17094

Closed
jenseng wants to merge 1 commit intorails:mainfrom
jenseng:foreign_key_options
Closed

support for db-specific options to add_foreign_key#17094
jenseng wants to merge 1 commit intorails:mainfrom
jenseng:foreign_key_options

Conversation

@jenseng
Copy link
Contributor

@jenseng jenseng commented Sep 29, 2014

:sql_options can be used to pass along arbitrary clauses to the generated
ADD FOREIGN KEY statement, e.g. "DEFERRABLE INITIALLY DEFERRED"

also include logic to infer :sql_options for postgres when dumping so that
schema.rb is accurate. no such logic is needed for mysql, as it doesn't
support any options beyond what's already on master.

:sql_options can be used to pass along arbitrary clauses to the generated
ADD FOREIGN KEY statement.

also include logic to infer :sql_options for postgres when dumping so that
schema.rb is accurate. no such logic is needed for mysql, as it doesn't
support any options beyond what's already on master.
@jenseng
Copy link
Contributor Author

jenseng commented Sep 29, 2014

cc @senny

this is more or less analogous to foreigner's :options option. mainly wanted to start a discussion, as i could see this going a couple different directions, e.g.

  1. is there a better name than :sql_options?
  2. should we not bother inferring the :sql_options when dumping the schema? i.e. instead encourage people to use structure.sql?
  3. alternatively, would it make sense to make some (all?) of these into first-class options to add_foreign_key? for example, DEFERRABLE etc. is supported by sqlite and oracle, and sql server has an analog for NOT VALID ... so if we're interested in expanding foreign key support to other adapters, it might be nice to have options like :deferrable, :deferred and :validate

@senny
Copy link
Member

senny commented Sep 30, 2014

As outlined in the discussion on #15606 I'd like to start with a basic implementation and then move towards a more complete one. For 4.2 this means that you'd have to drop down to raw SQL and structure.sql.

We are currently discussing the future of schema.rb and structure.sql. I don't think we should expand schema.rb with more vendor specific stuff. It opens the doors for bugs and is just not time well spent. With structure.sql we can build on top of what the databases give us without rebuilding existing functionality.

When it comes to add_foreign_key and remove_foreign_key I'm open for more complete API's.

@carlosantoniodasilva
Copy link
Member

@jenseng hey, are you interesting in moving this forward? If so, can you please bring it up-to-date with master, and I will do some review on this PR later to give you some feedback? Otherwise I can take it from here on top of what you did. Thanks!

@jenseng
Copy link
Contributor Author

jenseng commented Apr 5, 2017

@carlosantoniodasilva feel free to run with this, it's not currently on my radar. Thanks!

@rails-bot
Copy link

rails-bot bot commented Dec 17, 2019

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.
Thank you for your contributions.

@rails-bot rails-bot bot added the stale label Dec 17, 2019
@rails-bot rails-bot bot closed this Dec 24, 2019
@rails-bot rails-bot bot removed the stale label Jan 2, 2020
@smoyte
Copy link

smoyte commented Mar 24, 2020

I would love to see this get some traction. I'm surprised nobody else has needed DEFERRABLE. It's such a useful tool in some situations. And I really don't want to drop down to structure.sql.

@smoyte
Copy link

smoyte commented Mar 24, 2020

@carlosantoniodasilva any way I can help out here?

@rails-bot
Copy link

rails-bot bot commented Jun 22, 2020

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.
Thank you for your contributions.

@rails-bot rails-bot bot added the stale label Jun 22, 2020
Base automatically changed from master to main January 14, 2021 17:00
@rafaelfranca
Copy link
Member

Closing in favor of #41487. Thank you for the contribution.

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.

5 participants