Skip to content

Commit

Permalink
Merge pull request #49121 from skipkayhil/hm-backport-49090
Browse files Browse the repository at this point in the history
Fix change_column not setting precision for sqlite [7-0-stable]
  • Loading branch information
rafaelfranca committed Sep 5, 2023
2 parents ac44103 + 8db97a7 commit 39eddbd
Show file tree
Hide file tree
Showing 4 changed files with 88 additions and 42 deletions.
5 changes: 5 additions & 0 deletions activerecord/CHANGELOG.md
Original file line number Diff line number Diff line change
@@ -1,3 +1,8 @@
* Fix `change_column` not setting `precision: 6` on `datetime` columns when
using 7.0+ Migrations and SQLite.

*Hartley McGuire*

* Fix unscope is not working in specific case

Before:
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,12 @@ module ActiveRecord
module ConnectionAdapters
module SQLite3
class TableDefinition < ActiveRecord::ConnectionAdapters::TableDefinition
def change_column(column_name, type, **options)
name = column_name.to_s
@columns_hash[name] = nil
column(name, type, **options)
end

def references(*args, **options)
super(*args, type: :integer, **options)
end
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -287,10 +287,7 @@ def change_column_null(table_name, column_name, null, default = nil) # :nodoc:

def change_column(table_name, column_name, type, **options) # :nodoc:
alter_table(table_name) do |definition|
definition[column_name].instance_eval do
self.type = aliased_types(type.to_s, type)
self.options.merge!(options)
end
definition.change_column(column_name, type, **options)
end
end

Expand Down
114 changes: 76 additions & 38 deletions activerecord/test/cases/migration/compatibility_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -314,22 +314,6 @@ def migrate(x)
end
end

def test_datetime_doesnt_set_precision_on_create_table
migration = Class.new(ActiveRecord::Migration[6.1]) {
def migrate(x)
create_table :more_testings do |t|
t.datetime :published_at
end
end
}.new

ActiveRecord::Migrator.new(:up, [migration], @schema_migration).migrate

assert connection.column_exists?(:more_testings, :published_at, **precision_implicit_default)
ensure
connection.drop_table :more_testings rescue nil
end

def test_datetime_doesnt_set_precision_on_add_column_5_0
migration = Class.new(ActiveRecord::Migration[5.0]) {
def migrate(x)
Expand All @@ -354,28 +338,6 @@ def migrate(x)
assert connection.column_exists?(:testings, :published_at, **precision_implicit_default)
end

def test_datetime_doesnt_set_precision_on_change_column_6_1
create_migration = Class.new(ActiveRecord::Migration[6.1]) {
def migrate(x)
create_table :more_testings do |t|
t.date :published_at
end
end
}.new(nil, 0)

change_migration = Class.new(ActiveRecord::Migration[6.1]) {
def migrate(x)
change_column :more_testings, :published_at, :datetime
end
}.new(nil, 1)

ActiveRecord::Migrator.new(:up, [create_migration, change_migration], @schema_migration).migrate

assert connection.column_exists?(:more_testings, :published_at, **precision_implicit_default)
ensure
connection.drop_table :more_testings rescue nil
end

if current_adapter?(:SQLite3Adapter)
def test_references_stays_as_integer_column_on_create_table_with_reference_6_0
migration = Class.new(ActiveRecord::Migration[6.0]) {
Expand Down Expand Up @@ -457,6 +419,44 @@ def migrate(x)
connection.drop_table :more_testings rescue nil
end

def test_datetime_doesnt_set_precision_on_create_table
migration = Class.new(migration_class) {
def migrate(x)
create_table :more_testings do |t|
t.datetime :published_at
end
end
}.new

ActiveRecord::Migrator.new(:up, [migration], @schema_migration).migrate

assert connection.column_exists?(:more_testings, :published_at, **precision_implicit_default)
ensure
connection.drop_table :more_testings rescue nil
end

def test_datetime_doesnt_set_precision_on_change_column
create_migration = Class.new(migration_class) {
def migrate(x)
create_table :more_testings do |t|
t.date :published_at
end
end
}.new(nil, 0)

change_migration = Class.new(migration_class) {
def migrate(x)
change_column :more_testings, :published_at, :datetime
end
}.new(nil, 1)

ActiveRecord::Migrator.new(:up, [create_migration, change_migration], @schema_migration).migrate

assert connection.column_exists?(:more_testings, :published_at, **precision_implicit_default)
ensure
connection.drop_table :more_testings rescue nil
end

private
def precision_implicit_default
if current_adapter?(:Mysql2Adapter, :TrilogyAdapter)
Expand Down Expand Up @@ -491,6 +491,44 @@ def migrate(x)
ensure
connection.drop_table :more_testings rescue nil
end

def test_datetime_sets_precision_6_on_create_table
migration = Class.new(migration_class) {
def migrate(x)
create_table :more_testings do |t|
t.datetime :published_at
end
end
}.new

ActiveRecord::Migrator.new(:up, [migration], @schema_migration).migrate

assert connection.column_exists?(:more_testings, :published_at, precision: 6)
ensure
connection.drop_table :more_testings rescue nil
end

def test_datetime_sets_precision_6_on_change_column
create_migration = Class.new(migration_class) {
def migrate(x)
create_table :more_testings do |t|
t.date :published_at
end
end
}.new(nil, 0)

change_migration = Class.new(migration_class) {
def migrate(x)
change_column :more_testings, :published_at, :datetime
end
}.new(nil, 1)

ActiveRecord::Migrator.new(:up, [create_migration, change_migration], @schema_migration).migrate

assert connection.column_exists?(:more_testings, :published_at, precision: 6)
ensure
connection.drop_table :more_testings rescue nil
end
end

class BaseCompatibilityTest < ActiveRecord::TestCase
Expand Down

0 comments on commit 39eddbd

Please sign in to comment.