Translate MySQL CONVERT() expressions to SQLite#356
Conversation
Promote the %simpleExpr_factored fragment to a real "simpleExprBody" rule so it creates its own AST node. This separates the core expression (CONVERT, CAST, literals, etc.) from trailing modifiers (COLLATE, CONCAT_PIPES) that remain in the parent "simpleExpr" node, making individual expression handlers simpler.
SQLite doesn't support the CONVERT() function. Translate its two forms: - CONVERT(expr, type) is equivalent to CAST(expr AS type). - CONVERT(expr USING charset) is a character set conversion that is a no-op in SQLite, as all text is stored as UTF-8. Fixes #344
JanJakes
requested review from
a team,
bgrgicak and
brandonpayton
and removed request for
a team and
brandonpayton
|
The Query Monitor tests failure is unrelated to this PR and I will address it separately. It seems that QM 4.0 is incompatbible with our integration. |
| CONVERT('abc', BINARY) AS expr_1, | ||
| CONVERT('abc', CHAR) AS expr_2, | ||
| CONVERT('-10', SIGNED) AS expr_3, | ||
| CONVERT('-10', UNSIGNED) AS expr_4, |
There was a problem hiding this comment.
I have one question, in MySQL SELECT CONVERT('-10', UNSIGNED); will return 18446744073709551606.
Why is '-10' expected here?
There was a problem hiding this comment.
@bgrgicak I missed this, thanks for catching that! It seems that the main problem is that SQLite doesn't support UNSIGNED integers, so it cannot store values as large as 18446744073709551606.
That looks like a bigger problem, and I'm not sure whether it's reasonably solvable 😬 I guess for now, we should add a TODO comment explaining the issue.
There was a problem hiding this comment.
A TODO sounds good if we can't resolve it in this PR.
In this case, is it better to throw an error or silently return the incorrect value?
There was a problem hiding this comment.
@bgrgicak There are likely more issues like this where we translate a MySQL construct to a similar one in SQLite, but they don't behave 100% the same. I'm not sure if UNSIGNED is the only incompatibility here, but it will affect not only CONVERT but especially CAST and potentially some arithmetic, etc.
Therefore, I think the TODO is fine (and I can also create a ticket), and we don't need to try to detect and fail the invalid cases at the moment. Fortunately, casting a signed negative integer to an unsigned one seems to be a very strange use case that is hopefully not encountered in reality.
MySQL wraps negative values on UNSIGNED casts using modular arithmetic (2^64), but SQLite has no unsigned integer type. Mark this as a known limitation to be addressed.
## Release `2.2.23` Version bump and changelog update for release `2.2.23`. **Changelog draft:** * Add Query Monitor 4.0 support ([#357](#357)) * Translate MySQL CONVERT() expressions to SQLite ([#356](#356)) **Full changelog:** v2.2.22...release/v2.2.23 ## Next steps 1. **Review** the changes in this pull request. 2. **Push** any additional edits to this branch (`release/v2.2.23`). 3. **Merge** this pull request to complete the release. Merging will automatically build the plugin ZIP, create a [GitHub release](https://github.com/WordPress/sqlite-database-integration/releases), and deploy to [WordPress.org](https://wordpress.org/plugins/sqlite-database-integration/).
2 participants
Summary
Fixes #344.
MySQL's
CONVERT()function was passed through to SQLite unchanged, causing queries likeSELECT CONVERT('Customer' USING utf8mb4) COLLATE utf8mb4_binto fail with a syntax error.This PR has two parts:
Extract
simpleExprBodyas a named grammar rule. The%simpleExpr_factoredfragment is promoted to a realsimpleExprBodyrule so it creates its own AST node. This separates the core expression (CONVERT, CAST, literals, etc.) from trailing modifiers (COLLATE, CONCAT_PIPES) that remain in the parentsimpleExprnode, making individual expression handlers simpler.Translate
CONVERT()expressions. Adds explicit handling for both forms ofCONVERT()in the AST-based driver:CONVERT(expr, type)is translated toCAST(expr AS type), reusing the existingcastTypetranslation.CONVERT(expr USING charset)is reduced to just the expression, as SQLite stores all text as UTF-8 and charset conversions are not needed.Test plan
testConverttranslation test — verifies SQL-to-SQL translation for both CONVERT forms and COLLATE.testConvertExpression— verifies CONVERT with type casting (BINARY, CHAR, SIGNED, UNSIGNED, DECIMAL, DATE).testConvertUsingExpression— verifies CONVERT with charset conversion (utf8mb4, utf8, latin1).testConvertUsingWithCollate— verifies the exact query from CONVERT() queries fail with error #344.testConvertWithColumnReferences— verifies CONVERT with column references in SELECT, WHERE, and ORDER BY.