search

mbleigh / acts-as-taggable-on

A tagging plugin for Rails applications that allows for custom tagging along dynamic contexts.
http://mbleigh.lighthouseapp.com/projects/10116-acts-as-taggable-on
MIT License
4.96k stars 1.19k forks source link

migration 6_add_missing_indexes_on_taggings redundant index? #1052

Open akostadinov opened 2 years ago

akostadinov commented 2 years ago

In 6_add_missing_indexes_on_taggings.rb [1] I see two seemingly redundant indices created:

add_index ActsAsTaggableOn.taggings_table, :tagger_id
add_index ActsAsTaggableOn.taggings_table, [:tagger_id, :tagger_type]

Same goes for

add_index ActsAsTaggableOn.taggings_table, :taggable_id
add_index ActsAsTaggableOn.taggings_table, [:taggable_id, :taggable_type, :tagger_id, :context], name: 'taggings_idy'

And there is one additional reduntant one from 2_add_missing_unique_indices.rb#L12:

    add_index ActsAsTaggableOn.taggings_table, :tag_id unless index_exists? ActsAsTaggableOn.taggings_table, :tag_id
vs
    add_index ActsAsTaggableOn.taggings_table,
              [:tag_id, :taggable_id, :taggable_type, :context, :tagger_id, :tagger_type],
              unique: true, name: 'taggings_idx'

Because the composite index starts with :tagger_id it seems that a separate index just on :tagger_id is probably redundant because DB engine can use the composite index when querying tagger_id [2]. Same as the other two cases.

Did any practical results showed that both indices are needed or is there any other reason to have them both?

P.S. The rollback of this migration does nothing because everything is in if condition. So when running rollback nothing is rolled back.

[1] https://github.com/mbleigh/acts-as-taggable-on/blob/072767bd03b43b18555f58da2c754096e5e82aca/db/migrate/6_add_missing_indexes_on_taggings.rb#L11-L15 [2] https://user3141592.medium.com/single-vs-composite-indexes-in-relational-databases-58d0eb045cbe

gerard76 commented 2 years ago

I came here wondering about the redundant indices as well. Both MySQL and Postgresql can use the composite indices for those.

fatkodima commented 2 years ago

is probably redundant because DB engine can use the composite index when querying

It is not "probably", it is for sure in every relational dbms.

These indexes are redundant. Unfortunately, this gem has some problems with correct/non-redundant/useful indexes and PRs involving adding new indexes should be better reviewed. And overall all existing indexes should be reviewed.