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

Support multiple indexes on the same column when loading the schema #26019

Merged

Conversation

agrobbin
Copy link
Contributor

@agrobbin agrobbin commented Aug 2, 2016

We use Postgres for quite a few of our Rails applications, and one of the things we use constantly are partial unique indices, allowing us to have multiple indices on 1 column with different restrictions based on other column data. While everything in our production systems works fine, and adding new partial unique indices via migrations also works fine, loading multiple indices on the same column (String or Array) via our db/schema.rb file turns out to not work quite as expected.

If you have a schema like this:

ActiveRecord::Schema.define do
  create_table :sponsorships do |t|
    t.string "user_id"
    t.index ["user_id"], name: "index_sponsorships_on_user_id_unique", unique: true, where: "(status = 1)", using: :btree
    t.index ["user_id"], name: "index_sponsorships_on_user_id", using: :btree
  end
end

When you run rails db:schema:load without this patch, your local database will only include the index_sponsorships_on_user_id index, skipping the index_sponsorships_on_user_id_unique index.

With this patch, all indices are loaded into the database, regardless of whether the columns in question already have an index.

As far as I can tell, this bug affects at least Rails 4.0+.

@rails-bot
Copy link

r? @matthewd

(@rails-bot has picked a reviewer for you, use r? to override)

@@ -328,7 +328,7 @@ def remove_column(name)
#
# index(:account_id, name: 'index_projects_on_account_id')
def index(column_name, options = {})
indexes[column_name] = options
indexes << options.merge(column_name: column_name)
Copy link
Member

@kamipo kamipo Aug 2, 2016

Choose a reason for hiding this comment

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

How about it?

       def index(column_name, options = {})
-        indexes[column_name] = options
+        indexes << [column_name, options]
       end

https://github.com/rails/rails/pull/26019/files#diff-21d4fbe002689dc4b0ab29f021585457L338

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Oh, duh, totally forgot that iterating over an array of arrays will yield each item in the subarray to each. Much simpler change, thanks!

@agrobbin agrobbin force-pushed the schema-load-unique-column-indices branch from b26143c to 9b3f329 Compare August 2, 2016 12:00
@agrobbin
Copy link
Contributor Author

@matthewd @maclover7 would love to get this reviewed and merged into Rails core. Any chance one of you has some time to take a look?

@rafaelfranca rafaelfranca merged commit 3e9070e into rails:master Aug 16, 2016
rafaelfranca added a commit that referenced this pull request Aug 16, 2016
…ices

Support multiple indexes on the same column when loading the schema
@rafaelfranca
Copy link
Member

Backported in 809757f

@agrobbin
Copy link
Contributor Author

Thanks @rafaelfranca!

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.

None yet

6 participants