Fix change_column not setting precision for sqlite #49090
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
There were a few 6.1 migration compatibility fixes in [previous][1] [commits][2]. Most importantly, those commits reorganized some of the compatibility tests to ensure that the tests would run against every Migration version. To continue the effort of improving test coverage for Migration compatibility, this commit converts tests for create_table and change_column setting the correct precision on datetime columns. While the create_table tests all pass, the change_column test did not pass for 7.0 versioned Migrations on sqlite. This was due to the sqlite adapter not using new_column_definition to set the options on the new column (new_column_definition is where precision: 6 gets set if no precision is specified). This happens because columns can't be modified in place in sqlite and instead the whole table must be recreated and the data copied. Before this commit, change_column would use the options of the existing column as a base and merge in the exact options (and type) passed to change_column. This commit changes the change_column method to replace the existing column without using the existing options. This ensures that precision: 6 is set consistently across adapters when change_column is used to create a datetime column. [1]: c2f838e [2]: 9b07b2d
rafaelfranca
force-pushed
the
hm-change-column-precision-6
branch
from
1013526
to
57a9e25
Compare
|
Can you open the benchmark PR? |
This was referenced Sep 2, 2023
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Motivation / Background
There were a few 6.1 migration compatibility fixes in previous commits. Most importantly, those commits reorganized some of the compatibility tests to ensure that the tests would run against every Migration version. To continue the effort of improving test coverage for Migration compatibility, this commit converts tests for create_table and change_column setting the correct precision on datetime columns.
While the create_table tests all pass, the change_column test did not pass for 7.0 versioned Migrations on sqlite. This was due to the sqlite adapter not using new_column_definition to set the options on the new column (new_column_definition is where precision: 6 gets set if no precision is specified). This happens because columns can't be modified in place in sqlite and instead the whole table must be recreated and the data copied. Before this commit, change_column would use the options of the existing column as a base and merge in the exact options (and type) passed to change_column.
Detail
This commit changes the change_column method to replace the existing column without using the existing options. This ensures that precision: 6 is set consistently across adapters when change_column is used to create a datetime column.
Additional information
If this is backport worthy I can make another PR since the tests will not cherry-pick cleanly onto 7-0-stable.
Checklist
Before submitting the PR make sure the following are checked:
[Fix #issue-number]