Skip to content

HTTPS clone URL

Subversion checkout URL

You can clone with
or
.
Download ZIP
Browse files

make add_index and remove_index more resilient; new rename_index meth…

…od; track database limits

[#3452 state:committed]

Signed-off-by: Jeremy Kemper <jeremy@bitsweat.net>
  • Loading branch information...
commit 99bcce7ec1e7f59db3e6f1e1d3cd02e15eb59602 1 parent a5696e3
@etiennebarrie etiennebarrie authored jeremy committed
View
57 activerecord/lib/active_record/connection_adapters/abstract/database_limits.rb
@@ -0,0 +1,57 @@
+module ActiveRecord
+ module ConnectionAdapters # :nodoc:
+ module DatabaseLimits
+
+ # the maximum length of a table alias
+ def table_alias_length
+ 255
+ end
+
+ # the maximum length of a column name
+ def column_name_length
+ 64
+ end
+
+ # the maximum length of a table name
+ def table_name_length
+ 64
+ end
+
+ # the maximum length of an index name
+ def index_name_length
+ 64
+ end
+
+ # the maximum number of columns per table
+ def columns_per_table
+ 1024
+ end
+
+ # the maximum number of indexes per table
+ def indexes_per_table
+ 16
+ end
+
+ # the maximum number of columns in a multicolumn index
+ def columns_per_multicolumn_index
+ 16
+ end
+
+ # the maximum number of elements in an IN (x,y,z) clause
+ def in_clause_length
+ 65535
+ end
+
+ # the maximum length of a SQL query
+ def sql_query_length
+ 1048575
+ end
+
+ # maximum number of joins in a single query
+ def joins_per_query
+ 256
+ end
+
+ end
+ end
+end
View
45 activerecord/lib/active_record/connection_adapters/abstract/schema_statements.rb
@@ -8,11 +8,6 @@ def native_database_types
{}
end
- # This is the maximum length a table alias can be
- def table_alias_length
- 255
- end
-
# Truncates a table alias according to the limits of the current adapter.
def table_alias_for(table_name)
table_name[0..table_alias_length-1].gsub(/\./, '_')
@@ -283,6 +278,14 @@ def add_index(table_name, column_name, options = {})
index_type = options
end
+ if index_name.length > index_name_length
+ @logger.warn("Index name '#{index_name}' on table '#{table_name}' is too long; the limit is #{index_name_length} characters. Skipping.")
+ return
+ end
+ if index_exists?(table_name, index_name, false)
+ @logger.warn("Index name '#{index_name}' on table '#{table_name}' already exists. Skipping.")
+ return
+ end
quoted_column_names = quoted_columns_for_index(column_names, options).join(", ")
execute "CREATE #{index_type} INDEX #{quote_column_name(index_name)} ON #{quote_table_name(table_name)} (#{quoted_column_names})"
@@ -299,7 +302,28 @@ def add_index(table_name, column_name, options = {})
# Remove the index named by_branch_party in the accounts table.
# remove_index :accounts, :name => :by_branch_party
def remove_index(table_name, options = {})
- execute "DROP INDEX #{quote_column_name(index_name(table_name, options))} ON #{quote_table_name(table_name)}"
+ index_name = index_name(table_name, options)
+ unless index_exists?(table_name, index_name, true)
+ @logger.warn("Index name '#{index_name}' on table '#{table_name}' does not exist. Skipping.")
+ return
+ end
+ remove_index!(table_name, index_name)
+ end
+
+ def remove_index!(table_name, index_name) #:nodoc:
+ execute "DROP INDEX #{quote_column_name(index_name)} ON #{table_name}"
+ end
+
+ # Rename an index.
+ #
+ # Rename the index_people_on_last_name index to index_users_on_last_name
+ # rename_index :people, 'index_people_on_last_name', 'index_users_on_last_name'
+ def rename_index(table_name, old_name, new_name)
+ # this is a naive implementation; some DBs may support this more efficiently (Postgres, for instance)
+ old_index_def = indexes(table_name).detect { |i| i.name == old_name }
+ return unless old_index_def
+ remove_index(table_name, :name => old_name)
+ add_index(table_name, old_index_def.columns, :name => new_name, :unique => old_index_def.unique)
end
def index_name(table_name, options) #:nodoc:
@@ -316,6 +340,15 @@ def index_name(table_name, options) #:nodoc:
end
end
+ # Verify the existence of an index.
+ #
+ # The default argument is returned if the underlying implementation does not define the indexes method,
+ # as there's no way to determine the correct answer in that case.
+ def index_exists?(table_name, index_name, default)
+ return default unless respond_to?(:indexes)
+ indexes(table_name).detect { |i| i.name == index_name }
+ end
+
# Returns a string of <tt>CREATE TABLE</tt> SQL statement(s) for recreating the
# entire structure of the database.
def structure_dump
View
2  activerecord/lib/active_record/connection_adapters/abstract_adapter.rb
@@ -11,6 +11,7 @@
require 'active_record/connection_adapters/abstract/connection_pool'
require 'active_record/connection_adapters/abstract/connection_specification'
require 'active_record/connection_adapters/abstract/query_cache'
+require 'active_record/connection_adapters/abstract/database_limits'
module ActiveRecord
module ConnectionAdapters # :nodoc:
@@ -29,6 +30,7 @@ module ConnectionAdapters # :nodoc:
# notably, the instance methods provided by SchemaStatement are very useful.
class AbstractAdapter
include Quoting, DatabaseStatements, SchemaStatements
+ include DatabaseLimits
include QueryCache
include ActiveSupport::Callbacks
define_callbacks :checkout, :checkin
View
9 activerecord/lib/active_record/connection_adapters/postgresql_adapter.rb
@@ -859,9 +859,12 @@ 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
- # Drops an index from a table.
- def remove_index(table_name, options = {})
- execute "DROP INDEX #{quote_table_name(index_name(table_name, options))}"
+ def remove_index!(table_name, index_name) #:nodoc:
+ execute "DROP INDEX #{quote_table_name(index_name)}"
+ end
+
+ def index_name_length
+ 63
end
# Maps logical Rails types to PostgreSQL-specific data types.
View
4 activerecord/lib/active_record/connection_adapters/sqlite_adapter.rb
@@ -244,8 +244,8 @@ def primary_key(table_name) #:nodoc:
column ? column['name'] : nil
end
- def remove_index(table_name, options={}) #:nodoc:
- execute "DROP INDEX #{quote_column_name(index_name(table_name, options))}"
+ def remove_index!(table_name, index_name) #:nodoc:
+ execute "DROP INDEX #{quote_column_name(index_name)}"
end
def rename_table(name, new_name)
View
5 activerecord/test/cases/active_schema_test_mysql.rb
@@ -16,6 +16,10 @@ def teardown
end
def test_add_index
+ # add_index calls index_exists? which can't work since execute is stubbed
+ ActiveRecord::ConnectionAdapters::MysqlAdapter.send(:define_method, :index_exists?) do |*|
+ false
+ end
expected = "CREATE INDEX `index_people_on_last_name` ON `people` (`last_name`)"
assert_equal expected, add_index(:people, :last_name, :length => nil)
@@ -30,6 +34,7 @@ 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})
+ ActiveRecord::ConnectionAdapters::MysqlAdapter.send(:remove_method, :index_exists?)
end
def test_drop_table
View
34 activerecord/test/cases/migration_test.rb
@@ -119,6 +119,40 @@ def test_add_index
end
end
+ def test_add_index_length_limit
+ good_index_name = 'x' * Person.connection.index_name_length
+ too_long_index_name = good_index_name + 'x'
+ assert_nothing_raised { Person.connection.add_index("people", "first_name", :name => too_long_index_name) }
+ assert !Person.connection.index_exists?("people", too_long_index_name, false)
+ assert_nothing_raised { Person.connection.add_index("people", "first_name", :name => good_index_name) }
+ assert Person.connection.index_exists?("people", good_index_name, false)
+ end
+
+ def test_remove_nonexistent_index
+ # we do this by name, so OpenBase is a wash as noted above
+ unless current_adapter?(:OpenBaseAdapter)
+ assert_nothing_raised { Person.connection.remove_index("people", "no_such_index") }
+ end
+ end
+
+ def test_rename_index
+ unless current_adapter?(:OpenBaseAdapter)
+ # keep the names short to make Oracle and similar behave
+ Person.connection.add_index('people', [:first_name], :name => 'old_idx')
+ assert_nothing_raised { Person.connection.rename_index('people', 'old_idx', 'new_idx') }
+ # if the adapter doesn't support the indexes call, pick defaults that let the test pass
+ assert !Person.connection.index_exists?('people', 'old_idx', false)
+ assert Person.connection.index_exists?('people', 'new_idx', true)
+ end
+ end
+
+ def test_double_add_index
+ unless current_adapter?(:OpenBaseAdapter)
+ Person.connection.add_index('people', [:first_name], :name => 'some_idx')
+ assert_nothing_raised { Person.connection.add_index('people', [:first_name], :name => 'some_idx') }
+ end
+ end
+
def testing_table_with_only_foo_attribute
Person.connection.create_table :testings, :id => false do |t|
t.column :foo, :string
View
4 activerecord/test/cases/schema_test_postgresql.rb
@@ -137,11 +137,11 @@ def test_dump_indexes_for_schema_two
def test_with_uppercase_index_name
ActiveRecord::Base.connection.execute "CREATE INDEX \"things_Index\" ON #{SCHEMA_NAME}.things (name)"
- assert_nothing_raised { ActiveRecord::Base.connection.remove_index :things, :name => "#{SCHEMA_NAME}.things_Index"}
+ assert_nothing_raised { ActiveRecord::Base.connection.remove_index! "things", "#{SCHEMA_NAME}.things_Index"}
ActiveRecord::Base.connection.execute "CREATE INDEX \"things_Index\" ON #{SCHEMA_NAME}.things (name)"
ActiveRecord::Base.connection.schema_search_path = SCHEMA_NAME
- assert_nothing_raised { ActiveRecord::Base.connection.remove_index :things, :name => "things_Index"}
+ assert_nothing_raised { ActiveRecord::Base.connection.remove_index! "things", "things_Index"}
ActiveRecord::Base.connection.schema_search_path = "public"
end
Please sign in to comment.
Something went wrong with that request. Please try again.