From 79874883914286e6486e23575e085d0f5031615f Mon Sep 17 00:00:00 2001 From: Matt Jones Date: Mon, 2 Nov 2009 00:00:39 -0500 Subject: [PATCH] make add_index and remove_index more resilient; new rename_index method; track database limits --- .../abstract/database_limits.rb | 57 ++++++++++++++++++++ .../abstract/schema_statements.rb | 41 ++++++++++++-- .../connection_adapters/abstract_adapter.rb | 2 + .../connection_adapters/postgresql_adapter.rb | 7 ++- .../connection_adapters/sqlite_adapter.rb | 7 ++- activerecord/test/cases/migration_test.rb | 25 +++++++++ 6 files changed, 131 insertions(+), 8 deletions(-) create mode 100644 activerecord/lib/active_record/connection_adapters/abstract/database_limits.rb diff --git a/activerecord/lib/active_record/connection_adapters/abstract/database_limits.rb b/activerecord/lib/active_record/connection_adapters/abstract/database_limits.rb new file mode 100644 index 0000000..0fbe3fd --- /dev/null +++ b/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 \ No newline at end of file diff --git a/activerecord/lib/active_record/connection_adapters/abstract/schema_statements.rb b/activerecord/lib/active_record/connection_adapters/abstract/schema_statements.rb index 7e82099..87112e1 100644 --- a/activerecord/lib/active_record/connection_adapters/abstract/schema_statements.rb +++ b/activerecord/lib/active_record/connection_adapters/abstract/schema_statements.rb @@ -8,11 +8,6 @@ module ActiveRecord {} 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(/\./, '_') @@ -268,6 +263,14 @@ module ActiveRecord else 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 = column_names.map { |e| quote_column_name(e) }.join(", ") execute "CREATE #{index_type} INDEX #{quote_column_name(index_name)} ON #{quote_table_name(table_name)} (#{quoted_column_names})" end @@ -283,7 +286,24 @@ module ActiveRecord # 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 #{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 + 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: @@ -300,6 +320,15 @@ module ActiveRecord 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 CREATE TABLE SQL statement(s) for recreating the # entire structure of the database. def structure_dump diff --git a/activerecord/lib/active_record/connection_adapters/abstract_adapter.rb b/activerecord/lib/active_record/connection_adapters/abstract_adapter.rb index 22871f2..b225776 100755 --- a/activerecord/lib/active_record/connection_adapters/abstract_adapter.rb +++ b/activerecord/lib/active_record/connection_adapters/abstract_adapter.rb @@ -11,6 +11,7 @@ require 'active_record/connection_adapters/abstract/quoting' 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 ActiveRecord # 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 diff --git a/activerecord/lib/active_record/connection_adapters/postgresql_adapter.rb b/activerecord/lib/active_record/connection_adapters/postgresql_adapter.rb index bc289ff..f0dead2 100644 --- a/activerecord/lib/active_record/connection_adapters/postgresql_adapter.rb +++ b/activerecord/lib/active_record/connection_adapters/postgresql_adapter.rb @@ -891,7 +891,12 @@ module ActiveRecord # Drops an index from a table. def remove_index(table_name, options = {}) - execute "DROP INDEX #{quote_table_name(index_name(table_name, options))}" + 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 + execute "DROP INDEX #{quote_table_name(index_name)}" end # Maps logical Rails types to PostgreSQL-specific data types. diff --git a/activerecord/lib/active_record/connection_adapters/sqlite_adapter.rb b/activerecord/lib/active_record/connection_adapters/sqlite_adapter.rb index 0bf97a9..1a838dd 100644 --- a/activerecord/lib/active_record/connection_adapters/sqlite_adapter.rb +++ b/activerecord/lib/active_record/connection_adapters/sqlite_adapter.rb @@ -245,7 +245,12 @@ module ActiveRecord end def remove_index(table_name, options={}) #:nodoc: - execute "DROP INDEX #{quote_column_name(index_name(table_name, options))}" + 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 + execute "DROP INDEX #{quote_column_name(index_name)}" end def rename_table(name, new_name) diff --git a/activerecord/test/cases/migration_test.rb b/activerecord/test/cases/migration_test.rb index 3b6326d..956f707 100644 --- a/activerecord/test/cases/migration_test.rb +++ b/activerecord/test/cases/migration_test.rb @@ -111,6 +111,31 @@ if ActiveRecord::Base.connection.supports_migrations? end 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 -- 1.5.3.1