alamb commented on code in PR #11466:
URL: https://github.com/apache/datafusion/pull/11466#discussion_r1678292019
##########
datafusion/sql/src/unparser/dialect.rs:
##########
@@ -95,4 +145,76 @@ impl Dialect for CustomDialect {
fn identifier_quote_style(&self, _: &str) -> Option<char> {
self.identifier_quote_style
}
+
+ fn supports_nulls_first_in_sort(&self) -> bool {
+ self.supports_nulls_first_in_sort
+ }
+
+ fn use_timestamp_for_date64(&self) -> bool {
+ self.use_timestamp_for_date64
+ }
+
+ fn interval_style(&self) -> IntervalStyle {
+ self.interval_style
+ }
+}
+
+// create a CustomDialectBuilder
Review Comment:
I think this comment belongs on `CustomDialectBuilder::new()`
Also, can you please add documenation to this (nice) new builder?
Specifically I think the struct itself should have an example of how to use it
and each method has a docstring that explains what it does (perhaps with a link
to the relevant method on `CustomDialect`
##########
datafusion/sql/src/unparser/expr.rs:
##########
@@ -1595,46 +1706,6 @@ mod tests {
),
(col("need-quoted").eq(lit(1)), r#"("need-quoted" = 1)"#),
(col("need quoted").eq(lit(1)), r#"("need quoted" = 1)"#),
Review Comment:
It might be nice to leave a hint that date/time testing moved elsewhere
```suggestion
(col("need quoted").eq(lit(1)), r#"("need quoted" = 1)"#),
// See test_interval_scalar_to_expr for interval literals
```
##########
datafusion/sql/src/unparser/expr.rs:
##########
@@ -1108,6 +1102,123 @@ impl Unparser<'_> {
}
}
+ fn interval_scalar_to_sql(&self, v: &ScalarValue) -> Result<ast::Expr> {
+ match self.dialect.interval_style() {
+ IntervalStyle::PostgresVerbose => {
+ let wrap_array = v.to_array()?;
+ let Some(result) = array_value_to_string(&wrap_array, 0).ok()
else {
+ return internal_err!(
+ "Unable to convert interval scalar value to string"
+ );
+ };
+ let interval = Interval {
+ value: Box::new(ast::Expr::Value(SingleQuotedString(
+ result.to_uppercase(),
+ ))),
+ leading_field: None,
+ leading_precision: None,
+ last_field: None,
+ fractional_seconds_precision: None,
+ };
+ Ok(ast::Expr::Interval(interval))
+ }
+ // If the interval standard is SQLStandard, implement a simple
unparse logic
+ IntervalStyle::SQLStandard => match v {
+ ScalarValue::IntervalYearMonth(v) => {
+ let Some(v) = v else {
+ return Ok(ast::Expr::Value(ast::Value::Null));
+ };
+ let interval = Interval {
+ value: Box::new(ast::Expr::Value(
+ ast::Value::SingleQuotedString(v.to_string()),
+ )),
+ leading_field: Some(ast::DateTimeField::Month),
+ leading_precision: None,
+ last_field: None,
+ fractional_seconds_precision: None,
+ };
+ Ok(ast::Expr::Interval(interval))
+ }
+ ScalarValue::IntervalDayTime(v) => {
+ let Some(v) = v else {
+ return Ok(ast::Expr::Value(ast::Value::Null));
+ };
+ let days = v.days;
+ let secs = v.milliseconds / 1_000;
+ let mins = secs / 60;
+ let hours = mins / 60;
+
+ let secs = secs - (mins * 60);
+ let mins = mins - (hours * 60);
+
+ let millis = v.milliseconds % 1_000;
+ let interval = Interval {
+ value: Box::new(ast::Expr::Value(
+ ast::Value::SingleQuotedString(format!(
+ "{days} {hours}:{mins}:{secs}.{millis:3}"
+ )),
+ )),
+ leading_field: Some(ast::DateTimeField::Day),
+ leading_precision: None,
+ last_field: Some(ast::DateTimeField::Second),
+ fractional_seconds_precision: None,
+ };
+ Ok(ast::Expr::Interval(interval))
+ }
+ ScalarValue::IntervalMonthDayNano(v) => {
+ let Some(v) = v else {
+ return Ok(ast::Expr::Value(ast::Value::Null));
+ };
+
+ if v.months >= 0 && v.days == 0 && v.nanoseconds == 0 {
+ let interval = Interval {
+ value: Box::new(ast::Expr::Value(
+
ast::Value::SingleQuotedString(v.months.to_string()),
+ )),
+ leading_field: Some(ast::DateTimeField::Month),
+ leading_precision: None,
+ last_field: None,
+ fractional_seconds_precision: None,
+ };
+ Ok(ast::Expr::Interval(interval))
+ } else if v.months == 0
+ && v.days >= 0
+ && v.nanoseconds % 1_000_000 == 0
+ {
+ let days = v.days;
+ let secs = v.nanoseconds / 1_000_000_000;
+ let mins = secs / 60;
+ let hours = mins / 60;
+
+ let secs = secs - (mins * 60);
+ let mins = mins - (hours * 60);
+
+ let millis = (v.nanoseconds % 1_000_000_000) /
1_000_000;
+
+ let interval = Interval {
+ value: Box::new(ast::Expr::Value(
+ ast::Value::SingleQuotedString(format!(
+ "{days} {hours}:{mins}:{secs}.{millis:03}"
+ )),
+ )),
+ leading_field: Some(ast::DateTimeField::Day),
+ leading_precision: None,
+ last_field: Some(ast::DateTimeField::Second),
+ fractional_seconds_precision: None,
+ };
+ Ok(ast::Expr::Interval(interval))
+ } else {
+ not_impl_err!("Unsupported IntervalMonthDayNano scalar
with both Month and DayTime for IntervalStyle::SQLStandard")
+ }
+ }
+ _ => Ok(ast::Expr::Value(ast::Value::Null)),
+ },
+ IntervalStyle::MySQL => {
+ not_impl_err!("Unsupported interval scalar for
IntervalStyle::MySQL")
Review Comment:
I see this PR doesn't implement MySQL style -- I wonder if it would be
better to simply not add the enum (and thus make it clear with the Rust
compiler that MySQL style isn't supported) rather than adding something in the
code that looks like it is supported but generates a runtime error
Or do you perhaps plan to implement the Mysql style shortly?
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
