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

adding indexes on tables in migration scripts #16258

Merged
merged 1 commit into from
Jul 25, 2014
Merged

Conversation

mattwarrenrnp
Copy link
Contributor

I have seen several developers new (and old) to rails miss adding indexes to columns in migration scripts. In the vast majority of use cases adding indexes to foreign keys is best practice. As this guide is a top search result I feel that it's important to show best practices rather than a minimum working solution.

With so much magic in Rails, one may expect indexes to be added for you. Having them in the guide serves as a reminder that you need to explicitly add them yourself.

@senny senny added the docs label Jul 22, 2014
@@ -109,6 +109,9 @@ class CreateOrders < ActiveRecord::Migration
t.datetime :order_date
t.timestamps
end

add_index :orders, :customer_id

Copy link
Member

Choose a reason for hiding this comment

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

Remove this extra line.

Copy link
Member

Choose a reason for hiding this comment

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

And in all your additions.

@senny
Copy link
Member

senny commented Jul 22, 2014

@mattwarrenrnp can you squash your commits together?

@mattwarrenrnp
Copy link
Contributor Author

travis failed on some intermittent error there unrelated to the patch..

@@ -109,6 +109,8 @@ class CreateOrders < ActiveRecord::Migration
t.datetime :order_date
t.timestamps
end

add_index :orders, :customer_id
Copy link
Member

Choose a reason for hiding this comment

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

can we use the :index option of belongs_to?

@senny
Copy link
Member

senny commented Jul 24, 2014

@mattwarrenrnp I noticed that most associations are defined with belongs_to or references. These helpers have an index: true option. We should use that to keep the examples compact.

missed one migration script in last commit

remove some empty lines

using the belongs_to index option to be more concise
senny added a commit that referenced this pull request Jul 25, 2014
adding indexes on tables in migration scripts [ci skip]
@senny senny merged commit 38c2fec into rails:master Jul 25, 2014
@senny
Copy link
Member

senny commented Jul 25, 2014

@mattwarrenrnp thank you.

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

Successfully merging this pull request may close these issues.

None yet

3 participants