Fix MySQL::SchemaDumper behavior about datetime precision value #44171
Conversation
By rails#42297, the default value of datetime precision in rails is now 6. With this change, if the value of datetime precision is 0, it should be dumped into schema, but it is not. So I modified it to dump to schema when the value of datetime precision is 0, and not to dump when the value is 6, which is the default value of rails.
|
Oh, so in postgresql and sqlite, value of precision is dumped regardless of whether it is default or not |
| @@ -53,7 +53,7 @@ def schema_limit(column) | |||
| end | |||
|
|
|||
| def schema_precision(column) | |||
| super unless /\A(?:date)?time(?:stamp)?\b/.match?(column.sql_type) && column.precision == 0 | |||
| super unless /\Atime(?:stamp)?\b/.match?(column.sql_type) && column.precision == 0 | |||
There was a problem hiding this comment.
Should this change to the following?
| super unless /\Atime(?:stamp)?\b/.match?(column.sql_type) && column.precision == 0 | |
| super unless /\A(?:date)?time(?:stamp)?\b/.match?(column.sql_type) && column.precision == 6 |
Ie. keep the same behavior towards datetime and timestamp columns, but do not dump the precision if it is the default. Ideally I'd have liked to refer to a constant (eg. DEFAULT_TIMESTAMP_PRECISION somewhere, but unfortunately, the code is littered with the magic literal 6...
There was a problem hiding this comment.
In PostgreSQL, it always dumps the value of precision, and I thought it would be simpler to use the same specification for MySQL.
I think it would be better to always dump the precision value for time and timestamp types.
There was a problem hiding this comment.
Are there any reason not to dump the default value of precision?
|
@robertomiranda Hello! 👋🏻 Hope you're doing well! |
|
@y0t4 @rafaelfranca Did this need a changelog? According to this comment, Rails' behavior changes from not dumping the default value of precision, to always dumping the value of precision, which will result in changes in developers' applications' |
|
@davidstosik sorry I'm late here, didn't notice earlier seem like Rafael has a permanent fix for this issue #44286 |
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.
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.
Summary
By #42297, the default value of datetime precision in Rails is now 6.
In the schema managed by Rails, I think preferred behavior is to omit the code when precision is 6, but write it for other values.
I think this change will help #43934.