-
Notifications
You must be signed in to change notification settings - Fork 2.1k
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
Backport to release-14: Fix parsing of CAST() statements #10512 and #10514 #10517
Backport to release-14: Fix parsing of CAST() statements #10512 and #10514 #10517
Conversation
* Fix parsing of CAST() statements CAST() was treated as an alias for CONVERT() but with slightly different syntax. This is also described in the documentation at https://dev.mysql.com/doc/refman/8.0/en/cast-functions.html, specifically: With CAST(expr AS type syntax, the CAST() function takes an expression of any type and produces a result value of the specified type. This operation may also be expressed as CONVERT(expr, type), which is equivalent. If expr is NULL, CAST() returns NULL. This is wrong sadly. CAST() is not equivalent to CONVERT(), specifically in the context of a CREATE TABLE. For JSON keys, the ARRAY attribute is possible on a CAST(), but that is not accepted for a CONVERT(). The difference in parsing also shows in MySQL: ``` mysql> select convert(json_keys(c), char(64) array) from t; ERROR 1064 (42000): You have an error in your SQL syntax; check the manual that corresponds to your MySQL server version for the right syntax to use near 'array) from t' at line 1 mysql> select cast(json_keys(c) as char(64) array) from t; ERROR 1235 (42000): This version of MySQL doesn't yet support 'Use of CAST( .. AS .. ARRAY) outside of functional index in CREATE(non-SELECT)/ALTER TABLE or in general expressions' ``` Here the first statement can't be parsed at all. The second is properly parsed, but ARRAY is not allowed in the context of CAST() in this situation (and only in a CREATE TABLE). This means we should really treat these as two separate expressions and don't store them both in the same structure. The change here creates a separate CAST structure, removes the ARRAY option from CONVERT and updates the grammar and all tests accordingly. Signed-off-by: Dirkjan Bussink <[email protected]> * Handle new cast expression in evalengine and planbuilder Signed-off-by: Dirkjan Bussink <[email protected]> * evalengine: do not duplicate CAST/CONVERT translation Signed-off-by: Vicent Marti <[email protected]> Co-authored-by: Vicent Marti <[email protected]> Signed-off-by: Manan Gupta <[email protected]>
This was accidentally removed in vitessio#10512 but it shouldn't have been. Signed-off-by: Dirkjan Bussink <[email protected]>
Review ChecklistHello reviewers! 👋 Please follow this checklist when reviewing this Pull Request. General
Bug fixes
Non-trivial changes
New/Existing features
Backward compatibility
|
d900091
to
ae65d6e
Compare
I have tried to parse |
@skywhat A quick test shows that MySQL does not support this syntax either:
So Vitess would not support this either as it aims to be compatible with MySQL. If you do have a query that is parsed on MySQL but is not on Vitess, please open a new issue with the exact query so we can look at why it isn't working then. |
Description
This is a backport of the PRs #10512 and #10514 for release-14. These PRs fix the parsing of CAST function when used in a CREATE TABLE statement.
Related Issue(s)
Checklist
Deployment Notes