Skip to content

Commit 2c5bfbd

Browse files
authored
Merge pull request rails#48974 from skipkayhil/hm-fix-change-table-precision
Fix 6.1 change_table setting datetime precision
2 parents ed5af00 + 9b07b2d commit 2c5bfbd

File tree

3 files changed

+151
-144
lines changed

3 files changed

+151
-144
lines changed

activerecord/CHANGELOG.md

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,7 @@
1+
* Fix `change_table` setting datetime precision for 6.1 Migrations
2+
3+
*Hartley McGuire*
4+
15
* Fix change_column setting datetime precision for 6.1 Migrations
26

37
*Hartley McGuire*

activerecord/lib/active_record/migration/compatibility.rb

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -209,6 +209,11 @@ def new_column_definition(name, type, **options)
209209
super
210210
end
211211

212+
def change(name, type, index: nil, **options)
213+
options[:precision] ||= nil
214+
super
215+
end
216+
212217
def column(name, type, index: nil, **options)
213218
options[:precision] ||= nil
214219
super

activerecord/test/cases/migration/compatibility_test.rb

Lines changed: 142 additions & 144 deletions
Original file line numberDiff line numberDiff line change
@@ -352,150 +352,6 @@ def migrate(x)
352352
connection.drop_table :more_testings rescue nil
353353
end
354354

355-
def test_datetime_doesnt_set_precision_on_change_table_4_2
356-
create_migration = Class.new(ActiveRecord::Migration[4.2]) {
357-
def migrate(x)
358-
create_table :more_testings do |t|
359-
t.datetime :published_at
360-
end
361-
end
362-
}.new
363-
364-
change_migration = Class.new(ActiveRecord::Migration[4.2]) {
365-
def migrate(x)
366-
change_table :more_testings do |t|
367-
t.datetime :published_at, default: Time.now
368-
end
369-
end
370-
}.new
371-
372-
ActiveRecord::Migrator.new(:up, [create_migration, change_migration], @schema_migration, @internal_metadata).migrate
373-
374-
assert connection.column_exists?(:more_testings, :published_at, **precision_implicit_default)
375-
ensure
376-
connection.drop_table :more_testings rescue nil
377-
end
378-
379-
def test_datetime_doesnt_set_precision_on_change_table_5_0
380-
create_migration = Class.new(ActiveRecord::Migration[5.0]) {
381-
def migrate(x)
382-
create_table :more_testings do |t|
383-
t.datetime :published_at
384-
end
385-
end
386-
}.new
387-
388-
change_migration = Class.new(ActiveRecord::Migration[5.0]) {
389-
def migrate(x)
390-
change_table :more_testings do |t|
391-
t.datetime :published_at, default: Time.now
392-
end
393-
end
394-
}.new
395-
396-
ActiveRecord::Migrator.new(:up, [create_migration, change_migration], @schema_migration, @internal_metadata).migrate
397-
398-
assert connection.column_exists?(:more_testings, :published_at, **precision_implicit_default)
399-
ensure
400-
connection.drop_table :more_testings rescue nil
401-
end
402-
403-
def test_datetime_doesnt_set_precision_on_change_table_5_1
404-
create_migration = Class.new(ActiveRecord::Migration[5.1]) {
405-
def migrate(x)
406-
create_table :more_testings do |t|
407-
t.datetime :published_at
408-
end
409-
end
410-
}.new
411-
412-
change_migration = Class.new(ActiveRecord::Migration[5.1]) {
413-
def migrate(x)
414-
change_table :more_testings do |t|
415-
t.datetime :published_at, default: Time.now
416-
end
417-
end
418-
}.new
419-
420-
ActiveRecord::Migrator.new(:up, [create_migration, change_migration], @schema_migration, @internal_metadata).migrate
421-
422-
assert connection.column_exists?(:more_testings, :published_at, **precision_implicit_default)
423-
ensure
424-
connection.drop_table :more_testings rescue nil
425-
end
426-
427-
def test_datetime_doesnt_set_precision_on_change_table_5_2
428-
create_migration = Class.new(ActiveRecord::Migration[5.2]) {
429-
def migrate(x)
430-
create_table :more_testings do |t|
431-
t.datetime :published_at
432-
end
433-
end
434-
}.new
435-
436-
change_migration = Class.new(ActiveRecord::Migration[5.2]) {
437-
def migrate(x)
438-
change_table :more_testings do |t|
439-
t.datetime :published_at, default: Time.now
440-
end
441-
end
442-
}.new
443-
444-
ActiveRecord::Migrator.new(:up, [create_migration, change_migration], @schema_migration, @internal_metadata).migrate
445-
446-
assert connection.column_exists?(:more_testings, :published_at, **precision_implicit_default)
447-
ensure
448-
connection.drop_table :more_testings rescue nil
449-
end
450-
451-
def test_datetime_doesnt_set_precision_on_change_table_6_0
452-
create_migration = Class.new(ActiveRecord::Migration[6.0]) {
453-
def migrate(x)
454-
create_table :more_testings do |t|
455-
t.datetime :published_at
456-
end
457-
end
458-
}.new
459-
460-
change_migration = Class.new(ActiveRecord::Migration[6.0]) {
461-
def migrate(x)
462-
change_table :more_testings do |t|
463-
t.datetime :published_at, default: Time.now
464-
end
465-
end
466-
}.new
467-
468-
ActiveRecord::Migrator.new(:up, [create_migration, change_migration], @schema_migration, @internal_metadata).migrate
469-
470-
assert connection.column_exists?(:more_testings, :published_at, **precision_implicit_default)
471-
ensure
472-
connection.drop_table :more_testings rescue nil
473-
end
474-
475-
def test_datetime_doesnt_set_precision_on_change_table_6_1
476-
create_migration = Class.new(ActiveRecord::Migration[6.1]) {
477-
def migrate(x)
478-
create_table :more_testings do |t|
479-
t.datetime :published_at
480-
end
481-
end
482-
}.new
483-
484-
change_migration = Class.new(ActiveRecord::Migration[6.1]) {
485-
def migrate(x)
486-
change_table :more_testings do |t|
487-
t.datetime :published_at, default: Time.now
488-
end
489-
end
490-
}.new
491-
492-
ActiveRecord::Migrator.new(:up, [create_migration, change_migration], @schema_migration, @internal_metadata).migrate
493-
494-
assert connection.column_exists?(:more_testings, :published_at, **precision_implicit_default)
495-
ensure
496-
connection.drop_table :more_testings rescue nil
497-
end
498-
499355
def test_datetime_doesnt_set_precision_on_add_column_5_0
500356
migration = Class.new(ActiveRecord::Migration[5.0]) {
501357
def migrate(x)
@@ -842,6 +698,148 @@ def precision_implicit_default
842698
end
843699
end
844700

701+
module DefaultPrecisionImplicitTestCases
702+
def test_datetime_doesnt_set_precision_on_change_table
703+
create_migration = Class.new(migration_class) {
704+
def migrate(x)
705+
create_table :more_testings do |t|
706+
t.datetime :published_at
707+
end
708+
end
709+
}.new(nil, 1)
710+
711+
change_migration = Class.new(migration_class) {
712+
def migrate(x)
713+
change_table :more_testings do |t|
714+
t.change :published_at, :datetime, default: Time.now
715+
end
716+
end
717+
}.new(nil, 2)
718+
719+
ActiveRecord::Migrator.new(:up, [create_migration, change_migration], @schema_migration, @internal_metadata).migrate
720+
721+
assert connection.column_exists?(:more_testings, :published_at, **precision_implicit_default)
722+
ensure
723+
connection.drop_table :more_testings rescue nil
724+
end
725+
726+
private
727+
def precision_implicit_default
728+
if current_adapter?(:Mysql2Adapter, :TrilogyAdapter)
729+
{ precision: 0 }
730+
else
731+
{ precision: nil }
732+
end
733+
end
734+
end
735+
736+
module DefaultPrecisionSixTestCases
737+
def test_datetime_sets_precision_6_on_change_table
738+
create_migration = Class.new(migration_class) {
739+
def migrate(x)
740+
create_table :more_testings do |t|
741+
t.datetime :published_at
742+
end
743+
end
744+
}.new(nil, 1)
745+
746+
change_migration = Class.new(migration_class) {
747+
def migrate(x)
748+
change_table :more_testings do |t|
749+
t.change :published_at, :datetime, default: Time.now
750+
end
751+
end
752+
}.new(nil, 2)
753+
754+
ActiveRecord::Migrator.new(:up, [create_migration, change_migration], @schema_migration, @internal_metadata).migrate
755+
756+
assert connection.column_exists?(:more_testings, :published_at, precision: 6)
757+
ensure
758+
connection.drop_table :more_testings rescue nil
759+
end
760+
end
761+
762+
class BaseCompatibilityTest < ActiveRecord::TestCase
763+
attr_reader :connection
764+
765+
def setup
766+
@connection = ActiveRecord::Base.connection
767+
@schema_migration = @connection.schema_migration
768+
@internal_metadata = @connection.internal_metadata
769+
770+
@verbose_was = ActiveRecord::Migration.verbose
771+
ActiveRecord::Migration.verbose = false
772+
end
773+
774+
def teardown
775+
ActiveRecord::Migration.verbose = @verbose_was
776+
@schema_migration.delete_all_versions rescue nil
777+
end
778+
end
779+
780+
class CompatibilityTest7_0 < BaseCompatibilityTest
781+
include DefaultPrecisionSixTestCases
782+
783+
private
784+
def migration_class
785+
ActiveRecord::Migration[7.0]
786+
end
787+
end
788+
789+
class CompatibilityTest6_1 < BaseCompatibilityTest
790+
include DefaultPrecisionImplicitTestCases
791+
792+
private
793+
def migration_class
794+
ActiveRecord::Migration[6.1]
795+
end
796+
end
797+
798+
class CompatibilityTest6_0 < BaseCompatibilityTest
799+
include DefaultPrecisionImplicitTestCases
800+
801+
private
802+
def migration_class
803+
ActiveRecord::Migration[6.0]
804+
end
805+
end
806+
807+
class CompatibilityTest5_2 < BaseCompatibilityTest
808+
include DefaultPrecisionImplicitTestCases
809+
810+
private
811+
def migration_class
812+
ActiveRecord::Migration[5.2]
813+
end
814+
end
815+
816+
class CompatibilityTest5_1 < BaseCompatibilityTest
817+
include DefaultPrecisionImplicitTestCases
818+
819+
private
820+
def migration_class
821+
ActiveRecord::Migration[5.1]
822+
end
823+
end
824+
825+
class CompatibilityTest5_0 < BaseCompatibilityTest
826+
include DefaultPrecisionImplicitTestCases
827+
828+
private
829+
def migration_class
830+
ActiveRecord::Migration[5.0]
831+
end
832+
end
833+
834+
class CompatibilityTest4_2 < BaseCompatibilityTest
835+
include DefaultPrecisionImplicitTestCases
836+
837+
private
838+
def migration_class
839+
ActiveRecord::Migration[4.2]
840+
end
841+
end
842+
845843
module LegacyPolymorphicReferenceIndexTestCases
846844
attr_reader :connection
847845

0 commit comments

Comments
 (0)