-
Notifications
You must be signed in to change notification settings - Fork 120
prepare 5.x major release #995
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
Closed
Closed
+13,904
−18,625
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
Remove the commands that wrap the phinx commands. This has resulted in more code being removed as well. I also uncovered a bug in `BaseSeed` where it would fail to invoke seeds that were in a non-default `source`.
I'm trying to keep the diff small but stan builds are finding all the runtime links to deleted code.
* Remove commands that are entry points to phinx Remove the commands that wrap the phinx commands. This has resulted in more code being removed as well. I also uncovered a bug in `BaseSeed` where it would fail to invoke seeds that were in a non-default `source`. * Remove more code that supported phinx. * Remove commented out code * Fix phpcs and stan builds * More fixes * Fix postgres tests. * Comment out code that will be deleted soon. I'm trying to keep the diff small but stan builds are finding all the runtime links to deleted code. * Appease typing gods * Fix phpcs * Make pattern work across more php versions
* Refactor to fully remove phinx dependency. * Fix tests. * Fix tests. * Fix URL. * Fix tests * Fix more tests. * Fix phpcs * Fix constraint action generation --------- Co-authored-by: Mark Story <mark@mark-story.com>
* Remove now unused code from imported phinx code Slim down `ConfigInterface` to only be what we are using. * Fix phpcs * Remove a backend condition in a template
* Fix risky tests * 5.x Remove adapter getPhinxType This was a method inherited from phinx and maintained for backwards compatibility. * Update baseline file
Remove getSqlType(). This method is now unused and has been replaced by cakephp/database features.
Remove getSqlType(). This method is now unused and has been replaced by cakephp/database features.
More follow up on the column type cleanup.
5.x - Remove some attributes that are no longer used
* Remove usage of deprecated constants. Remove usage of column type constants that are no longer supported. * Fix mysql tests * Update baseline file * Fix phpcs
Use the dev branch as 5.3 release hasn't started yet, but I'd like to validate the new reflection API by integrating it into migrations. * Remove remaining references to symfony classes. * Fix failing tests. * Remove unused cake_version block and update php version cakephp 5.3 requires 8.2 * Update deps to align with cakephp version * Don't ignore php versions * Fix expected SQL * Fix regression with postgres schema generation * Fix deprecation warnings from plugin classes * Restore jsonb column constant and update windows php version * Fix failing identity column tests cakephp/database does not support `identity => true, generated => null` as a way to remove identity clauses on columns, as `generated => null` is the default value which for identity columns ~= 'by default'. Fix and re-enable the identity column tests as cakephp 5.3 will have identity column support.
* Remove more phinx references * Fix tests * Correct constant value. * Fix phpcs
Use all the new methods that were added to simplify migrations. * Get postgres adapter tests passing * Update tests for mysql adapter - column names are case-sensitive. - ordering matters. * Move hasColumn() up to AbstractAdapter * Update sqlite has methods - columns are case-sensitive - order matters * Partially update sqlserver adapter tests - order and casing matter now. * Clean up some TODOs * Fix type error in stubfile * Fix tests - Column names are case-sensitive - Required fixes in cakephp/database to implement included columns
* Add support for timestampfractional type. This type should have support in migrations now that we are on cakephp/database. * Address a few TODOs * sqlserver now uses datetimefractional by default for columns generated by cakephp, datetime and timestamp columns use fractional versions. * Fix more sqlserver tests * Fix mistake
* Remove brittle mock test. getVersionLog() is called in enough places that we can rely on integration tests from the rest of the library. * Replace string queries with querybuilder. This removes a bunch of potential sql injection as we weren't safely escaping parameters before. * Cleanup
Having both `Migrations\Db\Table` and `Migrations\Db\Table\Table` can make reading migrations code confusing to read. Renaming one of the classes to TableMetadata avoids confusion.
…982) When a table was created via the API (e.g. $this->table()->create()) and then dropped via execute() (raw SQL), hasTable() would incorrectly return true because it checked an internal cache before querying the database. This fix limits the cache usage to dry-run mode only, where it's necessary to track what tables "would" exist. In normal mode, hasTable() now always queries the database to ensure accurate results. Also fixes SqlserverAdapter to pass the table name without schema prefix to the dialect, which was a latent bug hidden by the cache.
This adds support for PARTITION BY (RANGE, LIST, HASH, KEY) clauses when creating tables and managing partitions on existing tables. Features: - RANGE, RANGE COLUMNS, LIST, LIST COLUMNS, HASH, KEY partitioning - Composite partition keys - Add/drop partitions on existing tables - PostgreSQL declarative partitioning with auto-generated table names - Expression-based partitioning via Literal class New classes: - Partition: Value object for partition configuration - PartitionDefinition: Value object for individual partition definitions - AddPartition: Action for adding partitions to existing tables - DropPartition: Action for dropping partitions New Table methods: - partitionBy(): Define partitioning on new tables - addPartition(): Add partition definitions when creating tables - addPartitionToExisting(): Add partition to existing partitioned tables - dropPartition(): Remove partition from existing tables * Fix PHPStan and PHPCS errors in partition implementation - Remove redundant is_array() check in MysqlAdapter (is_scalar already excludes arrays) - Remove redundant ?? operator in PostgresAdapter (from key always exists) - Remove : static return type from Partition::addDefinition() per CakePHP coding standards
Implements upsert functionality that inserts new rows and updates existing rows on duplicate key conflicts. Database support: - MySQL: ON DUPLICATE KEY UPDATE - PostgreSQL: ON CONFLICT (...) DO UPDATE SET - SQLite: ON CONFLICT (...) DO UPDATE SET Usage: $table->insertOrUpdate($data, $updateColumns, $conflictColumns); $this->insertOrUpdate($tableName, $data, $updateColumns, $conflictColumns); Refs #950 * Remove redundant comment --------- Co-authored-by: Mark Story <mark@mark-story.com>
* Strip 'Seed' suffix from seed names in all command output Makes seed name display consistent across all commands by removing the 'Seed' suffix. The run command confirmation was already doing this, but status, reset, and execution output still showed the full class name. * Add ' seed' suffix to display names for clarity Changes output from "Users" to "Users seed" to make it clearer that seeds are being displayed, not other entities. * Refactor seed display name logic to utility method Move the seed name suffix stripping logic to Util::getSeedDisplayName() to avoid code duplication across SeedCommand, SeedResetCommand, SeedStatusCommand, and Manager.
* Make unsigned the default for int columns. * Opt-in for unsigned column defaults Use tearDown() to restore Configure defaults instead of running tests in separate processes. This improves test performance. * Fix MigrationHelperTest failing when run with adapter tests The adapter tests (MysqlAdapterTest, etc.) drop and recreate the database in their setUp method, which destroys the schema tables created by SchemaLoader. MigrationHelperTest then fails because the users and special_tags tables no longer exist. Use #[RunTestsInSeparateProcesses] to ensure MigrationHelperTest runs in isolation with its own fresh schema.
* Fix multiple partition SQL syntax for MySQL When adding multiple partitions to an existing table, MySQL requires: ALTER TABLE foo ADD PARTITION (PARTITION p1 ..., PARTITION p2 ...) Previously, each AddPartition action generated its own ADD PARTITION clause, which when joined with commas resulted in invalid SQL: ALTER TABLE foo ADD PARTITION (...), ADD PARTITION (...) This fix: - Batches AddPartition actions together in executeActions() - Adds new getAddPartitionsInstructions() method to AbstractAdapter with a default implementation that calls the single partition method - Overrides getAddPartitionsInstructions() in MysqlAdapter to generate correct batched SQL: ADD PARTITION (PARTITION p1, PARTITION p2) - Similarly batches DropPartition actions for efficiency - Adds gatherPartitions() to Plan.php to properly gather partition actions - Includes extensive tests for single/multiple partition add/drop scenarios Refs #986 * Fix coding standard - use single quotes * Trigger CI re-run * Add test for combined partition and column operations * Add SetPartitioning action for partitionBy() + update() Enables adding partitioning to existing non-partitioned tables using: $table->partitionBy(Partition::TYPE_RANGE_COLUMNS, 'created') ->addPartition('p2023', '2024-01-01') ->update(); Previously this generated no SQL. Now it properly generates: ALTER TABLE `table` PARTITION BY RANGE COLUMNS (created) (...) Changes: - Add SetPartitioning action class - Update Plan to handle SetPartitioning in gatherPartitions() - Add getSetPartitioningInstructions() to AbstractAdapter/MysqlAdapter - Create SetPartitioning action in Table::executeActions() when updating * Add test for composite partition keys * Each adapter now only needs to implement the collection methods * Remove unused import --------- Co-authored-by: mscherer <dereuromark@web.de> Co-authored-by: Jamison Bryant <jbryant@ticketsauce.com>
- Remove redundant Exception catch when Throwable already covers it - Use consistent error output formatting with <error> tags - Add verbose trace output to SeedCommand for consistency - Remove unused Exception imports
- Add --fake flag to mark seeds as executed without running them - Add --seed option to seeds reset for selective seed reset - Both features mirror similar functionality in migrations * Show clear message when faking idempotent seeds Idempotent seeds are not tracked, so faking them doesn't make sense. Instead of showing misleading "faking"/"faked" messages, now shows "skipped (idempotent)" to clarify what's happening.
* Remove phinx backend test from 5.x The phinx backend was removed in 5.x, so the test for MigrationsRollbackCommand is not applicable. This test was incorrectly merged from 4.x where phinx backend still exists. * Update PostgreSQL timestamp comparison file for 5.x - Update documentation URL from phinx to migrations/5 - Update timestamp type to timestampfractional for columns with precision/scale
- Add insertOrUpdate() and insertOrSkip() documentation to seeding guide
- Document MySQL ALTER TABLE ALGORITHM/LOCK options
- Fix grammar issue in index.rst ("can has" -> "has")
- Update phinxlog references to generic "migrations tracking table"
Resolve conflicts: - EntryCommandTest: Use minimal assertions without backend-specific text - PostgresAdapterTest: Use flexible assertions for identity column syntax - PendingMigrationsMiddlewareTest: Use assertInstanceOf for proper type checking
- Fix README coverage badge pointing to 3.x instead of 5.x - Update MigrationHelper comments (remove outdated phinx TODOs) - Add database-specific limitations documentation - Fix PHPStan issues by adding proper type hints for Bake event subjects - Update PHPStan baseline to remove fixed errors
* Improve insertOrUpdate() API consistency across database adapters - Add deprecation warning for MySQL when conflictColumns is passed (ignored) - Add RuntimeException for PostgreSQL/SQLite when conflictColumns is missing - Document database-specific behavior in Table.php and SeedInterface.php - Add tests for PostgreSQL and SQLite conflict column validation * Fix use statement ordering in PostgresAdapterTest * Change MySQL conflictColumns deprecationWarning to trigger_error Since insertOrUpdate is a new feature, using deprecationWarning() doesn't make semantic sense. Switch to trigger_error() with E_USER_WARNING to alert developers that the parameter is ignored on MySQL without implying the feature will be removed. * Add documentation for insertOrSkip and insertOrUpdate methods Documents the insert modes with database-specific behavior caveats, particularly the MySQL vs PostgreSQL/SQLite differences for upsert.
Accepted the 5.x branch's documentation for insertOrUpdate() and insertOrSkip() which has the correct three-argument API signature.
Docs: Improve documentation for 5.x release
# Conflicts: # tests/TestCase/Db/Adapter/MysqlAdapterTest.php
* Quote database names in PostgreSQL and SQL Server adapters MySQL adapter properly quotes database names in createDatabase/dropDatabase using quoteTableName(), but PostgreSQL and SQL Server did not. - PostgreSQL: Use quoteSchemaName() for database name and quoteString() for charset - SQL Server: Use quoteSchemaName() for database name and quoteString() for string comparisons This ensures consistent identifier quoting across all database adapters. * Fix docblock typos and copy-paste errors - Fix @params typo to @param in BaseMigration.php foreignKey() and index() methods - Fix copy-paste docblock errors in test seed files where "NumbersSeed seed" was used for classes with different names
- Fix SQL Server sp_rename to use quoteString() for proper escaping - Fix foreign key column quoting in PostgreSQL and SQL Server adapters to use quoteColumnName() instead of hard-coded quotes - Fix MigrationHelper::tableStatement() to escape table names - Fix @params typos in BaseMigration docblocks (should be @param) - Fix copy-paste docblock errors in test seed files
Member
|
I don't think this should be merged. We need to keep the 4.x branch around for any future bug fixes that need to be made for 4.x. |
Member
Author
|
Yes |
* Fix release readiness issues for 5.x - Add cycle detection to seed dependency ordering to prevent infinite recursion when seeds have circular dependencies - Add 'unsigned' to valid column options for API consistency with 'signed' - Fix SQL injection vulnerability in MysqlAdapter by replacing addslashes() with proper driver escaping for column comments - Fix non-existent $io->error() method call in DumpCommand (should be err()) - Document SQL Server check constraints as unsupported with improved error messages guiding users to use raw SQL * Fix additional release readiness issues from deep dive - Restrict unserialize() to safe CakePHP schema classes only - Fix strpos() logic bug by using str_contains() - Initialize $command property to prevent uninitialized access - Fix weak equality (!=) to strict (!==) in Table::saveData() - Fix copy-paste bug in Migrator (was using 'down' instead of 'missing') - Replace assert() with explicit RuntimeException in BaseSeed * Add missing CakePHP schema classes to unserialize allowlist TableSchema contains nested Column, Index, Constraint and other schema objects that also need to be allowed for deserialization. * Fix docblock annotation spacing in SqlserverAdapter
* Improve type safety in nullable detection and comparisons - Fix strpos() casting bug in ColumnParser - (bool)strpos() returns false when needle is at position 0, use str_contains() instead - Replace loose comparisons (== !=) with strict comparisons (=== !==) in Manager.php for version and breakpoint checks * Cast breakpoint to int for cross-database compatibility PostgreSQL and SQL Server return boolean columns as strings or actual booleans instead of integers, causing strict equality checks to fail.
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.
With the last PRs being merged in we can soon prepare the release.