-
Notifications
You must be signed in to change notification settings - Fork 120
Improve insertOrUpdate() API consistency across database adapters #992
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
Conversation
| * - **MySQL**: Uses `ON DUPLICATE KEY UPDATE`. The `$conflictColumns` parameter is | ||
| * ignored because MySQL automatically applies the update to all unique constraint | ||
| * violations. Passing `$conflictColumns` will trigger a deprecation warning. | ||
| * | ||
| * - **PostgreSQL/SQLite**: Uses `ON CONFLICT (...) DO UPDATE SET`. The `$conflictColumns` | ||
| * parameter is required and must specify the columns that have a unique constraint. | ||
| * A RuntimeException will be thrown if this parameter is empty. | ||
| * | ||
| * - **SQL Server**: Not currently supported. Use separate insert/update logic. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should this also be in the docs? There are a lot of caveats to using this method.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Added docs for insertOrSkip() and insertOrUpdate() in docs/en/seeding.rst with the database-specific caveats in a warning block.
|
Changed |
- 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
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.
Documents the insert modes with database-specific behavior caveats, particularly the MySQL vs PostgreSQL/SQLite differences for upsert.
69f50b5 to
ca4a1d4
Compare
Summary
Addresses API consistency issues with
insertOrUpdate()across different database adapters:MySQL: The
$conflictColumnsparameter is ignored because MySQL'sON DUPLICATE KEY UPDATEautomatically applies to all unique constraints. Now triggers a deprecation warning when passed.PostgreSQL/SQLite: The
$conflictColumnsparameter is required. Now throws aRuntimeExceptionwith a clear message if missing or empty.Documentation: Added comprehensive docblocks explaining the database-specific behavior in
Table.phpandSeedInterface.php.Changes
AbstractAdapter::getUpsertClause()when$conflictColumnsis passed (MySQL)PostgresAdapter::getConflictClause()when$conflictColumnsis missingSqliteAdapter::getUpsertClause()when$conflictColumnsis missingTable::insertOrUpdate()andSeedInterface::insertOrUpdate()Test plan