diff --git a/lib/safe-pg-migrations/base.rb b/lib/safe-pg-migrations/base.rb index 73210ef..93345ed 100644 --- a/lib/safe-pg-migrations/base.rb +++ b/lib/safe-pg-migrations/base.rb @@ -5,6 +5,7 @@ require 'safe-pg-migrations/helpers/satisfied_helper' require 'safe-pg-migrations/helpers/index_helper' require 'safe-pg-migrations/helpers/batch_over' +require 'safe-pg-migrations/helpers/session_setting_management' require 'safe-pg-migrations/plugins/verbose_sql_logger' require 'safe-pg-migrations/plugins/blocking_activity_logger' require 'safe-pg-migrations/plugins/statement_insurer/add_column' @@ -40,7 +41,9 @@ def setup_and_teardown(migration, connection, &block) VerboseSqlLogger.new.setup if verbose? PLUGINS.each { |plugin| connection.extend(plugin) } - connection.with_setting :lock_timeout, SafePgMigrations.config.pg_lock_timeout, &block + connection.with_setting :lock_timeout, SafePgMigrations.config.pg_lock_timeout do + connection.with_setting :statement_timeout, SafePgMigrations.config.pg_statement_timeout, &block + end ensure stdout_sql_logger&.teardown end diff --git a/lib/safe-pg-migrations/helpers/session_setting_management.rb b/lib/safe-pg-migrations/helpers/session_setting_management.rb new file mode 100644 index 0000000..5c00ed6 --- /dev/null +++ b/lib/safe-pg-migrations/helpers/session_setting_management.rb @@ -0,0 +1,35 @@ +# frozen_string_literal: true + +module SafePgMigrations + module Helpers + module SessionSettingManagement + def with_setting(key, value) + old_value = query_value("SHOW #{key}") + execute("SET #{key} TO #{quote(value)}") + begin + yield + ensure + begin + execute("SET #{key} TO #{quote(old_value)}") + rescue ActiveRecord::StatementInvalid => e + # Swallow `PG::InFailedSqlTransaction` exceptions so as to keep the + # original exception (if any). + raise unless e.cause.is_a?(PG::InFailedSqlTransaction) + end + end + end + + def without_statement_timeout(&block) + with_setting(:statement_timeout, 0, &block) + end + + def without_lock_timeout(&block) + with_setting(:lock_timeout, 0, &block) + end + + def without_timeout(&block) + without_statement_timeout { without_lock_timeout(&block) } + end + end + end +end diff --git a/lib/safe-pg-migrations/plugins/statement_insurer.rb b/lib/safe-pg-migrations/plugins/statement_insurer.rb index 9c3621e..9761eaf 100644 --- a/lib/safe-pg-migrations/plugins/statement_insurer.rb +++ b/lib/safe-pg-migrations/plugins/statement_insurer.rb @@ -1,20 +1,13 @@ # frozen_string_literal: true module SafePgMigrations - module StatementInsurer # rubocop:disable Metrics/ModuleLength + module StatementInsurer + include Helpers::SessionSettingManagement include AddColumn - %i[change_column].each do |method| - define_method method do |*args, &block| - with_setting(:statement_timeout, SafePgMigrations.config.pg_statement_timeout) { super(*args, &block) } - end - ruby2_keywords method - end - def validate_check_constraint(table_name, **options) - without_statement_timeout do - super - end + Helpers::Logger.say_method_call :validate_check_constraint, table_name, **options + without_statement_timeout { super } end def add_check_constraint(table_name, expression, **options) @@ -24,37 +17,37 @@ def add_check_constraint(table_name, expression, **options) options = check_constraint_options(table_name, expression, options) Helpers::Logger.say_method_call :add_check_constraint, table_name, expression, **options, -validate: false + validate: false super table_name, expression, **options, validate: false return unless options.fetch(:validate, true) - Helpers::Logger.say_method_call :validate_check_constraint, table_name, name: options[:name] validate_check_constraint table_name, name: options[:name] end + def validate_foreign_key(*, **) + without_statement_timeout { super } + end + ruby2_keywords def add_foreign_key(from_table, to_table, *args) options = args.last.is_a?(Hash) ? args.last : {} validate_present = options.key?(:validate) options[:validate] = false unless validate_present - with_setting(:statement_timeout, SafePgMigrations.config.pg_statement_timeout) do - super(from_table, to_table, **options) - end + + super(from_table, to_table, **options) return if validate_present - suboptions = options.slice(:name, :column) - without_statement_timeout { validate_foreign_key from_table, suboptions.present? ? nil : to_table, **suboptions } + sub_options = options.slice(:name, :column) + validate_foreign_key from_table, sub_options.present? ? nil : to_table, **sub_options end ruby2_keywords def create_table(*) - with_setting(:statement_timeout, SafePgMigrations.config.pg_statement_timeout) do - super do |td| - yield td if block_given? - td.indexes.map! do |key, index_options| - index_options[:algorithm] ||= :default - [key, index_options] - end + super do |td| + yield td if block_given? + td.indexes.map! do |key, index_options| + index_options[:algorithm] ||= :default + [key, index_options] end end end @@ -69,84 +62,44 @@ def add_check_constraint(table_name, expression, **options) end Helpers::Logger.say_method_call(:add_index, table_name, column_name, **options) - without_timeout { super(table_name, column_name, **options) } end ruby2_keywords def remove_index(table_name, *args) options = args.last.is_a?(Hash) ? args.last : { column: args.last } options[:algorithm] = :concurrently unless options.key?(:algorithm) - Helpers::Logger.say_method_call(:remove_index, table_name, **options) + Helpers::Logger.say_method_call(:remove_index, table_name, **options) without_timeout { super(table_name, **options) } end def change_column_null(table_name, column_name, null, default = nil) - if default || null || !Helpers::SatisfiedHelper.satisfies_change_column_null_requirements? - with_setting(:statement_timeout, SafePgMigrations.config.pg_statement_timeout) { return super } - end + return super if default || null || !Helpers::SatisfiedHelper.satisfies_change_column_null_requirements? add_check_constraint table_name, "#{column_name} IS NOT NULL" Helpers::Logger.say_method_call :change_column_null, table_name, column_name, false - with_setting(:statement_timeout, SafePgMigrations.config.pg_statement_timeout) do - super table_name, column_name, false - end + super table_name, column_name, false - Helpers::Logger.say_method_call :remove_check_constraint, table_name, - "#{column_name} IS NOT NULL" + Helpers::Logger.say_method_call :remove_check_constraint, table_name, "#{column_name} IS NOT NULL" remove_check_constraint table_name, "#{column_name} IS NOT NULL" end def remove_column(table_name, column_name, *) foreign_key = foreign_key_for(table_name, column: column_name) - with_setting(:statement_timeout, SafePgMigrations.config.pg_statement_timeout) do - remove_foreign_key(table_name, name: foreign_key.name) if foreign_key - super - end + remove_foreign_key(table_name, name: foreign_key.name) if foreign_key + super end ruby2_keywords def drop_table(table_name, *args) foreign_keys(table_name).each do |foreign_key| - with_setting(:statement_timeout, SafePgMigrations.config.pg_statement_timeout) do - remove_foreign_key(table_name, name: foreign_key.name) - end + remove_foreign_key(table_name, name: foreign_key.name) end Helpers::Logger.say_method_call :drop_table, table_name, *args - with_setting(:statement_timeout, SafePgMigrations.config.pg_statement_timeout) do - super(table_name, *args) - end - end - - def with_setting(key, value) - old_value = query_value("SHOW #{key}") - execute("SET #{key} TO #{quote(value)}") - begin - yield - ensure - begin - execute("SET #{key} TO #{quote(old_value)}") - rescue ActiveRecord::StatementInvalid => e - # Swallow `PG::InFailedSqlTransaction` exceptions so as to keep the - # original exception (if any). - raise unless e.cause.is_a?(PG::InFailedSqlTransaction) - end - end - end - - def without_statement_timeout(&block) - with_setting(:statement_timeout, 0, &block) - end - - def without_lock_timeout(&block) - with_setting(:lock_timeout, 0, &block) - end - - def without_timeout(&block) - without_statement_timeout { without_lock_timeout(&block) } + super(table_name, *args) end end end diff --git a/lib/safe-pg-migrations/plugins/statement_insurer/add_column.rb b/lib/safe-pg-migrations/plugins/statement_insurer/add_column.rb index f7717d5..00ad593 100644 --- a/lib/safe-pg-migrations/plugins/statement_insurer/add_column.rb +++ b/lib/safe-pg-migrations/plugins/statement_insurer/add_column.rb @@ -7,9 +7,7 @@ module AddColumn options = args.last.is_a?(Hash) && args.last options ||= {} - if should_keep_default_implementation?(**options) - with_setting(:statement_timeout, SafePgMigrations.config.pg_statement_timeout) { return super } - end + return super if should_keep_default_implementation?(**options) raise <<~ERROR unless backfill_column_default_safe?(table_name) Table #{table_name} has more than #{SafePgMigrations.config.default_value_backfill_threshold} rows. @@ -23,18 +21,13 @@ module AddColumn default = options.delete(:default) null = options.delete(:null) - with_setting(:statement_timeout, SafePgMigrations.config.pg_statement_timeout) do - Helpers::Logger.say_method_call(:add_column, table_name, column_name, type, options) - super table_name, column_name, type, **options - end + Helpers::Logger.say_method_call(:add_column, table_name, column_name, type, options) + super table_name, column_name, type, **options Helpers::Logger.say_method_call(:change_column_default, table_name, column_name, default) change_column_default(table_name, column_name, default) - Helpers::Logger.say_method_call(:backfill_column_default, table_name, column_name) - without_statement_timeout do - backfill_column_default(table_name, column_name) - end + backfill_column_default(table_name, column_name) change_column_null(table_name, column_name, null) if null == false end @@ -59,9 +52,11 @@ def backfill_column_default(table_name, column_name) model = Class.new(ActiveRecord::Base) { self.table_name = table_name } quoted_column_name = quote_column_name(column_name) + Helpers::Logger.say_method_call(:backfill_column_default, table_name, column_name) + Helpers::BatchOver.new(model).each_batch do |batch| - batch - .update_all("#{quoted_column_name} = DEFAULT") + batch.update_all("#{quoted_column_name} = DEFAULT") + sleep SafePgMigrations.config.backfill_pause end end diff --git a/test/IdempotentStatements/drop_table_test.rb b/test/IdempotentStatements/drop_table_test.rb index 832ee7e..3ace1d0 100644 --- a/test/IdempotentStatements/drop_table_test.rb +++ b/test/IdempotentStatements/drop_table_test.rb @@ -29,12 +29,8 @@ def test_first_fk_already_removed calls = record_calls(@connection, :execute) { run_migration } assert_calls <<~CALLS.strip.split("\n"), calls - SET statement_timeout TO '5s' ALTER TABLE "users" DROP CONSTRAINT "fk_rails_d15efa01b1" - SET statement_timeout TO '70s' - SET statement_timeout TO '5s' DROP TABLE "users" - SET statement_timeout TO '70s' CALLS end @@ -44,11 +40,7 @@ def test_fks_already_removed calls = record_calls(@connection, :execute) { run_migration } - assert_calls <<~CALLS.strip.split("\n"), calls - SET statement_timeout TO '5s' - DROP TABLE "users" - SET statement_timeout TO '70s' - CALLS + assert_calls ['DROP TABLE "users"'], calls end end end diff --git a/test/IdempotentStatements/idempotent_statements_test.rb b/test/IdempotentStatements/idempotent_statements_test.rb index a69ae40..2b0d667 100644 --- a/test/IdempotentStatements/idempotent_statements_test.rb +++ b/test/IdempotentStatements/idempotent_statements_test.rb @@ -116,11 +116,11 @@ def change 'SET lock_timeout TO 0', 'CREATE INDEX CONCURRENTLY "my_custom_index_name" ON "users" ("email") WHERE email IS NOT NULL', "SET lock_timeout TO '4950ms'", - "SET statement_timeout TO '70s'", + "SET statement_timeout TO '5s'", 'SET statement_timeout TO 0', 'SET lock_timeout TO 0', "SET lock_timeout TO '4950ms'", - "SET statement_timeout TO '70s'", + "SET statement_timeout TO '5s'", ], calls end @@ -148,7 +148,7 @@ def change 'CREATE INDEX CONCURRENTLY "index_users_on_email" ON "users" ("email")', "SET lock_timeout TO '4950ms'", - "SET statement_timeout TO '70s'", + "SET statement_timeout TO '5s'", ], calls end @@ -218,18 +218,14 @@ def change execute_calls = record_calls(@connection, :execute) { run_migration } end assert_calls [ - "SET statement_timeout TO '5s'", 'ALTER TABLE "messages" ADD CONSTRAINT "fk_rails_273a25a7a6" FOREIGN KEY ("user_id") ' \ 'REFERENCES "users" ("id") NOT VALID', - "SET statement_timeout TO '70s'", 'SET statement_timeout TO 0', 'ALTER TABLE "messages" VALIDATE CONSTRAINT "fk_rails_273a25a7a6"', - "SET statement_timeout TO '70s'", "SET statement_timeout TO '5s'", - "SET statement_timeout TO '70s'", 'SET statement_timeout TO 0', 'ALTER TABLE "messages" VALIDATE CONSTRAINT "fk_rails_273a25a7a6"', - "SET statement_timeout TO '70s'", + "SET statement_timeout TO '5s'", ], execute_calls assert_equal [ @@ -261,18 +257,14 @@ def change end assert_calls [ - "SET statement_timeout TO '5s'", 'ALTER TABLE "messages" ADD CONSTRAINT "fk_rails_995937c106" FOREIGN KEY ("author_id") ' \ 'REFERENCES "users" ("id") NOT VALID', - "SET statement_timeout TO '70s'", 'SET statement_timeout TO 0', 'ALTER TABLE "messages" VALIDATE CONSTRAINT "fk_rails_995937c106"', - "SET statement_timeout TO '70s'", "SET statement_timeout TO '5s'", - "SET statement_timeout TO '70s'", 'SET statement_timeout TO 0', 'ALTER TABLE "messages" VALIDATE CONSTRAINT "fk_rails_995937c106"', - "SET statement_timeout TO '70s'", + "SET statement_timeout TO '5s'", ], execute_calls assert_equal [ @@ -305,18 +297,14 @@ def change end assert_calls [ - "SET statement_timeout TO '5s'", 'ALTER TABLE "messages" ADD CONSTRAINT "fk_rails_273a25a7a6" FOREIGN KEY ("user_id") ' \ 'REFERENCES "users" ("id") NOT VALID', - "SET statement_timeout TO '70s'", 'SET statement_timeout TO 0', 'ALTER TABLE "messages" VALIDATE CONSTRAINT "fk_rails_273a25a7a6"', - "SET statement_timeout TO '70s'", "SET statement_timeout TO '5s'", - "SET statement_timeout TO '70s'", 'SET statement_timeout TO 0', 'ALTER TABLE "messages" VALIDATE CONSTRAINT "fk_rails_273a25a7a6"', - "SET statement_timeout TO '70s'", + "SET statement_timeout TO '5s'", ], execute_calls assert_equal [ @@ -351,20 +339,16 @@ def change end assert_calls [ - "SET statement_timeout TO '5s'", 'ALTER TABLE "messages" ADD CONSTRAINT "fk_rails_995937c106" FOREIGN KEY ("author_id") ' \ 'REFERENCES "users" ("id") NOT VALID', - "SET statement_timeout TO '70s'", 'SET statement_timeout TO 0', 'ALTER TABLE "messages" VALIDATE CONSTRAINT "fk_rails_995937c106"', - "SET statement_timeout TO '70s'", "SET statement_timeout TO '5s'", 'ALTER TABLE "messages" ADD CONSTRAINT "fk_rails_7f927086d2" FOREIGN KEY ("conversation_id") ' \ 'REFERENCES "conversations" ("id") NOT VALID', - "SET statement_timeout TO '70s'", 'SET statement_timeout TO 0', 'ALTER TABLE "messages" VALIDATE CONSTRAINT "fk_rails_7f927086d2"', - "SET statement_timeout TO '70s'", + "SET statement_timeout TO '5s'", ], execute_calls assert_equal [ @@ -399,7 +383,6 @@ def change refute_includes flat_calls(calls), 'CREATE INDEX CONCURRENTLY "index_users_on_name" ON "users" ("name")' assert_calls [ - "SET statement_timeout TO '5s'", 'SET statement_timeout TO 0', 'SET lock_timeout TO 0', "SET lock_timeout TO '4950ms'", @@ -409,7 +392,6 @@ def change 'CREATE INDEX "index_users_on_email" ON "users" ("email")', "SET lock_timeout TO '4950ms'", "SET statement_timeout TO '5s'", - "SET statement_timeout TO '70s'", ], calls end @@ -471,7 +453,7 @@ def change execute_calls = record_calls(@connection, :execute) { run_migration } end - assert_match(/ALTER TABLE "messages" DROP CONSTRAINT "fk_rails_\w*"/, flat_calls(execute_calls)[1]) + assert_match(/ALTER TABLE "messages" DROP CONSTRAINT "fk_rails_\w*"/, flat_calls(execute_calls)[2]) assert_equal [ '== 8128 : migrating ===========================================================', @@ -502,7 +484,7 @@ def change execute_calls = record_calls(@connection, :execute) { run_migration } end - assert_match(/ALTER TABLE "messages" DROP CONSTRAINT "fk_rails_\w*"/, flat_calls(execute_calls)[1]) + assert_match(/ALTER TABLE "messages" DROP CONSTRAINT "fk_rails_\w*"/, flat_calls(execute_calls)[2]) assert_equal [ '== 8128 : migrating ===========================================================', diff --git a/test/StatementInsurer/add_check_constraint_test.rb b/test/StatementInsurer/add_check_constraint_test.rb index 103c319..821b88d 100644 --- a/test/StatementInsurer/add_check_constraint_test.rb +++ b/test/StatementInsurer/add_check_constraint_test.rb @@ -44,7 +44,7 @@ def change 'ALTER TABLE "users" ADD CONSTRAINT chk_rails_8d5dc0bde6 CHECK (email IS NOT NULL) NOT VALID', 'SET statement_timeout TO 0', 'ALTER TABLE "users" VALIDATE CONSTRAINT "chk_rails_8d5dc0bde6"', - "SET statement_timeout TO '70s'", + "SET statement_timeout TO '5s'", ], calls end @@ -62,7 +62,8 @@ def change 'ALTER TABLE "users" ADD CONSTRAINT chk_rails_8d5dc0bde6 CHECK (email IS NOT NULL) NOT VALID', 'SET statement_timeout TO 0', 'ALTER TABLE "users" VALIDATE CONSTRAINT "chk_rails_8d5dc0bde6"', - "SET statement_timeout TO '70s'", + "SET statement_timeout TO '5s'", + ], calls end diff --git a/test/StatementInsurer/add_column_test.rb b/test/StatementInsurer/add_column_test.rb index 4288019..2c802d9 100644 --- a/test/StatementInsurer/add_column_test.rb +++ b/test/StatementInsurer/add_column_test.rb @@ -37,9 +37,7 @@ def up end assert_calls [ # The column is added with the default and not null constraint without any tricks - "SET statement_timeout TO '5s'", 'ALTER TABLE "users" ADD "admin" boolean DEFAULT FALSE NOT NULL', - "SET statement_timeout TO '70s'", ], execute_calls assert_equal [ @@ -59,9 +57,7 @@ def change calls = record_calls(@connection, :execute) { run_migration } assert_calls [ - "SET statement_timeout TO '5s'", 'ALTER TABLE "users" ADD "email" character varying', - "SET statement_timeout TO '70s'", ], calls end @@ -76,9 +72,7 @@ def change calls = record_calls(@connection, :execute) { run_migration } assert_calls [ - "SET statement_timeout TO '5s'", "ALTER TABLE \"users\" ADD \"email\" character varying DEFAULT '' NOT NULL", - "SET statement_timeout TO '70s'", ], calls end @@ -94,9 +88,7 @@ def change calls = record_calls(@connection, :execute) { run_migration } assert_calls [ - "SET statement_timeout TO '5s'", "ALTER TABLE \"users\" ADD \"email\" character varying DEFAULT '' NOT NULL", - "SET statement_timeout TO '70s'", ], calls end @@ -116,9 +108,7 @@ def change end assert_calls [ - "SET statement_timeout TO '5s'", "ALTER TABLE \"users\" ADD \"email\" character varying DEFAULT '' NOT NULL", - "SET statement_timeout TO '70s'", ], calls end @@ -137,20 +127,14 @@ def change calls = record_calls(@connection, :execute) { run_migration } assert_calls [ - "SET statement_timeout TO '5s'", 'ALTER TABLE "users" ADD "email" character varying', - "SET statement_timeout TO '70s'", "ALTER TABLE \"users\" ALTER COLUMN \"email\" SET DEFAULT 'roger@doctolib.com'", - 'SET statement_timeout TO 0', # exec_calls goes here - "SET statement_timeout TO '70s'", 'ALTER TABLE "users" ADD CONSTRAINT chk_rails_8d5dc0bde6 CHECK (email IS NOT NULL) NOT VALID', 'SET statement_timeout TO 0', 'ALTER TABLE "users" VALIDATE CONSTRAINT "chk_rails_8d5dc0bde6"', - "SET statement_timeout TO '70s'", "SET statement_timeout TO '5s'", 'ALTER TABLE "users" ALTER COLUMN "email" SET NOT NULL', - "SET statement_timeout TO '70s'", 'ALTER TABLE "users" DROP CONSTRAINT "chk_rails_8d5dc0bde6"', ], calls end @@ -169,13 +153,9 @@ def change calls = record_calls(@connection, :execute) { run_migration } assert_calls [ - "SET statement_timeout TO '5s'", 'ALTER TABLE "users" ADD "email" character varying', - "SET statement_timeout TO '70s'", "ALTER TABLE \"users\" ALTER COLUMN \"email\" SET DEFAULT 'roger@doctolib.com'", - 'SET statement_timeout TO 0', # exec_calls goes here - "SET statement_timeout TO '70s'", ], calls end diff --git a/test/StatementInsurer/change_column_null_test.rb b/test/StatementInsurer/change_column_null_test.rb index 0836a5f..f1cc303 100644 --- a/test/StatementInsurer/change_column_null_test.rb +++ b/test/StatementInsurer/change_column_null_test.rb @@ -61,20 +61,16 @@ def safe_pg_calls 'ALTER TABLE "users" ADD CONSTRAINT chk_rails_8d5dc0bde6 CHECK (email IS NOT NULL) NOT VALID', 'SET statement_timeout TO 0', 'ALTER TABLE "users" VALIDATE CONSTRAINT "chk_rails_8d5dc0bde6"', - "SET statement_timeout TO '70s'", "SET statement_timeout TO '5s'", 'ALTER TABLE "users" ALTER COLUMN "email" SET NOT NULL', - "SET statement_timeout TO '70s'", 'ALTER TABLE "users" DROP CONSTRAINT "chk_rails_8d5dc0bde6"', ] end def base_calls(with_default: false, action: 'SET') [ - "SET statement_timeout TO '5s'", with_default ? "UPDATE \"users\" SET \"email\"='roger@doctolib.com' WHERE \"email\" IS NULL" : nil, "ALTER TABLE \"users\" ALTER COLUMN \"email\" #{action} NOT NULL", - "SET statement_timeout TO '70s'", ].compact end diff --git a/test/StatementInsurer/drop_table_test.rb b/test/StatementInsurer/drop_table_test.rb index 7ca46c8..868a60f 100644 --- a/test/StatementInsurer/drop_table_test.rb +++ b/test/StatementInsurer/drop_table_test.rb @@ -16,8 +16,9 @@ def change calls = record_calls(@connection, :execute) { run_migration } - assert_calls ["SET statement_timeout TO '5s'", 'DROP TABLE "users"', "SET statement_timeout TO '70s'"], - calls + assert_calls <<~CALLS.strip.split("\n"), calls + DROP TABLE "users" + CALLS end def test_can_drop_table_with_foreign_key @@ -36,14 +37,10 @@ def change calls = record_calls(@connection, :execute) { run_migration } - assert_calls [ - "SET statement_timeout TO '5s'", - 'ALTER TABLE "users" DROP CONSTRAINT "fk_rails_253ea793f9"', - "SET statement_timeout TO '70s'", - "SET statement_timeout TO '5s'", - 'DROP TABLE "users"', - "SET statement_timeout TO '70s'", - ], calls + assert_calls <<~CALLS.strip.split("\n"), calls + ALTER TABLE "users" DROP CONSTRAINT "fk_rails_253ea793f9" + DROP TABLE "users" + CALLS end def test_can_drop_table_with_several_foreign_keys @@ -64,17 +61,11 @@ def change calls = record_calls(@connection, :execute) { run_migration } - assert_calls [ - "SET statement_timeout TO '5s'", - 'ALTER TABLE "users" DROP CONSTRAINT "fk_rails_253ea793f9"', - "SET statement_timeout TO '70s'", - "SET statement_timeout TO '5s'", - 'ALTER TABLE "users" DROP CONSTRAINT "fk_rails_d15efa01b1"', - "SET statement_timeout TO '70s'", - "SET statement_timeout TO '5s'", - 'DROP TABLE "users"', - "SET statement_timeout TO '70s'", - ], calls + assert_calls <<~CALLS.strip.split("\n"), calls + ALTER TABLE "users" DROP CONSTRAINT "fk_rails_253ea793f9" + ALTER TABLE "users" DROP CONSTRAINT "fk_rails_d15efa01b1" + DROP TABLE "users" + CALLS end end end diff --git a/test/StatementInsurer/remove_column_test.rb b/test/StatementInsurer/remove_column_test.rb index 7e2a6fb..1c55abb 100644 --- a/test/StatementInsurer/remove_column_test.rb +++ b/test/StatementInsurer/remove_column_test.rb @@ -18,8 +18,10 @@ def change calls = record_calls(@connection, :execute) { run_migration } - assert_equal ['ALTER TABLE "users" DROP CONSTRAINT "fk_rails_baad13daec"'], calls[2] - assert_equal ['ALTER TABLE "users" DROP COLUMN "password_id"'], calls[3] + assert_calls <<~CALLS.strip.split("\n"), calls + ALTER TABLE "users" DROP CONSTRAINT "fk_rails_baad13daec" + ALTER TABLE "users" DROP COLUMN "password_id" + CALLS end def test_can_remove_column_with_foreign_key_on_other_column diff --git a/test/StatementInsurer/statement_insurer_test.rb b/test/StatementInsurer/statement_insurer_test.rb index 073f095..faa972b 100644 --- a/test/StatementInsurer/statement_insurer_test.rb +++ b/test/StatementInsurer/statement_insurer_test.rb @@ -16,9 +16,7 @@ def change calls = record_calls(@connection, :execute) { run_migration } assert_calls [ - "SET statement_timeout TO '5s'", 'ALTER TABLE "users" ALTER COLUMN "email" TYPE text', - "SET statement_timeout TO '70s'", ], calls end @@ -34,25 +32,21 @@ def change calls = record_calls(@connection, :execute) { run_migration } assert_calls [ # The column is added. - "SET statement_timeout TO '5s'", 'ALTER TABLE "users" ADD "user_id" bigint', - "SET statement_timeout TO '70s'", # The index is created concurrently. 'SET statement_timeout TO 0', 'SET lock_timeout TO 0', 'CREATE INDEX CONCURRENTLY "index_users_on_user_id" ON "users" ("user_id")', "SET lock_timeout TO '4950ms'", - "SET statement_timeout TO '70s'", + "SET statement_timeout TO '5s'", # The foreign key is added. - "SET statement_timeout TO '5s'", 'ALTER TABLE "users" ADD CONSTRAINT "fk_rails_6d0b8b3c2f" FOREIGN KEY ("user_id") ' \ 'REFERENCES "users" ("id") NOT VALID', - "SET statement_timeout TO '70s'", 'SET statement_timeout TO 0', 'ALTER TABLE "users" VALIDATE CONSTRAINT "fk_rails_6d0b8b3c2f"', - "SET statement_timeout TO '70s'", + "SET statement_timeout TO '5s'", ], calls end @@ -72,10 +66,8 @@ def change calls = record_calls(@connection, :execute) { run_migration } assert_calls [ - "SET statement_timeout TO '5s'", 'ALTER TABLE "messages" ADD CONSTRAINT "fk_rails_273a25a7a6" FOREIGN KEY ("user_id") ' \ 'REFERENCES "users" ("id") NOT VALID', - "SET statement_timeout TO '70s'", ], calls end @@ -95,13 +87,11 @@ def change calls = record_calls(@connection, :execute) { run_migration } assert_calls [ - "SET statement_timeout TO '5s'", 'ALTER TABLE "messages" ADD CONSTRAINT "fk_rails_273a25a7a6" FOREIGN KEY ("user_id") ' \ 'REFERENCES "users" ("id") NOT VALID', - "SET statement_timeout TO '70s'", 'SET statement_timeout TO 0', 'ALTER TABLE "messages" VALIDATE CONSTRAINT "fk_rails_273a25a7a6"', - "SET statement_timeout TO '70s'", + "SET statement_timeout TO '5s'", ], calls end @@ -125,13 +115,11 @@ def change calls = record_calls(@connection, :execute) { run_migration } assert_calls [ - "SET statement_timeout TO '5s'", 'ALTER TABLE "messages" ADD CONSTRAINT "message_user_key" FOREIGN KEY ("author_id") ' \ 'REFERENCES "users" ("real_id") NOT VALID', - "SET statement_timeout TO '70s'", 'SET statement_timeout TO 0', 'ALTER TABLE "messages" VALIDATE CONSTRAINT "message_user_key"', - "SET statement_timeout TO '70s'", + "SET statement_timeout TO '5s'", ], calls end @@ -151,9 +139,7 @@ def change calls = record_calls(@connection, :execute) { run_migration } assert_calls [ - "SET statement_timeout TO '5s'", 'ALTER TABLE "messages" ADD CONSTRAINT "fk_rails_273a25a7a6" FOREIGN KEY ("user_id") REFERENCES "users" ("id")', - "SET statement_timeout TO '70s'", ], calls end @@ -170,7 +156,6 @@ def change calls = record_calls(@connection, :execute) { run_migration } assert_calls [ - "SET statement_timeout TO '5s'", # Create the table with constraints. 'CREATE TABLE "users" ("id" bigserial primary key, "email" character varying, "user_id" bigint, ' \ @@ -183,7 +168,6 @@ def change "SET lock_timeout TO '4950ms'", "SET statement_timeout TO '5s'", - "SET statement_timeout TO '70s'", ], calls run_migration(:down) diff --git a/test/legacy_active_record_support_test.rb b/test/legacy_active_record_support_test.rb index 27691ac..7c566c1 100644 --- a/test/legacy_active_record_support_test.rb +++ b/test/legacy_active_record_support_test.rb @@ -19,9 +19,7 @@ def change calls = record_calls(@connection, :execute) { run_migration } assert_calls [ - "SET statement_timeout TO '5s'", 'ALTER TABLE "messages" ADD CONSTRAINT "fk_rails_273a25a7a6" FOREIGN KEY ("user_id") REFERENCES "users" ("id")', - "SET statement_timeout TO '70s'", ], calls end end diff --git a/test/safe_pg_migrations_test.rb b/test/safe_pg_migrations_test.rb index c885377..ca6de5e 100644 --- a/test/safe_pg_migrations_test.rb +++ b/test/safe_pg_migrations_test.rb @@ -84,19 +84,15 @@ def change calls = record_calls(@connection, :execute) { run_migration } assert_calls [ # Both columns are added. - "SET statement_timeout TO '5s'", 'ALTER TABLE "users" ADD "email" character varying', - "SET statement_timeout TO '70s'", - "SET statement_timeout TO '5s'", 'ALTER TABLE "users" ADD "user_id" bigint', - "SET statement_timeout TO '70s'", # An index is created because of the column reference. 'SET statement_timeout TO 0', 'SET lock_timeout TO 0', 'CREATE INDEX CONCURRENTLY "index_users_on_user_id" ON "users" ("user_id")', "SET lock_timeout TO '4950ms'", - "SET statement_timeout TO '70s'", + "SET statement_timeout TO '5s'", ], calls run_migration(:down) @@ -119,7 +115,7 @@ def change 'SET lock_timeout TO 0', 'CREATE INDEX CONCURRENTLY "index_users_on_email" ON "users" ("email")', "SET lock_timeout TO '4950ms'", - "SET statement_timeout TO '70s'", + "SET statement_timeout TO '5s'", ], calls run_migration(:down) diff --git a/test/test_helper.rb b/test/test_helper.rb index 204853b..fe10594 100644 --- a/test/test_helper.rb +++ b/test/test_helper.rb @@ -64,7 +64,9 @@ def run_migration(direction = :up) def assert_calls(expected, actual) assert_equal [ "SET lock_timeout TO '4950ms'", + "SET statement_timeout TO '5s'", *expected, + "SET statement_timeout TO '70s'", "SET lock_timeout TO '70s'", ], flat_calls(actual) end diff --git a/test/verbose_sql_logger_test.rb b/test/verbose_sql_logger_test.rb index 288a300..1a94c09 100644 --- a/test/verbose_sql_logger_test.rb +++ b/test/verbose_sql_logger_test.rb @@ -97,8 +97,9 @@ def assert_logs_match(stdout) assert_match('SHOW lock_timeout', logs[0]) assert_match("SET lock_timeout TO '4950ms'", logs[1]) - assert_match('SELECT * from pg_stat_activity', logs[-3]) - assert_match('SELECT version()', logs[-2]) + assert_match('SELECT * from pg_stat_activity', logs[-4]) + assert_match('SELECT version()', logs[-3]) + assert_match("SET statement_timeout TO '70s'", logs[-2]) assert_match("SET lock_timeout TO '70s'", logs[-1]) end end