Skip to content

HTTPS clone URL

Subversion checkout URL

You can clone with HTTPS or Subversion.

Download ZIP

Loading…

Use type column first in multi-column indexes #14143

Merged
merged 1 commit into from

5 participants

Derek Prior Parker Selbert Rafael Mendonça França Yves Senn Robin Dupret
Derek Prior

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 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 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.

Parker Selbert

This looks straight forward and helpful to me. Nicely documented commit as well. :thumbsup:

Rafael Mendonça França

This depends of your usage. Usually you do PolymorphicClass.find(id), since id is unique.

It would be hard to change this to something that please everyone, so I think we should keep as it is.

Derek Prior

This depends of your usage. Usually you do PolymorphicClass.find(id), since id is unique.

And the change here does not impact that use case at all.

It would be hard to change this to something that please everyone, so I think we should keep as it is.

In what circumstance would you ever want to find a record that has a specific polymorphic_id column without either restricting that by the record's id column or by the polymorphic_type column. I honestly can't think of a single use case for that behavior.

Conversely, I can think of plenty of times where I'd want to find all records of a certain polymorphic_type regardless of the polymorphic_id.

Rafael Mendonça França

We will need a CHANGELOG entry for this change

Derek Prior

@rafaelfranca Changelog added and rebased against master.

activerecord/CHANGELOG.md
((5 lines not shown))
* `create_join_table` removes a common prefix when generating the join table.
This matches the existing behavior of HABTM associations.
+* Make possible to change `record_timestamps` inside Callbacks.
Yves Senn Owner
senny added a note

this entry doesn't look related.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Yves Senn
Owner

This only fixes it half way. We also need to patch the references method. There is also test-case for this.

...erecord/test/cases/migration/references_index_test.rb
@@ -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)
Yves Senn Owner
senny added a note

can you change it to Ruby 1.9 Hash syntax?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Derek Prior derekprior Use type column first in multi-column indexes
`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
9cdd0a1
Derek Prior

@senny, ready for another look. /cc @sgrif

Yves Senn
Owner

@derekprior thank you :yellow_heart:

Yves Senn senny merged commit 9cdd0a1 into from
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Commits on Oct 24, 2014
  1. Derek Prior

    Use type column first in multi-column indexes

    derekprior authored
    `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
This page is out of date. Refresh to see the latest.
4 activerecord/CHANGELOG.md
View
@@ -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"})
2  activerecord/lib/active_record/connection_adapters/abstract/schema_definitions.rb
View
@@ -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
2  activerecord/lib/active_record/connection_adapters/abstract/schema_statements.rb
View
@@ -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
4 activerecord/test/cases/migration/references_index_test.rb
View
@@ -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
@@ -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
2  activerecord/test/cases/migration/references_statements_test.rb
View
@@ -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
Something went wrong with that request. Please try again.