lib/strong_migrations/checker.rb in strong_migrations-0.6.8 vs lib/strong_migrations/checker.rb in strong_migrations-0.7.0

- old
+ new

@@ -94,32 +94,43 @@ raise_error :add_column_default, add_command: command_str("add_column", [table, column, type, options.except(:default)]), change_command: command_str("change_column_default", [table, column, default]), remove_command: command_str("remove_column", [table, column]), code: backfill_code(table, column, default), - append: append + append: append, + rewrite_blocks: rewrite_blocks end if type.to_s == "json" && postgresql? - raise_error :add_column_json + raise_error :add_column_json, + command: command_str("add_column", [table, column, :jsonb, options]) end when :change_column table, column, type, options = args options ||= {} safe = false existing_column = connection.columns(table).find { |c| c.name.to_s == column.to_s } if existing_column - sql_type = existing_column.sql_type.split("(").first + existing_type = existing_column.sql_type.split("(").first if postgresql? case type.to_s - when "string", "text" - # safe to change limit for varchar - safe = ["character varying", "text"].include?(sql_type) + when "string" + # safe to increase limit or remove it + # not safe to decrease limit or add a limit + case existing_type + when "character varying" + safe = !options[:limit] || (existing_column.limit && options[:limit] >= existing_column.limit) + when "text" + safe = !options[:limit] + end + when "text" + # safe to change varchar to text (and text to text) + safe = ["character varying", "text"].include?(existing_type) when "numeric", "decimal" # numeric and decimal are equivalent and can be used interchangably - safe = ["numeric", "decimal"].include?(sql_type) && + safe = ["numeric", "decimal"].include?(existing_type) && ( ( # unconstrained !options[:precision] && !options[:scale] ) || ( @@ -128,11 +139,11 @@ options[:precision] >= existing_column.precision && options[:scale] == existing_column.scale ) ) when "datetime", "timestamp", "timestamptz" - safe = ["timestamp without time zone", "timestamp with time zone"].include?(sql_type) && + safe = ["timestamp without time zone", "timestamp with time zone"].include?(existing_type) && postgresql_version >= Gem::Version.new("12") && connection.select_all("SHOW timezone").first["TimeZone"] == "UTC" end elsif mysql? || mariadb? case type.to_s @@ -140,17 +151,23 @@ # https://dev.mysql.com/doc/refman/5.7/en/innodb-online-ddl-operations.html # https://mariadb.com/kb/en/innodb-online-ddl-operations-with-the-instant-alter-algorithm/#changing-the-data-type-of-a-column # increased limit, but doesn't change number of length bytes # 1-255 = 1 byte, 256-65532 = 2 bytes, 65533+ = too big for varchar limit = options[:limit] || 255 - safe = ["varchar"].include?(sql_type) && + safe = ["varchar"].include?(existing_type) && limit >= existing_column.limit && (limit <= 255 || existing_column.limit > 255) end end end - raise_error :change_column unless safe + + # unsafe to set NOT NULL for safe types + if safe && existing_column.null && options[:null] == false + raise_error :change_column_with_not_null + end + + raise_error :change_column, rewrite_blocks: rewrite_blocks unless safe when :create_table table, options = args options ||= {} raise_error :create_table if options[:force] @@ -174,11 +191,11 @@ else options = options.merge(index: {algorithm: :concurrently}) end if options.delete(:foreign_key) - headline = "Adding a validated foreign key locks the table." + headline = "Adding a foreign key blocks writes on both tables." append = " Then add the foreign key in separate migrations." else headline = "Adding an index non-concurrently locks the table." @@ -245,10 +262,14 @@ raise_error :add_foreign_key, add_foreign_key_code: command_str("add_foreign_key", [from_table, to_table, options.merge(validate: false)]), validate_foreign_key_code: command_str("validate_foreign_key", [from_table, to_table]) end end + when :validate_foreign_key + if postgresql? && writes_blocked? + raise_error :validate_foreign_key + end end StrongMigrations.checks.each do |check| @migration.instance_exec(method, args, &check) end @@ -257,20 +278,21 @@ result = yield # outdated statistics + a new index can hurt performance of existing queries if StrongMigrations.auto_analyze && direction == :up && method == :add_index if postgresql? - # TODO remove verbose in 0.7.0 - connection.execute "ANALYZE VERBOSE #{connection.quote_table_name(args[0].to_s)}" + connection.execute "ANALYZE #{connection.quote_table_name(args[0].to_s)}" elsif mariadb? || mysql? connection.execute "ANALYZE TABLE #{connection.quote_table_name(args[0].to_s)}" end end result end + private + def set_timeouts if !@timeouts_set if StrongMigrations.statement_timeout statement = if postgresql? @@ -301,22 +323,21 @@ @timeouts_set = true end end - private - def connection @migration.connection end def version @migration.version end def safe? - @safe || ENV["SAFETY_ASSURED"] || @migration.is_a?(ActiveRecord::Schema) || direction == :down || version_safe? + @safe || ENV["SAFETY_ASSURED"] || @migration.is_a?(ActiveRecord::Schema) || + (direction == :down && !StrongMigrations.check_down) || version_safe? end def version_safe? version && version <= StrongMigrations.start_after end @@ -417,15 +438,20 @@ timeout.to_i * 1000 end end def constraints(table_name) - query = <<-SQL - SELECT conname AS name, pg_get_constraintdef(oid) AS def FROM pg_constraint - WHERE contype = 'c' - AND convalidated - AND conrelid = #{connection.quote(connection.quote_table_name(table_name))}::regclass + query = <<~SQL + SELECT + conname AS name, + pg_get_constraintdef(oid) AS def + FROM + pg_constraint + WHERE + contype = 'c' AND + convalidated AND + conrelid = #{connection.quote(connection.quote_table_name(table_name))}::regclass SQL connection.select_all(query.squish).to_a end def raise_error(message_key, header: nil, append: nil, **vars) @@ -468,9 +494,26 @@ else str_args << last_arg.inspect end "#{command} #{str_args.join(", ")}" + end + + def writes_blocked? + query = <<~SQL + SELECT + relation::regclass::text + FROM + pg_locks + WHERE + mode IN ('ShareRowExclusiveLock', 'AccessExclusiveLock') AND + pid = pg_backend_pid() + SQL + connection.select_all(query.squish).any? + end + + def rewrite_blocks + mysql? || mariadb? ? "writes" : "reads and writes" end def backfill_code(table, column, default) model = table.to_s.classify "#{model}.unscoped.in_batches do |relation| \n relation.update_all #{column}: #{default.inspect}\n sleep(0.01)\n end"