Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Upgrading to Rails 7.0 changes precision when changing column type to datetime #48965

Closed
willnet opened this issue Aug 17, 2023 · 2 comments · Fixed by #48969
Closed

Upgrading to Rails 7.0 changes precision when changing column type to datetime #48965

willnet opened this issue Aug 17, 2023 · 2 comments · Fixed by #48969

Comments

@willnet
Copy link
Contributor

willnet commented Aug 17, 2023

In Rails 6.1 and below, precision is nil when using change_column to set the column type to datetime without explicit option. In Rails 7.0, precision is 6 even if the migration version is specified as ActiveRecord::Migration[6.1]. This will result in different schemas when running rails db:migrate in Rails 6.1 and Rails 7.0.

#42297 changed the default datetime precision to 6. In Rails7.0, ActiveRecord::Migration[6.1] or lower version includes the following support to maintain the original behavior.

but there seems to be no support for change_column

Steps to reproduce

Using PostgreSQL because I can't reproduce with sqlite3

# frozen_string_literal: true

require "bundler/inline"

gemfile(true) do
  source "https://rubygems.org"

  git_source(:github) { |repo| "https://github.com/#{repo}.git" }

  # Activate the gem you are reporting the issue against.
  gem "activerecord", "~> 7.0.0"
  gem "pg"
end

require "active_record"
require "minitest/autorun"
require "logger"

# docker run --rm -e POSTGRES_PASSWORD=postgres -p 9999:5432 postgres
ActiveRecord::Base.establish_connection("postgres://postgres:postgres@localhost:9999/postgres")
ActiveRecord::Base.logger = Logger.new(STDOUT)

ActiveRecord::Schema[6.1].define do
  create_table :posts, force: true do |t|
    t.date :published_at
  end
end

class ChangePublishedAt < ActiveRecord::Migration[6.1]
  def change
    change_column :posts, :published_at, :datetime
  end
end

class Post < ActiveRecord::Base
end

class BugTest < Minitest::Test
  def test_precision_when_change_column_to_datetime
    ChangePublishedAt.migrate(:up)
    Post.reset_column_information
    published_at_column = Post.columns.find { |c| c.name == "published_at" }
    assert_nil published_at_column.precision # fail. Expected 6 to be nil.
  end
end

Expected behavior

the test pass

Actual behavior

the test fails.

System configuration

Rails version:
7.0.7
Ruby version:
3.2.2

@willnet
Copy link
Contributor Author

willnet commented Aug 17, 2023

For reference, here is the code to check the behavior in Rails 6.1.

# frozen_string_literal: true

require "bundler/inline"

gemfile(true) do
  source "https://rubygems.org"

  git_source(:github) { |repo| "https://github.com/#{repo}.git" }

  # Activate the gem you are reporting the issue against.
  gem "activerecord", "~> 6.1.0"
  gem "pg"
end

require "active_record"
require "minitest/autorun"
require "logger"

# docker run --rm -e POSTGRES_PASSWORD=postgres -p 9999:5432 postgres
ActiveRecord::Base.establish_connection("postgres://postgres:postgres@localhost:9999/postgres")
ActiveRecord::Base.logger = Logger.new(STDOUT)

ActiveRecord::Schema.define do
  create_table :posts, force: true do |t|
    t.date :published_at
  end
end

class ChangePublishedAt < ActiveRecord::Migration[6.1]
  def change
    change_column :posts, :published_at, :datetime
  end
end

class Post < ActiveRecord::Base
end

class BugTest < Minitest::Test
  def test_precision_when_change_column_to_datetime
    ChangePublishedAt.migrate(:up)
    Post.reset_column_information
    published_at_column = Post.columns.find { |c| c.name == "published_at" }
    assert_nil published_at_column.precision # pass
  end
end

@skipkayhil
Copy link
Member

Thanks for the great reproduction! I'll have a PR to fix this up shortly

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants