Skip to content

Commit

Permalink
Use type column first in multi-column indexes
Browse files Browse the repository at this point in the history
`add_reference` can very helpfully add a multi-column index when you use
it to add a polymorphic reference. However, the first column in the
index is the `id` column, which is less than ideal.

The [PostgreSQL docs][1] say:
> A multicolumn B-tree index can be used with query conditions that
> involve any subset of the index's columns, but the index is most
> efficient when there are constraints on the leading (leftmost)
> columns.

The [MySQL docs][2] say:
> MySQL can use multiple-column indexes for queries that test all the
> columns in the index, or queries that test just the first column, the
> first two columns, the first three columns, and so on. If you specify
> the columns in the right order in the index definition, a single
> composite index can speed up several kinds of queries on the same
> table.

In a polymorphic relationship, the type column is much more likely to be
useful as the first column in an index than the id column. That is, I'm
more likely to query on type without an id than I am to query on id
without a type.

[1]: http://www.postgresql.org/docs/9.3/static/indexes-multicolumn.html
[2]: http://dev.mysql.com/doc/refman/5.0/en/multiple-column-indexes.html
  • Loading branch information
derekprior committed Oct 24, 2014
1 parent 85faea4 commit 9cdd0a1
Show file tree
Hide file tree
Showing 5 changed files with 9 additions and 5 deletions.
4 changes: 4 additions & 0 deletions activerecord/CHANGELOG.md
@@ -1,3 +1,7 @@
* Use type column first in multi-column indexes created with `add-reference`

*Derek Prior*

* `AR::UnknownAttributeError` now includes the class name of a record.

User.new(name: "Yuki Nishijima", project_attributes: {name: "kaminari"})
Expand Down
Expand Up @@ -312,7 +312,7 @@ def references(*args)
args.each do |col|
column("#{col}_id", type, options)
column("#{col}_type", :string, polymorphic.is_a?(Hash) ? polymorphic : options) if polymorphic
index(polymorphic ? %w(id type).map { |t| "#{col}_#{t}" } : "#{col}_id", index_options.is_a?(Hash) ? index_options : {}) if index_options
index(polymorphic ? %w(type id).map { |t| "#{col}_#{t}" } : "#{col}_id", index_options.is_a?(Hash) ? index_options : {}) if index_options
end
end
alias :belongs_to :references
Expand Down
Expand Up @@ -629,7 +629,7 @@ def add_reference(table_name, ref_name, options = {})
type = options.delete(:type) || :integer
add_column(table_name, "#{ref_name}_id", type, options)
add_column(table_name, "#{ref_name}_type", :string, polymorphic.is_a?(Hash) ? polymorphic : options) if polymorphic
add_index(table_name, polymorphic ? %w[id type].map{ |t| "#{ref_name}_#{t}" } : "#{ref_name}_id", index_options.is_a?(Hash) ? index_options : {}) if index_options
add_index(table_name, polymorphic ? %w[type id].map{ |t| "#{ref_name}_#{t}" } : "#{ref_name}_id", index_options.is_a?(Hash) ? index_options : {}) if index_options
end
alias :add_belongs_to :add_reference

Expand Down
4 changes: 2 additions & 2 deletions activerecord/test/cases/migration/references_index_test.rb
Expand Up @@ -55,7 +55,7 @@ def test_creates_polymorphic_index
t.references :foo, :polymorphic => true, :index => true
end

assert connection.index_exists?(table_name, [:foo_id, :foo_type], :name => :index_testings_on_foo_id_and_foo_type)
assert connection.index_exists?(table_name, [:foo_type, :foo_id], name: :index_testings_on_foo_type_and_foo_id)
end
end

Expand Down Expand Up @@ -93,7 +93,7 @@ def test_creates_polymorphic_index_for_existing_table
t.references :foo, :polymorphic => true, :index => true
end

assert connection.index_exists?(table_name, [:foo_id, :foo_type], :name => :index_testings_on_foo_id_and_foo_type)
assert connection.index_exists?(table_name, [:foo_type, :foo_id], name: :index_testings_on_foo_type_and_foo_id)
end
end
end
Expand Down
Expand Up @@ -42,7 +42,7 @@ def test_does_not_create_reference_id_index

def test_creates_polymorphic_index
add_reference table_name, :taggable, polymorphic: true, index: true
assert index_exists?(table_name, [:taggable_id, :taggable_type])
assert index_exists?(table_name, [:taggable_type, :taggable_id])
end

def test_creates_reference_type_column_with_default
Expand Down

0 comments on commit 9cdd0a1

Please sign in to comment.