Fix up decimal migration_diff #921
Closed
Conversation
This file contains hidden or 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
markstory
reviewed
Oct 16, 2025
dereuromark
force-pushed
the
fix-decimal
branch
from
77036ab to
6bf9e92
Compare
- Created schema-dump-test_comparisons_mysql.lock and pgsql.lock with DECIMAL(8,2) - Modified setup migrations to CREATE table instead of CHANGE column - Added expected output files in mysql/ and pgsql/ subdirectories with CHANGE column - This allows the diff command to compare DB (10,2) vs dump (8,2) and generate changeColumn migration
Added PHPStan ignore comment for setMigrations() call since we now support both AbstractMigration and BaseMigration which both implement MigrationInterface, but the parent method expects specifically AbstractMigration[].
dereuromark
force-pushed
the
fix-decimal
branch
from
6bf9e92 to
04708b7
Compare
…ompatible with Phinx - Modified BaseMigration constructor to accept both signatures: - BaseMigration($version) - AbstractMigration($env, $version, $input, $output) - Removed the entire getMigrations() override from CakeManager (100+ lines of duplicate code) - Simplified executeMigration() by removing conditional instantiation - This is a much cleaner solution than overriding the entire method
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.
Resolves #659
The Bug: The getColumnOption() method will only convert limit to precision/scale if precision is already set. But when a decimal column is changed (e.g., from DECIMAL(4,2) to DECIMAL(6,2)), the $changedAttributes array in
BakeMigrationDiffCommand::getColumns() may only contain the changed values, not necessarily the precision field!
The fix requires ensuring that when a decimal column is changed, both the precision (total digits) and scale (decimal places) are included in the $changedAttributes array, or modifying the getColumnOption() method to
handle decimal types even when the precision field is not initially set.