You are viewing a plain text version of this content. The canonical link for it is here.
Posted to github@datafusion.apache.org by "sgrebnov (via GitHub)" <gi...@apache.org> on 2024/07/22 03:41:35 UTC
[PR] Improve unparser MySQL compatibility [datafusion]
sgrebnov opened a new pull request, #11589:
URL: https://github.com/apache/datafusion/pull/11589
Please let me know if you prefer to split this into separate PRs. Combined as they 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](https://dev.mysql.com/doc/refman/8.4/en/cast-functions.html#function_cast) does not support Timestamp for CAST and requires DATETIME
> DATETIME[(M)]
Produces a [DATETIME](https://dev.mysql.com/doc/refman/8.4/en/datetime.html) value. If the optional M value is given, it specifies the fractional seconds precision.
Timezone information is correctly handled by MySQL when casting with `DATETIME`.
#### 2. Unsupported `date_part` function to extract Datetime subfield
MySQL 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:
```sql
date_part('YEAR', date '2001-02-16')
EXTRACT(YEAR from date '2001-02-16')
```
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](https://dev.mysql.com/doc/refman/8.4/en/cast-functions.html#function_cast) does not support BigInt and requires SIGNED for CAST
> SIGNED [INTEGER]
Produces a signed [BIGINT](https://dev.mysql.com/doc/refman/8.4/en/integer-types.html) value.
Note: sqlparser crate does not have Signed type so we use `ast::DataType::Custom` as a workaround
## Rationale 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).
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org
Re: [PR] Improve unparser MySQL compatibility [datafusion]
Posted by "alamb (via GitHub)" <gi...@apache.org>.
alamb merged PR #11589:
URL: https://github.com/apache/datafusion/pull/11589
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org
Re: [PR] Improve unparser MySQL compatibility [datafusion]
Posted by "sgrebnov (via GitHub)" <gi...@apache.org>.
sgrebnov commented on PR #11589:
URL: https://github.com/apache/datafusion/pull/11589#issuecomment-2246389843
Thank you Andrew @alamb!
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org
Re: [PR] Improve unparser MySQL compatibility [datafusion]
Posted by "alamb (via GitHub)" <gi...@apache.org>.
alamb commented on PR #11589:
URL: https://github.com/apache/datafusion/pull/11589#issuecomment-2245876211
Thanks again @sgrebnov
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org
Re: [PR] Improve unparser MySQL compatibility [datafusion]
Posted by "sgrebnov (via GitHub)" <gi...@apache.org>.
sgrebnov commented on code in PR #11589:
URL: https://github.com/apache/datafusion/pull/11589#discussion_r1685929050
##########
datafusion/sql/src/unparser/expr.rs:
##########
@@ -1184,10 +1338,7 @@ impl Unparser<'_> {
fractional_seconds_precision: None,
};
Ok(ast::Expr::Interval(interval))
- } else if v.months == 0
- && v.days >= 0
- && v.nanoseconds % 1_000_000 == 0
- {
+ } else if v.months == 0 && v.nanoseconds % 1_000_000 == 0 {
Review Comment:
This is intentional change (removed `v.days >= 0`) to fix interval generation for `IntervalStyle::SQLStandard` and negative days, the following additional test was added for SQLStandard (didn't work previously)
```rust
(
interval_month_day_nano_lit("-1.5 DAY"),
IntervalStyle::SQLStandard,
"INTERVAL '-1 -12:0:0.000' DAY TO SECOND",
),
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org
Re: [PR] Improve unparser MySQL compatibility [datafusion]
Posted by "alamb (via GitHub)" <gi...@apache.org>.
alamb commented on code in PR #11589:
URL: https://github.com/apache/datafusion/pull/11589#discussion_r1687023298
##########
datafusion/sql/src/unparser/dialect.rs:
##########
@@ -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?
Review Comment:
❤️
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org