Repository navigation
Fix '0' column default preservation during non-strict INSERT - #569
noruzzamans wants to merge 1 commit into
Conversation
In non-strict mode, omitted columns are filtered from synthetic subqueries when they have a default value. A loose truthiness check previously caused string '0' to be evaluated as false, substituting an implicit default empty string instead of preserving the column default. Use strict null check consistent with the rest of the codebase, and add regression assertions in testNonStrictSqlModeNotNullWithDefault.
|
No actionable comments were generated in the recent review.
 |
| Layer / File(s) | Summary |
|---|---|
Recognize zero defaults packages/mysql-on-sqlite/src/sqlite/class-wp-mysql-on-sqlite.php, packages/mysql-on-sqlite/tests/WP_MySQL_On_SQLite_Tests.php |
The omitted-column filter checks whether COLUMN_DEFAULT is non-null. The test covers omitted string and integer columns with zero defaults. |
Priority:
Estimated code review effort: 2 (Simple) | ~8 minutes
Change: Bug fix · Severity of issue fixed: Medium
Possibly related issues
- WordPress/sqlite-database-integration#551 — This change affects omitted-column handling for zero defaults, but does not add full type normalization.
- WordPress/sqlite-database-integration#545 — This change recognizes a non-null
YEAR DEFAULT '0', but does not convert the value to MySQL’s expected2000.
Possibly related PRs
- WordPress/sqlite-database-integration#555 — That PR writes
COLUMN_DEFAULTmetadata; this PR reads it when handling omitted columns. - WordPress/sqlite-database-integration#542 — Both PRs affect how zero defaults in column metadata are handled.
Suggested reviewers: janjakes
Merge Risk: 
285be
-
The zero-default fix is correctly scoped and no merge-blocking risk remains.
Pre-merge checks: 
-
Check name Status Explanation Description Check Check skipped - CodeRabbit’s high-level summary is enabled. Title check The title clearly describes the main change: preserving '0' column defaults during non-strict INSERT operations. Linked Issues check Issue #552requires non-strict INSERT handling to preserve a defined default of string'0'. The PR changestranslate_insert_or_replace_body()to treatCOLUMN_DEFAULTas defined when it is not `n…Out of Scope Changes check The reviewed changes contain one driver condition update and regression test coverage in the related test method. Both changes directly support issue #552. No unrelated product behavior or files appea…Docstring Coverage Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 1 functions across 1 files. (1 skipped: 1 … Full details: Linked Issues check
-
Explanation
Issue
#552requires non-strict INSERT handling to preserve a defined default of string'0'. The PR changestranslate_insert_or_replace_body()to treatCOLUMN_DEFAULTas defined when it is notnull. The PR adds regression assertions forDEFAULT '0'andDEFAULT 0intestNonStrictSqlModeNotNullWithDefault. These changes address the linked issue. The supplied evidence does not report test execution, but it shows the required automated regression coverage.
Full details: Out of Scope Changes check
-
Explanation
The reviewed changes contain one driver condition update and regression test coverage in the related test method. Both changes directly support issue
#552. No unrelated product behavior or files appear in the supplied change summary.
Full details: Docstring Coverage
-
Explanation
Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 1 functions across 1 files. (1 skipped: 1 too large.)
-
Finishing Touches
-
🧪 Generate unit tests (beta)
-
- Create a new PR
-
- Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts
Warning
Some tools did not complete. Review the errors below.
🔧 PHPStan (2.3.0)
-
PHPStan reported 1 non-file analysis error(s); analysis is incomplete
🔧 PHPMD (3.0.0)
-
packages/mysql-on-sqlite/tests/WP_MySQL_On_SQLite_Tests.php
-
PHPMD could not process this file (exit code 255): 0/1 [░░░░░░░░░░░░░░░░░░░░░░░░░░░░] 0%
1/1 [▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓] 100%
PHP Fatal error: Allowed memory size of 134217728 bytes exhausted (tried to allocate 2097152 bytes) in phar:///usr/bin/phpmd/vendor/pdepend/pdepend/src/Util/Cache/Driver/FileCacheDriver.php on line 183
-
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.
Comment @coderabbitai help to get the list of available commands.
Fixes #552.
Description
In MySQL non-strict mode, omitted columns during
INSERTare filtered out from the synthetic wrapper subquery if they have a default value defined, allowing SQLite to populate their schema default.Previously,
translate_insert_or_replace_body()evaluated$column['COLUMN_DEFAULT']with a loose truthiness check ($has_default = $column['COLUMN_DEFAULT']). In PHP, string'0'evaluates tofalse, treating the column as having no default value and incorrectly populating it with an implicit default empty string ('').This PR:
null !== $column['COLUMN_DEFAULT'], consistent with other default checks in the driver.testNonStrictSqlModeNotNullWithDefaultcoveringDEFAULT '0'andDEFAULT 0.Testing instructions
Run unit tests in
packages/mysql-on-sqlite:Summary by CodeRabbit
0when inserting rows without explicitly providing those columns. Such columns now receive their configured defaults instead of being treated as if no default were set.