-
Notifications
You must be signed in to change notification settings - Fork 1.2k
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
Improve unparser MySQL compatibility #11589
Merged
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
sgrebnov
commented
Jul 22, 2024
alamb
approved these changes
Jul 22, 2024
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you @sgrebnov -- this makes sense to me
@@ -36,8 +42,8 @@ pub trait Dialect: Send + Sync { | |||
true | |||
} | |||
|
|||
// Does the dialect use TIMESTAMP to represent Date64 rather than DATETIME? | |||
// E.g. Trino, Athena and Dremio does not have DATETIME data type | |||
/// Does the dialect use TIMESTAMP to represent Date64 rather than DATETIME? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
❤️
Thanks again @sgrebnov |
Thank you Andrew @alamb! |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Please let me know if you prefer me to split this into separate PRs. Combined as the changes are related/follow the same logic and to avoid resolving conflicts after PRs are merged.
Which issue does this PR close?
PR addresses unparser issues producing invalid sql for MySQL and implements support for
IntervalStyle::MySQL
1. Invalid
CAST(col AS Timestamp)
SQL for MySQL.MySQL cast function does not support Timestamp for CAST and requires DATETIME
Timezone information is correctly handled by MySQL when casting with
DATETIME
.2. Unsupported
date_part
function to extract Datetime subfieldMySQL does not support date_part function so PR introduces configurable option to specify Datetime subfield extraction style for unparsing. Different DBMSs follow different standards; popular ones are:
Some DBMSs, like Postgres, support both, whereas others like MySQL, Amazon Athena require EXTRACT.
3. invalid
CAST(col AS BigInt)
SQL for MySQL.MySQL cast function does not support BigInt and requires SIGNED for CAST
Note: sqlparser crate does not have Signed type so we use
ast::DataType::Custom
as a workaroundRationale for this change
PRs implements
IntervalStyle::MySQL
and adds support for alternate formats for Timestamp, Int64, Datetime part unparsing via Dialect customization.What changes are included in this PR?
Please see above
Are these changes tested?
Manual testing + unit test for each modification.
Are there any user-facing changes?
CustomDialectBuilder
now supports more options for Dialect customization (Timestamp, Int64 unparsing and Datetime part extraction).