[samuelmbabhazi]

Fixes #454
Fixes #90

MySQL accepts date arithmetic written with the INTERVAL operators, in three forms: expr + INTERVAL n unit, expr - INTERVAL n unit and INTERVAL n unit + expr. The parser already accepts them, but the translator passed the expression through unchanged, so SQLite failed with near "5": syntax error on the example from #454, and on the NOW() - INTERVAL 1 DAY from #90.

The three forms now go through the same DATETIME() translation as DATE_ADD() and DATE_SUB(), extracted into translate_interval_arithmetic():

• the suffix forms are handled in a new bitExpr branch, which folds each interval into the expression on its left, so a chain like expr + INTERVAL 5 HOUR + INTERVAL 1 DAY becomes nested DATETIME() calls applied in MySQL's order;
• the prefix form is handled in translate_simple_expr_body(), next to the existing BINARY, CAST and CONVERT branches.

Sharing the helper also fixes two cases where DATE_ADD() and DATE_SUB() diverged from MySQL:

• a negative value, INTERVAL -5 HOUR, produced the modifier '+-5 HOUR', which SQLite rejects, so the result was NULL;
• a computed value, INTERVAL 2 * 3 HOUR, produced '-' || 2 * 3 || ' HOUR', and since || binds tighter than * in SQLite the result was NULL as well.

The value is now parenthesized and subtraction negates it instead of prefixing -. As a result, the modifier emitted for a column default such as DATE_ADD(NOW(), INTERVAL 1 YEAR) changes from '+' || 1 || ' YEAR' to (1) || ' YEAR'; the pinned assertion in testCreateTableWithExpressionDefault is updated accordingly. Both forms are valid SQLite, so existing databases are unaffected.

Testing

Every expected value in the new tests was taken from MySQL 8.4 by running the same statements, including the NULL cases, the WEEK conversion, a lowercase unit, a string value, and the column and WHERE cases in testIntervalArithmeticOnColumns.

Without the source change, the three new tests fail: the two infix tests with SQLite's syntax error, the DATE_ADD() one on the negative value.

MySQL rejects INTERVAL 5 HOUR - expr with a syntax error, and so does the parser here, so that form is left alone.

Not changed, and shared with the existing DATE_ADD() translation: a date-only input yields a DATETIME string where MySQL returns a DATE, month arithmetic does not clamp to the end of the month ('2024-01-31' + INTERVAL 1 MONTH gives 2024-03-02, MySQL gives 2024-02-29), and the QUARTER and composite units such as HOUR_MINUTE are not translated. Those are worth their own change.

Summary by CodeRabbit

• New Features

  • Added support for MySQL INTERVAL date arithmetic in SQLite-compatible queries.
  • Supports chained additions and subtractions, computed and negative intervals, column values, and WEEK intervals.
  • Supports interval expressions in filtering, sorting, and default date values.

• Bug Fixes

  • Improved handling of DATE_ADD() and DATE_SUB() expressions, including negative intervals and null values.

[coderabbitai]

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Advanced

Run ID: c55c6fa8-3634-4413-bda2-ceb3abf3ca6a

📥 Commits

Reviewing files that changed from the base of the PR and between 085ea60 and dccd7b2.

📒 Files selected for processing (2)

• packages/mysql-on-sqlite/src/sqlite/class-wp-mysql-on-sqlite.php
• packages/mysql-on-sqlite/tests/WP_MySQL_On_SQLite_Tests.php

Included review availability: Your plan provides up to 2 included reviews per hour; 1 remains after this review.

---

📝 Walkthrough

Walkthrough

The translator now supports MySQL INTERVAL arithmetic in date expressions and date functions. It converts intervals to SQLite DATETIME() modifiers and adds tests for computed, negative, chained, nullable, column-based, and default-expression cases.

Changes

Interval arithmetic support

Layer / File(s)	Summary
Translate interval expressions packages/mysql-on-sqlite/src/sqlite/class-wp-mysql-on-sqlite.php	bitExpr nodes now support chained interval arithmetic. Prefix intervals and DATE_ADD()/DATE_SUB() use shared translation logic with support for WEEK, computed values, and negative intervals.
Validate interval behavior packages/mysql-on-sqlite/tests/WP_MySQL_On_SQLite_Tests.php	Tests cover negative and computed intervals, both operand positions, chained expressions, NULL values, column expressions, query predicates, and translated default expressions.

Priority: ➖ Normal

Estimated code review effort: 3 (Moderate) | ~25 minutes

Change: Bug fix · Severity of issue fixed: Medium

Suggested reviewers: janjakes

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 45.45% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 11 functions across 2 files.	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ 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 and concisely describes the main change: translating date arithmetic that uses MySQL INTERVAL operators.
Linked Issues check	✅ Passed	The changes satisfy the coding requirements in [#454] and [#90]. translate_bit_expr() translates suffix interval arithmetic, including chained + INTERVAL and - INTERVAL forms such as `DATE(start…
Out of Scope Changes check	✅ Passed	The changes stay within the linked issue scope. The implementation updates the shared MySQL-to-SQLite date translation, and the added or updated PHPUnit tests verify the required date arithmetic and i…

• Fix all pre-merge checks with AI

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create a new PR

Warning

Some tools did not complete. Review the errors below.

🔧 PHPMD (2.15.0)

packages/mysql-on-sqlite/tests/WP_MySQL_On_SQLite_Tests.php

PHPMD could not process this file (exit code 255): PHP Fatal error: Allowed memory size of 134217728 bytes exhausted (tried to allocate 20480 bytes) in phar:///usr/bin/phpmd/vendor/pdepend/pdepend/src/main/php/PDepend/Util/Cache/Driver/FileCacheDriver.php on line 209

---

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

• X
• Mastodon
• Reddit
• LinkedIn

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

[Nandan108]

A real-world case for this PR: WooCommerce's checkout stock hold uses the operator form.

ReserveStock::reserve_stock_for_product() (ReserveStock.php, line 226) inserts the reservation with:

SELECT %d, %d, %d, NOW(), ( NOW() + INTERVAL %d MINUTE ) FROM DUAL

On trunk today, SELECT NOW() + INTERVAL 60 MINUTE fails with near "60": syntax error, while DATE_ADD(NOW(), INTERVAL 60 MINUTE) works. The failed insert leaves the reservation count at zero, so WooCommerce reports "Not enough units of ... are available in stock to fulfil this order." for every checkout of a product that doesn't allow backorders, as long as woocommerce_hold_stock_minutes is set (it is, by default). We hit this running WooCommerce on WordPress Playground with the 3.0.2 plugin, and worked around it by rewriting the query in a mu-plugin.

With this PR, the statement translates. Happy to test a rebased version against the WooCommerce checkout if that helps.

[samuelmbabhazi]

Thanks @Nandan108, good case. Both WooCommerce statements, NOW() + INTERVAL 60 MINUTE in the reservation insert and NOW() - INTERVAL 60 MINUTE in the cleanup query, are the forms this PR translates, and FROM DUAL is already handled, so the whole insert goes through.

The branch is current with trunk, so it can be tested as is. On Playground, the hold row in wc_reserved_stock should get an expires sixty minutes after its timestamp, and the stock message should go away.

[JanJakes]

Thanks for working on this. I drafted a longer review a while ago, but I think a single summary will be more useful.

I don’t think this can be merged in the current form. Some queries that previously failed with a syntax error now run and return incorrect results. In UPDATE and DELETE, that can mean changing or deleting data.

The main problem is that the translation now relies on the parse tree grouping, but our grammar doesn’t consistently represent MySQL’s operator precedence. Previously, the translator preserved the token order and left precedence to SQLite. Wrapping parts of that tree in DATETIME() changes how the expressions are evaluated.

To solve this, we either need to make the grammar represent MySQL precedence correctly, or we could try to regroup the expressions during translation. The new parser implemented in #429 solves all the precedence issues, so a full fix can also mean waiting for the parser switch. We could consider a focused fix here; I'm exploring some options in #539.

Below is a summary of the problems with the current approach.

Main issues

1. The prefix form absorbs logical operators.

   In MySQL, the operand after + stops before AND, OR, and XOR. Here, they can end up inside DATETIME():

   DELETE FROM t
   WHERE INTERVAL 30 DAY + d < '2025-01-01' AND n = 99;
   
   -- Translated as:
   DELETE FROM t
   WHERE DATETIME(d < '2025-01-01' AND n = 99, (30) || ' DAY');

   SQLite turns the boolean result into an ancient date string, which it treats as true. This can delete every row, while MySQL deletes none.

2. Numeric date operands have different meanings in MySQL and SQLite.

   MySQL does include comparisons in the prefix operand, so:

   WHERE INTERVAL 1 DAY + d > '2029-01-01'

   means:

   WHERE INTERVAL 1 DAY + (d > '2029-01-01')

   MySQL treats the resulting 0 or 1 as an invalid date and returns NULL. SQLite interprets it as a Julian day and produces a date string that counts as true. The same problem occurs with IS NULL.

   DATE_ADD() already has this limitation, but the new prefix form exposes it directly in WHERE clauses. Numeric dates need MySQL’s YYYYMMDD[hhmmss] interpretation, with invalid values returning NULL.

3. Unsupported units silently produce NULL.

   This affects QUARTER, MICROSECOND, composite units such as HOUR_MINUTE, and SQL_TSI_* aliases. For example:

   UPDATE t SET d = d + INTERVAL 1 QUARTER;

   sets d to NULL instead of adding three months. Before this PR, the query failed without changing the data.

   Please resolve units by token type, translate the supported ones, and reject the rest explicitly. MICROSECOND also needs special handling because DATETIME() drops fractional seconds.

4. Mixed ordinary and interval arithmetic is grouped incorrectly.

   With d holding the DATETIME value 2024-01-01 10:00:00:

   • d + INTERVAL 1 DAY + 0 becomes d + DATETIME(0, ...), returning -2689 instead of 20240102100000.
   • 1 + d + INTERVAL 1 DAY becomes 1 + DATETIME(d, ...), returning 2025 instead of 2024-01-02 10:00:01.

Other findings

• Interval values need MySQL-compatible conversion. For example, numeric INTERVAL 1.5 DAY adds two days in MySQL but 36 hours here. MySQL also accepts INTERVAL '1 DAY' DAY as one day; this translation returns NULL.
• The tests need to cover these failure cases, including logical operators, mixed arithmetic, UPDATE/DELETE, and unsupported or aliased units.
• The translate_bit_expr() docblock says date arithmetic is the only unsupported form, but DIV, MOD, and ^ also fail in SQLite.
• The INTERVAL forms of ADDDATE() and SUBDATE() could share the new helper; they still fail with a syntax error.

[samuelmbabhazi]

Thanks for the detailed review, Jan. Agreed: with the current grammar the grouping can't be trusted, and the prefix form and unit handling turned syntax errors into wrong data. #539 is the right way forward; I'll test the WooCommerce reservation case from Nandan against it and report there.

[JanJakes]

@samuelmbabhazi I'm now exploring an even simpler alternative that would cover the main cases in suffix forms like DATE(start_datetime - INTERVAL 5 HOUR) and NOW() - INTERVAL 1 DAY but would explicitly reject prefix forms (like INTERVAL 1 DAY + d), so they wouldn't silently translate incorrectly. I will report how much simpler that would get and if it would be a more reasonable fix until the parser switch.

[samuelmbabhazi]

@JanJakes Suffix-only with an explicit rejection of the prefix form sounds right to me, and the cases we have in hand all fit it: #454 is DATE(start_datetime - INTERVAL 5 HOUR), #90 is (NOW() - INTERVAL 1 DAY), and WooCommerce's stock hold uses ( NOW() + INTERVAL %d MINUTE ) for the reservation and timestamp < ( NOW() - INTERVAL %d MINUTE ) for the cleanup. I ran those WooCommerce statements against #539 with the driver directly (the reservation INSERT ... FROM DUAL, the reserved-stock read, the own-holds exclusion and the expiry DELETE): all behave as on MySQL, and the review's prefix cases now fail or return NULL as expected. Happy to rerun the same set on the simpler branch when it's up.

[JanJakes]

@samuelmbabhazi I repushed #539 with a more focused solution. I'll close this in favor of #539.