diff options
16 files changed, 160 insertions, 48 deletions
diff --git a/activerecord/CHANGELOG.md b/activerecord/CHANGELOG.md index 55c4214eee..c34236d4be 100644 --- a/activerecord/CHANGELOG.md +++ b/activerecord/CHANGELOG.md @@ -1,3 +1,9 @@ +* Use given algorithm while removing index from database. + + Fixes #24190. + + *Mehmet Emin İNAÇ* + * Update payload names for `sql.active_record` instrumentation to be more descriptive. diff --git a/activerecord/lib/active_record/connection_adapters/abstract/schema_definitions.rb b/activerecord/lib/active_record/connection_adapters/abstract/schema_definitions.rb index 788a455773..1041db0b8f 100644 --- a/activerecord/lib/active_record/connection_adapters/abstract/schema_definitions.rb +++ b/activerecord/lib/active_record/connection_adapters/abstract/schema_definitions.rb @@ -410,6 +410,10 @@ module ActiveRecord def aliased_types(name, fallback) "timestamp" == name ? :datetime : fallback end + + def integer_like_primary_key?(type, options) + options[:primary_key] && [:integer, :bigint].include?(type) && !options.key?(:default) + end end class AlterTable # :nodoc: diff --git a/activerecord/lib/active_record/connection_adapters/abstract_mysql_adapter.rb b/activerecord/lib/active_record/connection_adapters/abstract_mysql_adapter.rb index 7cd086084a..ae991d3d79 100644 --- a/activerecord/lib/active_record/connection_adapters/abstract_mysql_adapter.rb +++ b/activerecord/lib/active_record/connection_adapters/abstract_mysql_adapter.rb @@ -284,7 +284,7 @@ module ActiveRecord def table_comment(table_name) # :nodoc: scope = quoted_scope(table_name) - query_value(<<-SQL.strip_heredoc, "SCHEMA") + query_value(<<-SQL.strip_heredoc, "SCHEMA").presence SELECT table_comment FROM information_schema.tables WHERE table_schema = #{scope[:schema]} @@ -311,6 +311,11 @@ module ActiveRecord execute("ALTER TABLE #{quote_table_name(table_name)} #{sqls}") end + def change_table_comment(table_name, comment) #:nodoc: + comment = "" if comment.nil? + execute("ALTER TABLE #{quote_table_name(table_name)} COMMENT #{quote(comment)}") + end + # Renames a table. # # Example: @@ -351,18 +356,19 @@ module ActiveRecord def change_column_default(table_name, column_name, default_or_changes) #:nodoc: default = extract_new_default_value(default_or_changes) - column = column_for(table_name, column_name) - change_column table_name, column_name, column.sql_type, default: default + change_column table_name, column_name, nil, default: default end def change_column_null(table_name, column_name, null, default = nil) #:nodoc: - column = column_for(table_name, column_name) - unless null || default.nil? execute("UPDATE #{quote_table_name(table_name)} SET #{quote_column_name(column_name)}=#{quote(default)} WHERE #{quote_column_name(column_name)} IS NULL") end - change_column table_name, column_name, column.sql_type, null: null + change_column table_name, column_name, nil, null: null + end + + def change_column_comment(table_name, column_name, comment) #:nodoc: + change_column table_name, column_name, nil, comment: comment end def change_column(table_name, column_name, type, options = {}) #:nodoc: @@ -668,6 +674,7 @@ module ActiveRecord def change_column_sql(table_name, column_name, type, options = {}) column = column_for(table_name, column_name) + type ||= column.sql_type unless options.key?(:default) options[:default] = column.default @@ -716,7 +723,7 @@ module ActiveRecord def remove_index_sql(table_name, options = {}) index_name = index_name_for_remove(table_name, options) - "DROP INDEX #{index_name}" + "DROP INDEX #{quote_column_name(index_name)}" end def add_timestamps_sql(table_name, options = {}) diff --git a/activerecord/lib/active_record/connection_adapters/mysql/schema_definitions.rb b/activerecord/lib/active_record/connection_adapters/mysql/schema_definitions.rb index b22a2e4da7..eff96af87a 100644 --- a/activerecord/lib/active_record/connection_adapters/mysql/schema_definitions.rb +++ b/activerecord/lib/active_record/connection_adapters/mysql/schema_definitions.rb @@ -4,11 +4,6 @@ module ActiveRecord module ConnectionAdapters module MySQL module ColumnMethods - def primary_key(name, type = :primary_key, **options) - options[:auto_increment] = true if [:integer, :bigint].include?(type) && !options.key?(:default) - super - end - def blob(*args, **options) args.each { |name| column(name, :blob, options) } end @@ -62,6 +57,10 @@ module ActiveRecord include ColumnMethods def new_column_definition(name, type, **options) # :nodoc: + if integer_like_primary_key?(type, options) + options[:auto_increment] = true + end + case type when :virtual type = options[:type] diff --git a/activerecord/lib/active_record/connection_adapters/postgresql/schema_definitions.rb b/activerecord/lib/active_record/connection_adapters/postgresql/schema_definitions.rb index f1489e4d69..cb13f9fec1 100644 --- a/activerecord/lib/active_record/connection_adapters/postgresql/schema_definitions.rb +++ b/activerecord/lib/active_record/connection_adapters/postgresql/schema_definitions.rb @@ -44,15 +44,8 @@ module ActiveRecord # a record (as primary keys cannot be +nil+). This might be done via the # +SecureRandom.uuid+ method and a +before_save+ callback, for instance. def primary_key(name, type = :primary_key, **options) - options[:auto_increment] = true if [:integer, :bigint].include?(type) && !options.key?(:default) if type == :uuid options[:default] = options.fetch(:default, "gen_random_uuid()") - elsif options.delete(:auto_increment) == true && %i(integer bigint).include?(type) - type = if type == :bigint || options[:limit] == 8 - :bigserial - else - :serial - end end super @@ -185,6 +178,18 @@ module ActiveRecord class TableDefinition < ActiveRecord::ConnectionAdapters::TableDefinition include ColumnMethods + + def new_column_definition(name, type, **options) # :nodoc: + if integer_like_primary_key?(type, options) + type = if type == :bigint || options[:limit] == 8 + :bigserial + else + :serial + end + end + + super + end end class Table < ActiveRecord::ConnectionAdapters::Table diff --git a/activerecord/lib/active_record/connection_adapters/sqlite3/schema_definitions.rb b/activerecord/lib/active_record/connection_adapters/sqlite3/schema_definitions.rb index 501f17dbad..2010de1ce2 100644 --- a/activerecord/lib/active_record/connection_adapters/sqlite3/schema_definitions.rb +++ b/activerecord/lib/active_record/connection_adapters/sqlite3/schema_definitions.rb @@ -3,27 +3,19 @@ module ActiveRecord module ConnectionAdapters module SQLite3 - module ColumnMethods - def primary_key(name, type = :primary_key, **options) - if %i(integer bigint).include?(type) && (options.delete(:auto_increment) == true || !options.key?(:default)) - type = :primary_key - end - - super - end - end - class TableDefinition < ActiveRecord::ConnectionAdapters::TableDefinition - include ColumnMethods - def references(*args, **options) super(*args, type: :integer, **options) end alias :belongs_to :references - end - class Table < ActiveRecord::ConnectionAdapters::Table - include ColumnMethods + def new_column_definition(name, type, **options) # :nodoc: + if integer_like_primary_key?(type, options) + type = :primary_key + end + + super + end end end end diff --git a/activerecord/lib/active_record/connection_adapters/sqlite3/schema_statements.rb b/activerecord/lib/active_record/connection_adapters/sqlite3/schema_statements.rb index a512702b7b..f4e55147df 100644 --- a/activerecord/lib/active_record/connection_adapters/sqlite3/schema_statements.rb +++ b/activerecord/lib/active_record/connection_adapters/sqlite3/schema_statements.rb @@ -39,10 +39,6 @@ module ActiveRecord end end - def update_table_definition(table_name, base) - SQLite3::Table.new(table_name, base) - end - def create_schema_dumper(options) SQLite3::SchemaDumper.create(self, options) end diff --git a/activerecord/lib/active_record/migration/command_recorder.rb b/activerecord/lib/active_record/migration/command_recorder.rb index a3a5e0fa16..ac7d506fd1 100644 --- a/activerecord/lib/active_record/migration/command_recorder.rb +++ b/activerecord/lib/active_record/migration/command_recorder.rb @@ -161,8 +161,8 @@ module ActiveRecord table, columns, options = *args options ||= {} - index_name = options[:name] - options_hash = index_name ? { name: index_name } : { column: columns } + options_hash = options.slice(:name, :algorithm) + options_hash[:column] = columns if !options_hash[:name] [:remove_index, [table, options_hash]] end diff --git a/activerecord/lib/active_record/migration/compatibility.rb b/activerecord/lib/active_record/migration/compatibility.rb index 87c1c58aff..502cef2e20 100644 --- a/activerecord/lib/active_record/migration/compatibility.rb +++ b/activerecord/lib/active_record/migration/compatibility.rb @@ -20,6 +20,11 @@ module ActiveRecord class V5_0 < V5_1 module TableDefinition + def primary_key(name, type = :primary_key, **options) + type = :integer if type == :primary_key + super + end + def references(*args, **options) super(*args, type: :integer, **options) end @@ -86,6 +91,14 @@ module ActiveRecord end end + def add_column(table_name, column_name, type, options = {}) + if type == :primary_key + type = :integer + options[:primary_key] = true + end + super + end + def add_reference(table_name, ref_name, **options) super(table_name, ref_name, type: :integer, **options) end diff --git a/activerecord/lib/active_record/relation/calculations.rb b/activerecord/lib/active_record/relation/calculations.rb index 42d43224fa..0889d61c92 100644 --- a/activerecord/lib/active_record/relation/calculations.rb +++ b/activerecord/lib/active_record/relation/calculations.rb @@ -391,7 +391,7 @@ module ActiveRecord def build_count_subquery(relation, column_name, distinct) relation.select_values = [ if column_name == :all - distinct ? table[Arel.star] : Arel.sql("1") + distinct ? table[Arel.star] : Arel.sql(FinderMethods::ONE_AS_ONE) else column_alias = Arel.sql("count_column") aggregate_column(column_name).as(column_alias) diff --git a/activerecord/test/cases/comment_test.rb b/activerecord/test/cases/comment_test.rb index 1bcafd4b55..584e03d196 100644 --- a/activerecord/test/cases/comment_test.rb +++ b/activerecord/test/cases/comment_test.rb @@ -142,5 +142,27 @@ if ActiveRecord::Base.connection.supports_comments? assert_match %r[t\.string\s+"absent_comment"\n], output assert_no_match %r[t\.string\s+"absent_comment", comment:\n], output end + + def test_change_table_comment + @connection.change_table_comment :commenteds, "Edited table comment" + assert_equal "Edited table comment", @connection.table_comment("commenteds") + end + + def test_change_table_comment_to_nil + @connection.change_table_comment :commenteds, nil + assert_nil @connection.table_comment("commenteds") + end + + def test_change_column_comment + @connection.change_column_comment :commenteds, :name, "Edited column comment" + column = Commented.columns_hash["name"] + assert_equal "Edited column comment", column.comment + end + + def test_change_column_comment_to_nil + @connection.change_column_comment :commenteds, :name, nil + column = Commented.columns_hash["name"] + assert_nil column.comment + end end end diff --git a/activerecord/test/cases/instrumentation_test.rb b/activerecord/test/cases/instrumentation_test.rb index 86ffe8b0d7..e6e8468757 100644 --- a/activerecord/test/cases/instrumentation_test.rb +++ b/activerecord/test/cases/instrumentation_test.rb @@ -8,7 +8,7 @@ module ActiveRecord def test_payload_name_on_load Book.create(name: "test book") subscriber = ActiveSupport::Notifications.subscribe("sql.active_record") do |*args| - event = ActiveSupport::Notifications::Event.new *args + event = ActiveSupport::Notifications::Event.new(*args) if event.payload[:sql].match "SELECT" assert_equal "Book Load", event.payload[:name] end @@ -20,7 +20,7 @@ module ActiveRecord def test_payload_name_on_create subscriber = ActiveSupport::Notifications.subscribe("sql.active_record") do |*args| - event = ActiveSupport::Notifications::Event.new *args + event = ActiveSupport::Notifications::Event.new(*args) if event.payload[:sql].match "INSERT" assert_equal "Book Create", event.payload[:name] end @@ -32,7 +32,7 @@ module ActiveRecord def test_payload_name_on_update subscriber = ActiveSupport::Notifications.subscribe("sql.active_record") do |*args| - event = ActiveSupport::Notifications::Event.new *args + event = ActiveSupport::Notifications::Event.new(*args) if event.payload[:sql].match "UPDATE" assert_equal "Book Update", event.payload[:name] end @@ -45,7 +45,7 @@ module ActiveRecord def test_payload_name_on_update_all subscriber = ActiveSupport::Notifications.subscribe("sql.active_record") do |*args| - event = ActiveSupport::Notifications::Event.new *args + event = ActiveSupport::Notifications::Event.new(*args) if event.payload[:sql].match "UPDATE" assert_equal "Book Update All", event.payload[:name] end @@ -58,7 +58,7 @@ module ActiveRecord def test_payload_name_on_destroy subscriber = ActiveSupport::Notifications.subscribe("sql.active_record") do |*args| - event = ActiveSupport::Notifications::Event.new *args + event = ActiveSupport::Notifications::Event.new(*args) if event.payload[:sql].match "DELETE" assert_equal "Book Destroy", event.payload[:name] end diff --git a/activerecord/test/cases/migration/command_recorder_test.rb b/activerecord/test/cases/migration/command_recorder_test.rb index 0b5e983f14..58bc558619 100644 --- a/activerecord/test/cases/migration/command_recorder_test.rb +++ b/activerecord/test/cases/migration/command_recorder_test.rb @@ -213,6 +213,11 @@ module ActiveRecord assert_equal [:remove_index, [:table, { name: "new_index" }]], remove end + def test_invert_add_index_with_algorithm_option + remove = @recorder.inverse_of :add_index, [:table, :one, algorithm: :concurrently] + assert_equal [:remove_index, [:table, { column: :one, algorithm: :concurrently }]], remove + end + def test_invert_remove_index add = @recorder.inverse_of :remove_index, [:table, :one] assert_equal [:add_index, [:table, :one]], add diff --git a/activerecord/test/cases/migration/compatibility_test.rb b/activerecord/test/cases/migration/compatibility_test.rb index e1311c0f21..1ae15eb439 100644 --- a/activerecord/test/cases/migration/compatibility_test.rb +++ b/activerecord/test/cases/migration/compatibility_test.rb @@ -199,6 +199,57 @@ class LegacyPrimaryKeyTest < ActiveRecord::TestCase assert_match %r{create_table "legacy_primary_keys", id: :integer, default: nil}, schema end + if current_adapter?(:Mysql2Adapter, :PostgreSQLAdapter) + def test_legacy_primary_key_in_create_table_should_be_integer + @migration = Class.new(ActiveRecord::Migration[5.0]) { + def change + create_table :legacy_primary_keys, id: false do |t| + t.primary_key :id + end + end + }.new + + @migration.migrate(:up) + + schema = dump_table_schema "legacy_primary_keys" + assert_match %r{create_table "legacy_primary_keys", id: :(?:integer|serial), (?!default: nil)}, schema + end + + def test_legacy_primary_key_in_change_table_should_be_integer + @migration = Class.new(ActiveRecord::Migration[5.0]) { + def change + create_table :legacy_primary_keys, id: false do |t| + t.integer :dummy + end + change_table :legacy_primary_keys do |t| + t.primary_key :id + end + end + }.new + + @migration.migrate(:up) + + schema = dump_table_schema "legacy_primary_keys" + assert_match %r{create_table "legacy_primary_keys", id: :(?:integer|serial), (?!default: nil)}, schema + end + + def test_add_column_with_legacy_primary_key_should_be_integer + @migration = Class.new(ActiveRecord::Migration[5.0]) { + def change + create_table :legacy_primary_keys, id: false do |t| + t.integer :dummy + end + add_column :legacy_primary_keys, :id, :primary_key + end + }.new + + @migration.migrate(:up) + + schema = dump_table_schema "legacy_primary_keys" + assert_match %r{create_table "legacy_primary_keys", id: :(?:integer|serial), (?!default: nil)}, schema + end + end + def test_legacy_join_table_foreign_keys_should_be_integer @migration = Class.new(ActiveRecord::Migration[5.0]) { def change diff --git a/railties/lib/rails/commands/runner/runner_command.rb b/railties/lib/rails/commands/runner/runner_command.rb index cd9462e08f..30fbf04982 100644 --- a/railties/lib/rails/commands/runner/runner_command.rb +++ b/railties/lib/rails/commands/runner/runner_command.rb @@ -32,13 +32,13 @@ module Rails ARGV.replace(command_argv) if code_or_file == "-" - eval($stdin.read, binding, "stdin") + eval($stdin.read, TOPLEVEL_BINDING, "stdin") elsif File.exist?(code_or_file) $0 = code_or_file Kernel.load code_or_file else begin - eval(code_or_file, binding, __FILE__, __LINE__) + eval(code_or_file, TOPLEVEL_BINDING, __FILE__, __LINE__) rescue SyntaxError, NameError => error $stderr.puts "Please specify a valid ruby command or the path of a script to run." $stderr.puts "Run '#{self.class.executable} -h' for help." diff --git a/railties/test/application/runner_test.rb b/railties/test/application/runner_test.rb index 64c46c4b45..aa5d495c97 100644 --- a/railties/test/application/runner_test.rb +++ b/railties/test/application/runner_test.rb @@ -128,5 +128,17 @@ module ApplicationTests assert_match "production", rails("runner", "puts Rails.env") end end + + def test_can_call_same_name_class_as_defined_in_thor + app_file "app/models/task.rb", <<-MODEL + class Task + def self.count + 42 + end + end + MODEL + + assert_match "42", rails("runner", "puts Task.count") + end end end |