Skip to content

HTTPS clone URL

Subversion checkout URL

You can clone with HTTPS or Subversion.

Download ZIP

Loading…

Additional index type support #6101

Closed
wants to merge 6 commits into from

8 participants

Stefan Huber Alexey Gaziev Steve Klabnik Richard Schneeman Keenan Brock Rafael Mendonça França Tatsuro Baba Jeremy Kemper
Stefan Huber

Added add_index override in postgresql_adapter and mysql_adapters to allow custom index type support.

add_index(:wikis, :body, :method => 'gin')

It's basically an updated version of this older pull request

...tive_record/connection_adapters/postgresql_adapter.rb
@@ -1197,6 +1198,15 @@ def rename_column(table_name, column_name, new_column_name)
execute "ALTER TABLE #{quote_table_name(table_name)} RENAME COLUMN #{quote_column_name(column_name)} TO #{quote_column_name(new_column_name)}"
end
+ def add_index(table_name, column_name, options = {}) #:nodoc:

I am not overly excited about this, but the syntax differs too much to apply this directly in the abstract adapter. :-(

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Alexey Gaziev

nice! :+1:

Steve Klabnik
Collaborator

This will need a rebase, but I know lots of people (like @schneems and @tenderlove) are pumped about supporting specific postgres types.

..._record/connection_adapters/abstract_mysql_adapter.rb
@@ -473,6 +474,15 @@ def rename_column(table_name, column_name, new_column_name) #:nodoc:
execute("ALTER TABLE #{quote_table_name(table_name)} #{rename_column_sql(table_name, column_name, new_column_name)}")
end
+ def add_index(table_name, column_name, options = {}) #:nodoc:
+ if Hash === options && options[:method]
Richard Schneeman Collaborator

I prefer

options.is_a? Hash

over

Hash === options

would also maybe change options[:method] => options[:method].blank? to protect against anyone using an empty string

Rafael Mendonça França Owner

I agree about options.is_a? Hash, but I disagree about the check for blank?. I don't think we need to be so defensive.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Richard Schneeman
Collaborator

I'm :+1: for this feature, being able to specify types of indexes is nice definitely better than having to go in and write it manually:

  def up
    execute "CREATE INDEX products_gin_data ON products USING GIN(data)"
  end

  def down
    execute "DROP INDEX products_gin_data"
  end

I made a comment on code style, do you think you could fix that up, rebase this code, and we can get someone from ActiveRecord to take a look?

Keenan Brock

Object#method(name) is defined.

When trying to backport this to rails 3.2 it gave me a little grief.

Can we pick a different name for it?

The documentation seems to call it index_type for both postgres and my sql. While I don't like having the word index twice (IndexDefinition#index_type), it is more rails friendly than overriding Object#method.

Unfortunately, the create index code currently uses an index_type variable to specify unique. IndexDefinition uses 'unique' for this purpose, so it is only a local variable in 2 places. But a side note none the less

Thoughts?

Rafael Mendonça França
Owner

@kbrock Maybe just type?

Keenan Brock

Heh, type sounds good. I wasn't sure if that was too vague. I'm probably overt hinking this

I notice in the mysql code, it is called :Index_type (with an uppercase i)


So, what is needed to push this forward?

Rafael Mendonça França
Owner

@MSNexploder could you change to :type?

Stefan Huber

Totally forgot about this pull request...
Sure, I will look into it. :)

MSNexploder added some commits
Stefan Huber MSNexploder Merge branch 'master' into additional_index_support
Conflicts:
	activerecord/lib/active_record/connection_adapters/postgresql_adapter.rb
90e29d2
Stefan Huber MSNexploder renamed index method to index type 0e07baf
Tatsuro Baba

:+1: I want this feature to use Trigram full text search on postgresql.

Jeremy Kemper
Owner

Perhaps this should be a :using option to mirror the SQL syntax.

Steve Klabnik
Collaborator

@MSNexploder this will need to be squashed, and a CHANGELOG entry added.

Rafael Mendonça França
Owner

Closed in favor of #9451

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Commits on May 1, 2012
  1. Stefan Huber

    added optional method options to add_index

    MSNexploder authored
    adds custom index method support in PostgreSQL
  2. Stefan Huber

    move tests to proper file

    MSNexploder authored
  3. Stefan Huber

    ocd

    MSNexploder authored
  4. Stefan Huber
Commits on Oct 3, 2012
  1. Stefan Huber

    Merge branch 'master' into additional_index_support

    MSNexploder authored
    Conflicts:
    	activerecord/lib/active_record/connection_adapters/postgresql_adapter.rb
  2. Stefan Huber
This page is out of date. Refresh to see the latest.
2  activerecord/lib/active_record/connection_adapters/abstract/schema_definitions.rb
View
@@ -8,7 +8,7 @@ module ConnectionAdapters #:nodoc:
# Abstract representation of an index definition on a table. Instances of
# this type are typically created and returned by methods in database
# adapters. e.g. ActiveRecord::ConnectionAdapters::AbstractMysqlAdapter#indexes
- class IndexDefinition < Struct.new(:table, :name, :unique, :columns, :lengths, :orders, :where) #:nodoc:
+ class IndexDefinition < Struct.new(:table, :name, :unique, :columns, :lengths, :orders, :where, :type) #:nodoc:
end
# Abstract representation of a column definition. Instances of this type
8 activerecord/lib/active_record/connection_adapters/abstract/schema_statements.rb
View
@@ -386,6 +386,14 @@ def rename_column(table_name, column_name, new_column_name)
#
# Note: only supported by PostgreSQL
#
+ # ====== Creating an index with a specific method
+ # add_index(:developers, :name, :type => 'btree')
+ # generates
+ # CREATE INDEX index_developers_on_name ON developers USING btree (name) -- PostgreSQL
+ # CREATE INDEX index_developers_on_name USING btree ON developers (name) -- MySQL
+ #
+ # Note: only supported by PostgreSQL and MySQL
+ #
def add_index(table_name, column_name, options = {})
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)} (#{index_columns})#{index_options}"
10 activerecord/lib/active_record/connection_adapters/abstract_mysql_adapter.rb
View
@@ -423,6 +423,7 @@ def indexes(table_name, name = nil) #:nodoc:
next if row[:Key_name] == 'PRIMARY' # skip the primary key
current_index = row[:Key_name]
indexes << IndexDefinition.new(row[:Table], row[:Key_name], row[:Non_unique].to_i == 0, [], [])
+ indexes.last.type = row[:Index_type].downcase.to_sym
end
indexes.last.columns << row[:Column_name]
@@ -497,6 +498,15 @@ def rename_column(table_name, column_name, new_column_name) #:nodoc:
execute("ALTER TABLE #{quote_table_name(table_name)} #{rename_column_sql(table_name, column_name, new_column_name)}")
end
+ def add_index(table_name, column_name, options = {}) #:nodoc:
+ if options.is_a?(Hash) && options[:type]
+ 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)} USING #{options[:type]} ON #{quote_table_name(table_name)} (#{index_columns})#{index_options}"
+ else
+ super
+ end
+ end
+
# Maps logical Rails types to MySQL-specific data types.
def type_to_sql(type, limit = nil, precision = nil, scale = nil)
case type.to_s
12 activerecord/lib/active_record/connection_adapters/postgresql/schema_statements.rb
View
@@ -124,8 +124,9 @@ def indexes(table_name, name = nil)
desc_order_columns = inddef.scan(/(\w+) DESC/).flatten
orders = desc_order_columns.any? ? Hash[desc_order_columns.map {|order_column| [order_column, :desc]}] : {}
where = inddef.scan(/WHERE (.+)$/).flatten[0]
+ type = inddef.scan(/USING (.+?) /).flatten[0].to_sym
- column_names.empty? ? nil : IndexDefinition.new(table_name, index_name, unique, column_names, [], orders, where)
+ column_names.empty? ? nil : IndexDefinition.new(table_name, index_name, unique, column_names, [], orders, where, type)
end.compact
end
@@ -377,6 +378,15 @@ def rename_column(table_name, column_name, new_column_name)
execute "ALTER TABLE #{quote_table_name(table_name)} RENAME COLUMN #{quote_column_name(column_name)} TO #{quote_column_name(new_column_name)}"
end
+ def add_index(table_name, column_name, options = {}) #:nodoc:
+ if options.is_a?(Hash) && options[:type]
+ 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)} USING #{options[:type]} (#{index_columns})#{index_options}"
+ else
+ super
+ end
+ end
+
def remove_index!(table_name, index_name) #:nodoc:
execute "DROP INDEX #{quote_table_name(index_name)}"
end
12 activerecord/test/cases/adapters/mysql/active_schema_test.rb
View
@@ -35,6 +35,18 @@ def test_add_index
expected = "CREATE INDEX `index_people_on_last_name_and_first_name` ON `people` (`last_name`(15), `first_name`(10))"
assert_equal expected, add_index(:people, [:last_name, :first_name], :length => {:last_name => 15, :first_name => 10})
+
+ %w(btree hash).each do |type|
+ expected = "CREATE INDEX `index_people_on_last_name` USING #{type} ON `people` (`last_name`)"
+ assert_equal expected, add_index(:people, :last_name, :type => type)
+ end
+
+ expected = "CREATE INDEX `index_people_on_last_name` USING btree ON `people` (`last_name`(10))"
+ assert_equal expected, add_index(:people, :last_name, :length => 10, :type => :btree)
+
+ expected = "CREATE INDEX `index_people_on_last_name_and_first_name` USING btree ON `people` (`last_name`(15), `first_name`(15))"
+ assert_equal expected, add_index(:people, [:last_name, :first_name], :length => 15, :type => :btree)
+
ActiveRecord::ConnectionAdapters::MysqlAdapter.send(:remove_method, :index_name_exists?)
end
19 activerecord/test/cases/adapters/mysql/schema_test.rb
View
@@ -35,6 +35,25 @@ def test_table_exists?
def test_table_exists_wrong_schema
assert(!@connection.table_exists?("#{@db_name}.zomg"), "table should not exist")
end
+
+ def test_dump_indexes
+ index_a_name = 'index_post_title'
+ index_b_name = 'index_post_body'
+
+ table = Post.table_name
+
+ @connection.execute "CREATE INDEX `#{index_a_name}` ON `#{table}` (`title`);"
+ @connection.execute "CREATE INDEX `#{index_b_name}` USING btree ON `#{table}` (`body`(10));"
+
+ indexes = @connection.indexes(table).sort_by {|i| i.name}
+ assert_equal 2,indexes.size
+
+ assert_equal :btree, indexes.select{|i| i.name == index_a_name}[0].type
+ assert_equal :btree, indexes.select{|i| i.name == index_b_name}[0].type
+
+ @connection.execute "DROP INDEX `#{index_a_name}` ON `#{table}`;"
+ @connection.execute "DROP INDEX `#{index_b_name}` ON `#{table}`;"
+ end
end
end
end
12 activerecord/test/cases/adapters/mysql2/active_schema_test.rb
View
@@ -35,6 +35,18 @@ def test_add_index
expected = "CREATE INDEX `index_people_on_last_name_and_first_name` ON `people` (`last_name`(15), `first_name`(10))"
assert_equal expected, add_index(:people, [:last_name, :first_name], :length => {:last_name => 15, :first_name => 10})
+
+ %w(btree hash).each do |type|
+ expected = "CREATE INDEX `index_people_on_last_name` USING #{type} ON `people` (`last_name`)"
+ assert_equal expected, add_index(:people, :last_name, :type => type)
+ end
+
+ expected = "CREATE INDEX `index_people_on_last_name` USING btree ON `people` (`last_name`(10))"
+ assert_equal expected, add_index(:people, :last_name, :length => 10, :type => :btree)
+
+ expected = "CREATE INDEX `index_people_on_last_name_and_first_name` USING btree ON `people` (`last_name`(15), `first_name`(15))"
+ assert_equal expected, add_index(:people, [:last_name, :first_name], :length => 15, :type => :btree)
+
ActiveRecord::ConnectionAdapters::Mysql2Adapter.send(:remove_method, :index_name_exists?)
end
18 activerecord/test/cases/adapters/mysql2/schema_test.rb
View
@@ -46,6 +46,24 @@ def test_tables_quoting
end
end
+ def test_dump_indexes
+ index_a_name = 'index_post_title'
+ index_b_name = 'index_post_body'
+
+ table = Post.table_name
+
+ @connection.execute "CREATE INDEX `#{index_a_name}` ON `#{table}` (`title`);"
+ @connection.execute "CREATE INDEX `#{index_b_name}` USING btree ON `#{table}` (`body`(10));"
+
+ indexes = @connection.indexes(table).sort_by {|i| i.name}
+ assert_equal 2,indexes.size
+
+ assert_equal :btree, indexes.select{|i| i.name == index_a_name}[0].type
+ assert_equal :btree, indexes.select{|i| i.name == index_b_name}[0].type
+
+ @connection.execute "DROP INDEX `#{index_a_name}` ON `#{table}`;"
+ @connection.execute "DROP INDEX `#{index_b_name}` ON `#{table}`;"
+ end
end
end
end
11 activerecord/test/cases/adapters/postgresql/active_schema_test.rb
View
@@ -31,6 +31,17 @@ def test_add_index
expected = %(CREATE UNIQUE INDEX "index_people_on_last_name" ON "people" ("last_name") WHERE state = 'active')
assert_equal expected, add_index(:people, :last_name, :unique => true, :where => "state = 'active'")
+ %w(gin gist hash btree).each do |type|
+ expected = %(CREATE INDEX "index_people_on_last_name" ON "people" USING #{type} ("last_name"))
+ assert_equal expected, add_index(:people, :last_name, :type => type)
+ end
+
+ expected = %(CREATE UNIQUE INDEX "index_people_on_last_name" ON "people" USING gist ("last_name"))
+ assert_equal expected, add_index(:people, :last_name, :unique => true, :type => :gist)
+
+ expected = %(CREATE UNIQUE INDEX "index_people_on_last_name" ON "people" USING gist ("last_name") WHERE state = 'active')
+ assert_equal expected, add_index(:people, :last_name, :unique => true, :where => "state = 'active'", :type => :gist)
+
ActiveRecord::ConnectionAdapters::PostgreSQLAdapter.send(:remove_method, :index_name_exists?)
end
22 activerecord/test/cases/adapters/postgresql/schema_test.rb
View
@@ -11,16 +11,19 @@ class SchemaTest < ActiveRecord::TestCase
INDEX_B_NAME = 'b_index_things_on_different_columns_in_each_schema'
INDEX_C_NAME = 'c_index_full_text_search'
INDEX_D_NAME = 'd_index_things_on_description_desc'
+ INDEX_E_NAME = 'e_index_things_on_name_vector'
INDEX_A_COLUMN = 'name'
INDEX_B_COLUMN_S1 = 'email'
INDEX_B_COLUMN_S2 = 'moment'
INDEX_C_COLUMN = %q{(to_tsvector('english', coalesce(things.name, '')))}
INDEX_D_COLUMN = 'description'
+ INDEX_E_COLUMN = 'name_vector'
COLUMNS = [
'id integer',
'name character varying(50)',
'email character varying(50)',
'description character varying(100)',
+ 'name_vector tsvector',
'moment timestamp without time zone default now()'
]
PK_TABLE_NAME = 'table_with_pk'
@@ -61,6 +64,8 @@ def setup
@connection.execute "CREATE INDEX #{INDEX_C_NAME} ON #{SCHEMA2_NAME}.#{TABLE_NAME} USING gin (#{INDEX_C_COLUMN});"
@connection.execute "CREATE INDEX #{INDEX_D_NAME} ON #{SCHEMA_NAME}.#{TABLE_NAME} USING btree (#{INDEX_D_COLUMN} DESC);"
@connection.execute "CREATE INDEX #{INDEX_D_NAME} ON #{SCHEMA2_NAME}.#{TABLE_NAME} USING btree (#{INDEX_D_COLUMN} DESC);"
+ @connection.execute "CREATE INDEX #{INDEX_E_NAME} ON #{SCHEMA_NAME}.#{TABLE_NAME} USING gin (#{INDEX_E_COLUMN});"
+ @connection.execute "CREATE INDEX #{INDEX_E_NAME} ON #{SCHEMA2_NAME}.#{TABLE_NAME} USING gin (#{INDEX_E_COLUMN});"
@connection.execute "CREATE TABLE #{SCHEMA_NAME}.#{PK_TABLE_NAME} (id serial primary key)"
@connection.execute "CREATE SEQUENCE #{SCHEMA_NAME}.#{UNMATCHED_SEQUENCE_NAME}"
@connection.execute "CREATE TABLE #{SCHEMA_NAME}.#{UNMATCHED_PK_TABLE_NAME} (id integer NOT NULL DEFAULT nextval('#{SCHEMA_NAME}.#{UNMATCHED_SEQUENCE_NAME}'::regclass), CONSTRAINT unmatched_pkey PRIMARY KEY (id))"
@@ -97,7 +102,7 @@ def test_raise_create_schema_with_existing_schema
def test_drop_schema
begin
- @connection.create_schema "test_schema3"
+ @connection.create_schema "test_schema3"
ensure
@connection.drop_schema "test_schema3"
end
@@ -236,15 +241,15 @@ def test_ignore_nil_schema_search_path
end
def test_dump_indexes_for_schema_one
- do_dump_index_tests_for_schema(SCHEMA_NAME, INDEX_A_COLUMN, INDEX_B_COLUMN_S1, INDEX_D_COLUMN)
+ do_dump_index_tests_for_schema(SCHEMA_NAME, INDEX_A_COLUMN, INDEX_B_COLUMN_S1, INDEX_D_COLUMN, INDEX_E_COLUMN)
end
def test_dump_indexes_for_schema_two
- do_dump_index_tests_for_schema(SCHEMA2_NAME, INDEX_A_COLUMN, INDEX_B_COLUMN_S2, INDEX_D_COLUMN)
+ do_dump_index_tests_for_schema(SCHEMA2_NAME, INDEX_A_COLUMN, INDEX_B_COLUMN_S2, INDEX_D_COLUMN, INDEX_E_COLUMN)
end
def test_dump_indexes_for_schema_multiple_schemas_in_search_path
- do_dump_index_tests_for_schema("public, #{SCHEMA_NAME}", INDEX_A_COLUMN, INDEX_B_COLUMN_S1, INDEX_D_COLUMN)
+ do_dump_index_tests_for_schema("public, #{SCHEMA_NAME}", INDEX_A_COLUMN, INDEX_B_COLUMN_S1, INDEX_D_COLUMN, INDEX_E_COLUMN)
end
def test_with_uppercase_index_name
@@ -344,15 +349,20 @@ def with_schema_search_path(schema_search_path)
@connection.schema_search_path = "'$user', public"
end
- def do_dump_index_tests_for_schema(this_schema_name, first_index_column_name, second_index_column_name, third_index_column_name)
+ def do_dump_index_tests_for_schema(this_schema_name, first_index_column_name, second_index_column_name, third_index_column_name, fourth_index_column_name)
with_schema_search_path(this_schema_name) do
indexes = @connection.indexes(TABLE_NAME).sort_by {|i| i.name}
- assert_equal 3,indexes.size
+ assert_equal 4,indexes.size
do_dump_index_assertions_for_one_index(indexes[0], INDEX_A_NAME, first_index_column_name)
do_dump_index_assertions_for_one_index(indexes[1], INDEX_B_NAME, second_index_column_name)
do_dump_index_assertions_for_one_index(indexes[2], INDEX_D_NAME, third_index_column_name)
+ do_dump_index_assertions_for_one_index(indexes[3], INDEX_E_NAME, fourth_index_column_name)
+ indexes.select{|i| i.name != INDEX_E_NAME}.each do |index|
+ assert_equal :btree, index.type
+ end
+ assert_equal :gin, indexes.select{|i| i.name == INDEX_E_NAME}[0].type
assert_equal :desc, indexes.select{|i| i.name == INDEX_D_NAME}[0].orders[INDEX_D_COLUMN]
end
end
Something went wrong with that request. Please try again.