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

DB schema dumper uncompatibility in Rails 7.0 #43909

Closed
mpg-tomohiko-mimura opened this issue Dec 17, 2021 · 18 comments
Closed

DB schema dumper uncompatibility in Rails 7.0 #43909

mpg-tomohiko-mimura opened this issue Dec 17, 2021 · 18 comments

Comments

@mpg-tomohiko-mimura
Copy link

Steps to reproduce

git@github.com:mito5525/precision_sample_app.git
cd precision_sample_app
bundle install

bin/rails db:drop db:create db:migrate
bin/rails db:schema:dump
git diff
# There is no difference.

bin/rails db:schema:load
bin/rails db:schema:dump
git diff

Expected behavior

No diffs in schema.rb

Actual behavior

The following differences will occur

diff --git a/db/schema.rb b/db/schema.rb
index 298ff3e..6d02e6b 100644
--- a/db/schema.rb
+++ b/db/schema.rb
@@ -13,7 +13,7 @@
 ActiveRecord::Schema.define(version: 2021_12_17_090228) do

   create_table "tasks", charset: "utf8mb4", force: :cascade do |t|
-    t.datetime "deleted_at"
+    t.datetime "deleted_at", precision: 6
     t.datetime "created_at", precision: 6, null: false
     t.datetime "updated_at", precision: 6, null: false
   end

I think there was a lack of compatibility considerations in #42297.

System configuration

Rails version: v7.0.0

Ruby version: v3.0.3

@morgoth
Copy link
Member

morgoth commented Jan 18, 2022

Same here.

  1. We dump schema from production database backup (so this is our source of truth)
  2. The schema contains columns as timestamp without time zone as this is how it was in previous versions and is in our production db
  3. From this schema.rb we bootstrap local database, which creates columns that are timestamp(6) without time zone so not really what is defined in the schema
  4. On the db:schema:dump the precision: 6 is added

@robertomiranda do you have a clue maybe what could cause it in #42297 ?

@bfad
Copy link

bfad commented Jan 20, 2022

We are also seeing an issue here. Our schema.rb doesn't specify a precision for MySQL datetimes, and none in set in our databases. However, if we drop and reload our databases in Rails 7, a precision of 6 is added to the MySQL fields, but the schema.rb file remains unchanged.

@bfad
Copy link

bfad commented Jan 20, 2022

Looks like #44171 may help fix this issue.

@morgoth
Copy link
Member

morgoth commented Jan 27, 2022

The merged PR was about mysql. We're using postgresql, so I think this still will be an issue

@rafaelfranca
Copy link
Member

Before loading the schema with a new version of Rails you should dump it. I'm going to fix this requirement, but right now that is the correct way to solve the issue.

@rafaelfranca
Copy link
Member

Long term fix is here #44286

rafaelfranca added a commit that referenced this issue Jan 28, 2022
Since #42297, Rails now generates
datetime columns on MySQL with a default precision of 6. This means
that users upgrading to Rails 7.0 from 6.1, when loading the database
schema, would get the new precision value, which would not match the
production schema.

To avoid problems like this in the future,
Rails will now freeze `ActiveRecord::Schema` class to
be the 7.0 implementation, and will allow access to other version
using the same mechanism of `ActiveRecord::Migration`.

The schema dumper will generate the new format which will include the
Rails version and will look like this:

```
ActiveRecord::Schema[7.0].define
```

Related to #43934 and #43909.
@justisb
Copy link

justisb commented Feb 3, 2022

Before loading the schema with a new version of Rails you should dump it. I'm going to fix this requirement, but right now that is the correct way to solve the issue.

I don't think that would solve this issue- because the old default precision: nil is not written in the dump. So there is no way to represent the Rails 6.1 (non-timestamps) datetime default precision in schema.rb, as the lack of precision keyword causes db:schema:load to always use precision: 6 in Rails 7.0.

I believe your pending PR that allows specifying the compatibility version of the Schema class will fix it- that solution looks great. But I've found no simple temporary fix, and have currently resorted to monkey patching ActiveRecord::Migration with the Compatibility::V6_1 definitions.

@rafaelfranca
Copy link
Member

I don't think that would solve this issue- because the old default precision: nil is not written in the dump.

This is already fixed in 7-0-stable.

rafaelfranca added a commit that referenced this issue Feb 7, 2022
Since #42297, Rails now generates
datetime columns on MySQL with a default precision of 6. This means
that users upgrading to Rails 7.0 from 6.1, when loading the database
schema, would get the new precision value, which would not match the
production schema.

To avoid problems like this in the future,
Rails will now freeze `ActiveRecord::Schema` class to
be the 7.0 implementation, and will allow access to other version
using the same mechanism of `ActiveRecord::Migration`.

The schema dumper will generate the new format which will include the
Rails version and will look like this:

```
ActiveRecord::Schema[7.0].define
```

Related to #43934 and #43909.
@justisb
Copy link

justisb commented Feb 7, 2022

I don't think that would solve this issue- because the old default precision: nil is not written in the dump.

This is already fixed in 7-0-stable.

I see the patch eca89fd from PR #44171 on 7-0-stable, but that will only affect MySQL and not Postgres, right? Whereas your PR should effectively solve for any DB adapter after setting the schema.rb version to 6.1. Unless you're referring to a different fix.

@rafaelfranca
Copy link
Member

Postgres should not have that problem. Does it?

@justisb
Copy link

justisb commented Feb 7, 2022

Here's a minimal reproduction with PG, forked from the MySQL example in the issue description:
https://github.com/justisb/precision_sample_app

bin/rails db:drop db:create db:migrate
bin/rails db:schema:dump
git diff
# No diff
bin/rails db:drop db:create db:schema:load
bin/rails db:schema:dump
git diff
diff --git a/db/schema.rb b/db/schema.rb
index 7664977..5e6ac9c 100644
--- a/db/schema.rb
+++ b/db/schema.rb
@@ -16,7 +16,7 @@ ActiveRecord::Schema.define(version: 2021_12_17_093703) do
   enable_extension "plpgsql"
 
   create_table "tasks", force: :cascade do |t|
-    t.datetime "deleted_at"
+    t.datetime "deleted_at", precision: 6
     t.datetime "created_at", precision: 6, null: false
     t.datetime "updated_at", precision: 6, null: false
   end

@rafaelfranca
Copy link
Member

Wait, that makes no sense at all. You don't even loaded the schema, so the precision in the database should not be 6. Are those really the steps you took?

@morgoth
Copy link
Member

morgoth commented Feb 7, 2022

@rafaelfranca The steps here #43909 (comment) are reproducing it for PostgreSQL

@rafaelfranca
Copy link
Member

That is still loading the schema generated in 6.1 without dumping in first in 7.0, so not really reproducing the issue. The fix for that is load the schema in 6.1 and them dump in 7.0.

@rafaelfranca
Copy link
Member

So steps to fix you problem are:

# Rails 6.1
bin/rails db:schema:load

# Rails 7.0
bin/rails db:schema:dump
# Commit the generated file

Or dump the production database using Rails 7.0.

@rafaelfranca
Copy link
Member

Ok. I think I got the problem now. #44356 should fix it. Can someone try?

@rafaelfranca
Copy link
Member

Ok, even #44356 would not fix it. #44358 should.

rafaelfranca added a commit that referenced this issue Feb 8, 2022
Since Rails 7.0 the datetime column precision is 6 by default.

That means that `t.datetime` calls without setting the `:precision`
option would have its precision set to 6.

The schema dumper was generating a call without precision for columns
with the precision set to `nil` (which has the same effect of 0), making
the schema dump incorrect for the next load since that would be interpreted
as if the precision was 6.

This didn't affect MySQL since #44171, since MySQL precision is 0 by default
not `nil` but it affected PostgreSQL and SQLite3.

Now the dumper will generate the precision as 0 for columns without
precision and omit it when the precision is 6.

Related to #43909.
public-rant pushed a commit to opensource-rant/rails that referenced this issue Feb 17, 2022
Since rails#42297, Rails now generates
datetime columns on MySQL with a default precision of 6. This means
that users upgrading to Rails 7.0 from 6.1, when loading the database
schema, would get the new precision value, which would not match the
production schema.

To avoid problems like this in the future,
Rails will now freeze `ActiveRecord::Schema` class to
be the 7.0 implementation, and will allow access to other version
using the same mechanism of `ActiveRecord::Migration`.

The schema dumper will generate the new format which will include the
Rails version and will look like this:

```
ActiveRecord::Schema[7.0].define
```

Related to rails#43934 and rails#43909.
public-rant pushed a commit to opensource-rant/rails that referenced this issue Feb 17, 2022
Since Rails 7.0 the datetime column precision is 6 by default.

That means that `t.datetime` calls without setting the `:precision`
option would have its precision set to 6.

The schema dumper was generating a call without precision for columns
with the precision set to `nil` (which has the same effect of 0), making
the schema dump incorrect for the next load since that would be interpreted
as if the precision was 6.

This didn't affect MySQL since rails#44171, since MySQL precision is 0 by default
not `nil` but it affected PostgreSQL and SQLite3.

Now the dumper will generate the precision as 0 for columns without
precision and omit it when the precision is 6.

Related to rails#43909.
@thisismydesign
Copy link

thisismydesign commented Feb 28, 2022

@rafaelfranca I have an issue with postgresql. When we migrated to Rails 7 precision: 6 got added to timestamps in our schema. It worked fine. After updating to 7.0.2 when I run db:migrate this is replaced with precision: nil. After this, one of our spec comparing dates is failing because of what looks like a precision related error:

Diff:
       @@ -1 +1 @@
       -Mon, 21 Feb 2022 14:10:49.640199500 CET +01:00
       +Mon, 21 Feb 2022 14:10:49.640199000 CET +01:00

More importantly, I think this means an actual schema change that would not be reflected on production, only locally.

Opened an issue: #44571

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

No branches or pull requests

7 participants