Skip to content

Fix ON UPDATE CURRENT_TIMESTAMP for explicit values and unchanged rows - #560

Merged
adamziel merged 2 commits into
trunkfrom
on-update-current-timestamp
Oct 23, 2026
Merged

adamziel merged 2 commits into
trunkfrom
on-update-current-timestamp

Conversation

@JanJakes

@JanJakes JanJakes commented Oct 23, 2026 •

Copy link
Copy Markdown
Member

The driver emulated ON UPDATE CURRENT_TIMESTAMP with a trigger that ran after every UPDATE. It overwrote explicit timestamps and changed timestamps on unchanged rows:

CREATE TABLE t (id INT PRIMARY KEY, n INT, ts TIMESTAMP NULL ON UPDATE CURRENT_TIMESTAMP);
INSERT INTO t VALUES (1, 1, '2000-01-01 00:00:00');

UPDATE t SET ts = '2001-01-01 00:00:00';  -- MySQL: keeps the explicit value
UPDATE t SET n = n;                       -- MySQL: keeps the timestamp
UPDATE t SET n = 2, ts = ts;              -- MySQL: keeps the timestamp
-- Previously, all three set ts to the current time.

Automatic updates were also missing in these cases:

  • Columns with DEFAULT CURRENT_TIMESTAMP never got an ON UPDATE trigger.
  • ALTER COLUMN ... SET DEFAULT (expr) could remove the trigger.
  • Rebuilding a legacy table could drop its triggers and lose its ON UPDATE behavior.

Changes

This PR handles automatic timestamps in the translated UPDATE and INSERT ... ON DUPLICATE KEY UPDATE statements:

  • Update timestamps only when the row changes. Compare the new values with the stored ones after type conversion.
  • Evaluate assignments once per row. Reuse the new values for change detection, keeping RAND() results consistent and avoiding repeated subqueries.
  • Keep explicitly assigned timestamps. Assigning a value, the column itself (ts = ts), or DEFAULT prevents the automatic update, as MySQL documents.
  • Handle columns with defaults. Automatic updates work with DEFAULT CURRENT_TIMESTAMP, expression defaults, and defaults changed by ALTER COLUMN.
  • Migrate old triggers. Restore missing ON UPDATE metadata before other migrations can rebuild tables, then remove the legacy and AST driver triggers. Match exact trigger names and their tables to preserve unrelated user triggers.

As in MySQL, changing 'a' to 'A' counts as a change. Assigning '1' to an INT column already containing 1 does not. Expected results were checked against MySQL 8.4 and 8.0.

Fixes #554.
Fixes #547.

Summary by CodeRabbit

  • Bug Fixes
    • ON UPDATE CURRENT_TIMESTAMP now updates automatically when another value in the row changes, while preserving the timestamp when assigned values remain unchanged. Explicit timestamp assignments continue to be respected.
    • Timestamp behavior is more consistent across updates involving aliases, joined tables, multiple timestamp columns, and duplicate-key updates.
  • Compatibility
    • Existing databases are migrated to preserve automatic timestamp behavior while removing legacy timestamp triggers. User-created triggers are retained.

@coderabbitai

coderabbitai Bot commented Oct 23, 2026 •

Copy link
Copy Markdown

Review in Change Stack →

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration
  • Source mode: Auto
  • Repository: Unavailable
  • Branch: Unavailable
  • File: No configuration file from this repository applied
  • Commit: Unavailable
  • Layers: defaults
  • Review profile: CHILL
  • Plan: Advanced
  • Run ID: ca2a94a6-fef9-4cb1-b1fa-5346e15eaac6
📥 Commits

Reviewing files that changed from the base of the PR and between 958f029 and 0a5f1db.

📒 Files selected for processing: 4
  • packages/mysql-on-sqlite/src/sqlite/class-wp-mysql-on-sqlite.php
  • packages/mysql-on-sqlite/src/sqlite/class-wp-sqlite-configurator.php
  • packages/mysql-on-sqlite/tests/WP_MySQL_On_SQLite_Tests.php
  • packages/mysql-on-sqlite/tests/WP_MySQL_On_SQLite_Translation_Tests.php

Included review availability: This review used your included allowance. Your plan provides up to 2 included reviews per hour; 1 remain after this review.

📝 Walkthrough

The driver now emulates ON UPDATE CURRENT_TIMESTAMP in translated update statements instead of using SQLite triggers. Configuration migrates legacy triggers by restoring timestamp metadata and dropping matched triggers. Tests cover update behavior, trigger migration, and rollback.

Changes: ON UPDATE CURRENT_TIMESTAMP

Layer / File(s) Summary
Translate automatic timestamp updates
packages/mysql-on-sqlite/src/sqlite/class-wp-mysql-on-sqlite.php, packages/mysql-on-sqlite/tests/WP_MySQL_On_SQLite_Tests.php, packages/mysql-on-sqlite/tests/WP_MySQL_On_SQLite_Translation_Tests.php
Translated updates compare assigned values with stored values and set unassigned automatic timestamp columns to CURRENT_TIMESTAMP only when a value changes. Explicit assignments remain intact, and assignment expressions are evaluated once. Table creation no longer generates timestamp triggers. Tests cover aliases, joins, duplicate-key updates, defaults, and multiple timestamp columns.
Migrate legacy timestamp triggers
packages/mysql-on-sqlite/src/sqlite/class-wp-sqlite-configurator.php, packages/mysql-on-sqlite/tests/WP_MySQL_On_SQLite_Tests.php
For saved driver versions at or before 3.1.1, configuration restores ON UPDATE CURRENT_TIMESTAMP metadata from matching legacy triggers and drops those triggers. Tests cover user-trigger preservation, legacy names, quoted identifiers, table rebuilds, and rollback.

Priority: ➖ Normal

Estimated code review effort: 4 (Complex) | ~45 minutes

Change: Bug fix · Severity of issue fixed: Medium

Sequence Diagram(s)

sequenceDiagram
  participant execute_update_statement
  participant translate_update_list
  participant SQLite
  execute_update_statement->>translate_update_list: Pass update list and target alias
  translate_update_list->>SQLite: Generate assignments with conditional timestamp updates
  SQLite-->>execute_update_statement: Execute translated update
Loading

Possibly related issues

  • #553: The tests cover automatic timestamp behavior after ALTER COLUMN changes a default, which overlaps with this issue’s default-persistence objective.
  • #556: The tests cover DROP DEFAULT on columns with ON UPDATE CURRENT_TIMESTAMP, although the issue’s broader nullable-column behavior is separate.

Possibly related PRs

Suggested reviewers: chubes4

Merge Risk: ⚪ Minimal · up to 0a5f1

This change emulates ON UPDATE CURRENT_TIMESTAMP inside translated UPDATE statements and removes the old triggers during migration, so explicit timestamps and unchanged rows are handled as in MySQL. I found no concrete merge-blocking risk in the supplied evidence.

Pre-merge checks: Passed 4 · Inconclusive 1

Failed Failed checks: 1 inconclusive

Check name Status Explanation Resolution
Docstring Coverage Inconclusive Docstring coverage is 70.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 10 functions across 3 files. (1 skipped: … Write docstrings for the functions missing them to satisfy the coverage threshold.
Passed Passed checks: 4 passed
Check name Status Explanation
Description Check Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check Passed The title clearly summarizes the main change: fixing automatic timestamp behavior for explicit assignments and unchanged rows.
Linked Issues check Passed The changes meet the coding requirements in #554 and #547. translate_update_list() reads EXTRA, evaluates assigned expressions once, compares converted values with COLLATE BINARY, preserves expl…
Out of Scope Changes check Passed The source changes, configurator migration, and test changes directly implement #554 and #547. Trigger removal, metadata recovery, expression reuse, and migration rollback coverage support the linked …
Full details: Linked Issues check

Explanation

The changes meet the coding requirements in #554 and #547. translate_update_list() reads EXTRA, evaluates assigned expressions once, compares converted values with COLLATE BINARY, preserves explicit timestamp assignments, and updates unassigned automatic-timestamp columns only when assigned data changes. The implementation applies to translated updates and duplicate-key updates, including aliases, defaults, temporary tables, and multiple automatic-timestamp columns. Table creation no longer adds unconditional driver triggers. The configurator restores ON UPDATE CURRENT_TIMESTAMP metadata and removes matching legacy and driver triggers before rebuild migrations. The added tests cover statement behavior, defaults, aliases, duplicate-key updates, migration, trigger matching, rollback, and repeated configuration.

Full details: Out of Scope Changes check

Explanation

The source changes, configurator migration, and test changes directly implement #554 and #547. Trigger removal, metadata recovery, expression reuse, and migration rollback coverage support the linked objectives. No unrelated change is established by the reviewed summary or focused diffs.

Full details: Docstring Coverage

Explanation

Docstring coverage is 70.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 10 functions across 3 files. (1 skipped: 1 too large.)

Resolution

Write docstrings for the functions missing them to satisfy the coverage threshold.

✨ Finishing Touches: 💡 1
📝 Generate docstrings: 💡
  • Commit to this branch
  • Create a new PR
🧪 Generate unit tests (beta)
  • Commit to this branch
  • 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 2 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.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@JanJakes
JanJakes force-pushed the on-update-current-timestamp branch from b35d7b6 to 02814c0 Compare October 23, 2026 13:50
The driver emulated ON UPDATE CURRENT_TIMESTAMP with an unconditional
AFTER UPDATE trigger. Explicit values, self-assignments, and unchanged
rows all got the current time. Columns with DEFAULT_GENERATED in EXTRA
never got a trigger.

Now, the SET list translation appends a CASE for each ON UPDATE column
that the statement doesn't assign. The column gets the current time only
when an assigned value differs from the stored one. This covers UPDATE
and ON DUPLICATE KEY UPDATE, and tables no longer get triggers.

See: #554
Remove the legacy and AST driver triggers before they can overwrite values
set by the UPDATE translation. Recover missing ON UPDATE column metadata
before other migrations rebuild tables and discard the legacy triggers.

Match exact trigger names and their tables to preserve unrelated triggers.
Cover legacy upgrades, table rebuilds, version gating, and rollback.

See: #554
Fixes: #547
$assigned_columns = array();
$value_expressions = array();
$changes = array();
foreach ( $assigned_values as $column_name => $value ) {

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

any chance $assigned_values is empty or contains something that isn't a literal column name?

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hm, it doesn't seem so. I'll go ahead and merge. Let's rollback and keep the discussion going if I'm wrong here.

@adamziel
adamziel merged commit e312a20 into trunk Oct 23, 2026
25 of 27 checks passed
@adamziel
adamziel deleted the on-update-current-timestamp branch October 23, 2026 19:24
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

ON UPDATE CURRENT_TIMESTAMP overwrites explicit values and updates unchanged rows ALTER TABLE drops legacy ON UPDATE CURRENT_TIMESTAMP triggers

2 participants