Skip to content
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

Parse signed/unsigned integer data types correctly in MySQL CAST #1739

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

mvzink
Copy link
Contributor

@mvzink mvzink commented Feb 21, 2025

MySQL doesn't have the same set of possible CAST types as for e.g. column definitions. For example, it raises a syntax error for CAST(1 AS INTEGER SIGNED) and instead expects CAST(1 AS SIGNED INTEGER).

This patch takes a somewhat unfortunate route of 1) adding a boolean flag that modifies the parse_data_type match expression, and 2) storing whether it was parsed this way in the Expr::Cast AST node to be used during formatting. Feedback and ideas for alternative approaches are very welcome.

Closes #1589

MySQL doesn't have the same set of possible CAST types as for e.g.
column definitions. For example, it raises a syntax error for `CAST(1 AS
INTEGER SIGNED)` and instead expects `CAST(1 AS SIGNED INTEGER)`.

This patch takes a somewhat unfortunate route of 1) adding a boolean
flag that modifies the `parse_data_type` match expression, and 2)
storing whether it was parsed this way in the `Expr::Cast` AST node to
be used during formatting. Feedback and ideas for alternative approaches
are very welcome.

Closes apache#1589
readysetbot pushed a commit to readysettech/readyset that referenced this pull request Feb 22, 2025
See relevant sqlparser [PR].

[PR]: apache/datafusion-sqlparser-rs#1739

Change-Id: I11a01b48f1649c6530cdbe7bd23c158e9fe4f888
format: Option<CastFormat>,
/// Whether this was parsed as a [MySQL]-style cast, which has a different syntax for
Copy link
Contributor

Choose a reason for hiding this comment

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

would it be possible to instead add the DataType variants that mysql uses? It seems like one issue is that the existing signed/unsigned datatypes aren't matching their expected string output. for example Datatype::UnsignedInteger outputs itself as INTEGER UNSIGNED.

Thinking expectation behaviour could be that we introduce
DataType::IntegerUnsigned => INTEGER UNSIGNED
and update
DataType::UnsignedInteger => UNSIGNED INTEGER

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.

MySQL datatype discrepancy between DDL and CAST
2 participants