Fix migration compatibility for default precision value on datetime columns

[robertomiranda]

Background

I have made a mistake on #42297, the definitions for add_column and column were put under the wrong class

rails/activerecord/lib/active_record/migration/compatibility.rb

Lines 286 to 335 in 246bac4

	class V4_2 < V5_0
	module TableDefinition
	def references(*, **options)
	options[:index] ||= false
	super
	end
	alias :belongs_to :references
	def timestamps(**options)
	options[:null] = true if options[:null].nil?
	super
	end
	def column(name, type, index: nil, **options)
	options[:precision] ||= nil
	super
	end
	end
	def add_reference(table_name, ref_name, **options)
	options[:index] ||= false
	super
	end
	alias :add_belongs_to :add_reference
	def add_timestamps(table_name, **options)
	options[:null] = true if options[:null].nil?
	super
	end
	def index_exists?(table_name, column_name, **options)
	column_names = Array(column_name).map(&:to_s)
	options[:name] =
	if options[:name].present?
	options[:name].to_s
	else
	connection.index_name(table_name, column: column_names)
	end
	super
	end
	def remove_index(table_name, column_name = nil, **options)
	options[:name] = index_name_for_remove(table_name, column_name, options)
	super
	end
	def add_column(table_name, column_name, type, **options)
	if type == :datetime
	options[:precision] ||= nil
	end

As you can notice above they are in the V4_2 class when they should be in V6_1, in 9b9d9c7 I could verify this by changing the versions of the migrations from 4.2 to 6.1, please the errors here https://buildkite.com/rails/rails/builds/78711#d5c2b003-4552-4173-afc9-61e46d8b4fde

Solution

Move the definitions to v6_1 and tweak V5_0 to add support older than 5.0 365acb6

cc @guilleiguaran @zzak @ghiculescu

[Tonkpils]

👋 @robertomiranda @guilleiguaran this seems to have broken some migrations in our application when using create_table

class AddColumnWorks < ActiveRecord::Migration[4.2]
  def change
    add_column :my_table, :some_column, :datetime, default: -> { "NOW()" }, null: false
  end
end

works but

class AddColumnWorks < ActiveRecord::Migration[4.2]
  def change
    create_table :my_table do |t|
      t.datetime :some_column, default: -> { "NOW()" }, null: false
    end
  end
end

fails with Invalid default value for 'some_column'

[robertomiranda]

@Tonkpils interesting, it would be good to add a test case, which RDMS are you using?

[Tonkpils]

which RDMS are you using?

mysql

[ghiculescu]

@Tonkpils is the issue specific to ActiveRecord::Migration[4.2] or does it happen on newer ones too?

[Tonkpils]

is the issue specific to ActiveRecord::Migration[4.2] or does it happen on newer ones too?

The compatibility broke for ActiveRecord::Migration[4.2] up to 5.0. ActiveRecord::Migration[6.1] works.

[robertomiranda]

Looks like they are broken from 4.2 to 6.0 I'm working on a fix here #42631

[Tonkpils]

Looks like they are broken from 4.2 to 6.0 I'm working on a fix here #42606 (comment)

For added context and in case it helps @robertomiranda , looks like the query generated when on this PR looks like

t.datetime :mycolumn, default: -> { "NOW()" }
`mycolumn` datetime(6) DEFAULT NOW()

because the precision is added (not sure how), the NOW() default would require this same precision. I'm not sure if the same is true when using default: Time.now

[robertomiranda]

because the precision is added (not sure how), the NOW() default would require this same precision. I'm not sure if the same is true when using default: Time.now

In theory the compatibility.rb should prevent the introduction of this behaviour to Rails version previous to 7.0