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

using btree removed from indexes #92

Closed
abatko opened this issue Dec 8, 2013 · 3 comments
Closed

using btree removed from indexes #92

abatko opened this issue Dec 8, 2013 · 3 comments

Comments

@abatko
Copy link

abatko commented Dec 8, 2013

Mostly as per your blog posts (http://blog.daniel-azuma.com/archives/69 and http://blog.daniel-azuma.com/archives/164#more-164), I am adding rgeo support to my rails application (which happens to include an out-of-the-box configuration of devise and rails_admin).

Consider the following migration:

class AddGeopointToPhotos < ActiveRecord::Migration
  def change
    add_column :photos, :geopoint, :point, srid: 3785
    add_index :photos, :geopoint, spatial: true
  end
end

After running bundle exec rake db:migrate, here's what git diff db/schema.rb shows:

...

+  add_index "photos", ["geopoint"], :name => "index_photos_on_geopoint", :spatial => true
+

...

-  add_index "rails_admin_histories", ["item", "table", "month", "year"], name: "index_rails_admin_histories", using: :btree
+  add_index "rails_admin_histories", ["item", "table", "month", "year"], :name => "index_rails_admin_histories"

...

-  add_index "users", ["email"], name: "index_users_on_email", unique: true, using: :btree
-  add_index "users", ["reset_password_token"], name: "index_users_on_reset_password_token", unique: true, using: :btree
+  add_index "users", ["email"], :name => "index_users_on_email", :unique => true
+  add_index "users", ["reset_password_token"], :name => "index_users_on_reset_password_token", :unique => true

Why was using: :btree removed from rails_admin_histories and users indexes?

@elsurudo
Copy link

elsurudo commented Jan 7, 2014

I just noticed the same thing...

@undergroundwebdesigns
Copy link

I've also noticed the same thing. Specifically, it appears this extension overwrites the add_index method completely, and doesn't include the new using: option, thus disallowing that customization.

I'm 90% sure that this is the code responsible (from main_adapter.rb):

def add_index(table_name, column_name, options = {})
# FULL REPLACEMENT. RE-CHECK ON NEW VERSIONS.
# We have to fully-replace because of the gist_clause.
options ||= {}
gist_clause = options.delete(:spatial) ? ' USING GIST' : ''
index_name, index_type, index_columns, index_options = add_index_options(table_name, column_name, options)
execute "CREATE #{index_type} INDEX #{quote_column_name(index_name)} ON #{quote_table_name(table_name)}#{gist_clause} (#{index_columns})#{index_options}"
end

As per the comment, it appears someone was aware that this might cause issues. I believe a simple fix would be to simply check for the :spatial option and replace it with :using => :gist before calling super, but I'm not 100% and unfortunately don't have time to test right now. I'll try to test properly and submit a pull request when I have some free time, but leaving this here in case someone else wants to jump on it first.

@teeparham
Copy link
Member

Fixed by 05c553fe620e513

Added tests: 789e06f37d5d

This fix is included in 2.0.1.

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

No branches or pull requests

4 participants