alamb commented on code in PR #5806:
URL: https://github.com/apache/arrow-datafusion/pull/5806#discussion_r1154771201
##########
datafusion/core/tests/sql/expr.rs:
##########
@@ -843,153 +843,153 @@ async fn test_interval_expressions() -> Result<()> {
// day nano intervals
test_expression!(
"interval '1'",
- "0 years 0 mons 0 days 0 hours 0 mins 1.000 secs"
+ "0 years 0 mons 0 days 0 hours 0 mins 1.000000000 secs"
);
test_expression!(
"interval '1 second'",
- "0 years 0 mons 0 days 0 hours 0 mins 1.000 secs"
+ "0 years 0 mons 0 days 0 hours 0 mins 1.000000000 secs"
);
test_expression!(
"interval '500 milliseconds'",
- "0 years 0 mons 0 days 0 hours 0 mins 0.500 secs"
+ "0 years 0 mons 0 days 0 hours 0 mins 0.500000000 secs"
);
test_expression!(
"interval '5 second'",
- "0 years 0 mons 0 days 0 hours 0 mins 5.000 secs"
+ "0 years 0 mons 0 days 0 hours 0 mins 5.000000000 secs"
);
test_expression!(
"interval '0.5 minute'",
- "0 years 0 mons 0 days 0 hours 0 mins 30.000 secs"
+ "0 years 0 mons 0 days 0 hours 0 mins 30.000000000 secs"
);
test_expression!(
"interval '.5 minute'",
- "0 years 0 mons 0 days 0 hours 0 mins 30.000 secs"
+ "0 years 0 mons 0 days 0 hours 0 mins 30.000000000 secs"
);
test_expression!(
"interval '5 minute'",
- "0 years 0 mons 0 days 0 hours 5 mins 0.000 secs"
+ "0 years 0 mons 0 days 0 hours 5 mins 0.000000000 secs"
);
test_expression!(
"interval '5 minute 1 second'",
- "0 years 0 mons 0 days 0 hours 5 mins 1.000 secs"
+ "0 years 0 mons 0 days 0 hours 5 mins 1.000000000 secs"
);
test_expression!(
"interval '1 hour'",
- "0 years 0 mons 0 days 1 hours 0 mins 0.000 secs"
+ "0 years 0 mons 0 days 1 hours 0 mins 0.000000000 secs"
);
test_expression!(
"interval '5 hour'",
- "0 years 0 mons 0 days 5 hours 0 mins 0.000 secs"
+ "0 years 0 mons 0 days 5 hours 0 mins 0.000000000 secs"
);
test_expression!(
"interval '1 day'",
- "0 years 0 mons 1 days 0 hours 0 mins 0.000 secs"
+ "0 years 0 mons 1 days 0 hours 0 mins 0.000000000 secs"
);
test_expression!(
"interval '1 week'",
- "0 years 0 mons 7 days 0 hours 0 mins 0.000 secs"
+ "0 years 0 mons 7 days 0 hours 0 mins 0.000000000 secs"
);
test_expression!(
"interval '2 weeks'",
- "0 years 0 mons 14 days 0 hours 0 mins 0.000 secs"
+ "0 years 0 mons 14 days 0 hours 0 mins 0.000000000 secs"
);
test_expression!(
"interval '1 day 1'",
- "0 years 0 mons 1 days 0 hours 0 mins 1.000 secs"
+ "0 years 0 mons 1 days 0 hours 0 mins 1.000000000 secs"
);
test_expression!(
"interval '0.5'",
- "0 years 0 mons 0 days 0 hours 0 mins 0.500 secs"
+ "0 years 0 mons 0 days 0 hours 0 mins 0.500000000 secs"
);
test_expression!(
"interval '0.5 day 1'",
- "0 years 0 mons 0 days 12 hours 0 mins 1.000 secs"
+ "0 years 0 mons 0 days 12 hours 0 mins 1.000000000 secs"
);
test_expression!(
"interval '0.49 day'",
- "0 years 0 mons 0 days 11 hours 45 mins 36.000 secs"
+ "0 years 0 mons 0 days 11 hours 45 mins 36.000000000 secs"
);
test_expression!(
"interval '0.499 day'",
- "0 years 0 mons 0 days 11 hours 58 mins 33.600 secs"
+ "0 years 0 mons 0 days 11 hours 58 mins 33.600000000 secs"
);
test_expression!(
"interval '0.4999 day'",
- "0 years 0 mons 0 days 11 hours 59 mins 51.360 secs"
+ "0 years 0 mons 0 days 11 hours 59 mins 51.360000000 secs"
);
test_expression!(
"interval '0.49999 day'",
- "0 years 0 mons 0 days 11 hours 59 mins 59.136 secs"
+ "0 years 0 mons 0 days 11 hours 59 mins 59.136000000 secs"
);
test_expression!(
"interval '0.49999999999 day'",
"0 years 0 mons 0 days 11 hours 59 mins 59.999999136 secs"
);
test_expression!(
"interval '5 day'",
- "0 years 0 mons 5 days 0 hours 0 mins 0.000 secs"
+ "0 years 0 mons 5 days 0 hours 0 mins 0.000000000 secs"
);
// Hour is ignored, this matches PostgreSQL
test_expression!(
"interval '5 day' hour",
- "0 years 0 mons 5 days 0 hours 0 mins 0.000 secs"
+ "0 years 0 mons 5 days 0 hours 0 mins 0.000000000 secs"
);
test_expression!(
"interval '5 day 4 hours 3 minutes 2 seconds 100 milliseconds'",
- "0 years 0 mons 5 days 4 hours 3 mins 2.100 secs"
+ "0 years 0 mons 5 days 4 hours 3 mins 2.100000000 secs"
);
// month intervals
test_expression!(
"interval '0.5 month'",
- "0 years 0 mons 15 days 0 hours 0 mins 0.000 secs"
+ "0 years 0 mons 15 days 0 hours 0 mins 0.000000000 secs"
);
test_expression!(
"interval '0.5' month",
- "0 years 0 mons 15 days 0 hours 0 mins 0.000 secs"
+ "0 years 0 mons 15 days 0 hours 0 mins 0.000000000 secs"
);
test_expression!(
"interval '1 month'",
- "0 years 1 mons 0 days 0 hours 0 mins 0.00 secs"
+ "0 years 1 mons 0 days 0 hours 0 mins 0.000000000 secs"
);
test_expression!(
"interval '1' MONTH",
- "0 years 1 mons 0 days 0 hours 0 mins 0.00 secs"
+ "0 years 1 mons 0 days 0 hours 0 mins 0.000000000 secs"
);
test_expression!(
"interval '5 month'",
- "0 years 5 mons 0 days 0 hours 0 mins 0.00 secs"
+ "0 years 5 mons 0 days 0 hours 0 mins 0.000000000 secs"
);
test_expression!(
"interval '13 month'",
- "1 years 1 mons 0 days 0 hours 0 mins 0.00 secs"
+ "0 years 13 mons 0 days 0 hours 0 mins 0.000000000 secs"
Review Comment:
I am not sure this matters -- but `interval` parsing now is `month/day/nano`
always and thus is not quite as nice a display as it was previously (no years,
instead it is always months). However perhaps if anyone cares we can improve
the display in arrow-rs for this kind of interval
##########
datafusion/core/tests/sql/timestamp.rs:
##########
@@ -968,52 +968,52 @@ async fn timestamp_array_add_interval() -> Result<()> {
let sql = "SELECT ts, ts - INTERVAL '8' MILLISECONDS FROM table_a";
let actual = execute_to_batches(&ctx, sql).await;
let expected = vec![
- "+----------------------------+-----------------------------------+",
- "| ts | table_a.ts - IntervalDayTime(\"8\") |",
- "+----------------------------+-----------------------------------+",
- "| 2020-09-08T13:42:29.190855 | 2020-09-08T13:42:29.182855 |",
- "| 2020-09-08T12:42:29.190855 | 2020-09-08T12:42:29.182855 |",
- "| 2020-09-08T11:42:29.190855 | 2020-09-08T11:42:29.182855 |",
- "+----------------------------+-----------------------------------+",
+
"+----------------------------+----------------------------------------------+",
+ "| ts | table_a.ts -
IntervalMonthDayNano(\"8000000\") |",
+
"+----------------------------+----------------------------------------------+",
+ "| 2020-09-08T13:42:29.190855 | 2020-09-08T13:42:29.182855
|",
+ "| 2020-09-08T12:42:29.190855 | 2020-09-08T12:42:29.182855
|",
+ "| 2020-09-08T11:42:29.190855 | 2020-09-08T11:42:29.182855
|",
+
"+----------------------------+----------------------------------------------+",
];
assert_batches_eq!(expected, &actual);
let sql = "SELECT ts, ts + INTERVAL '1' SECOND FROM table_b";
let actual = execute_to_batches(&ctx, sql).await;
let expected = vec![
-
"+----------------------------+--------------------------------------+",
- "| ts | table_b.ts + IntervalDayTime(\"1000\")
|",
-
"+----------------------------+--------------------------------------+",
- "| 2020-09-08T13:42:29.190855 | 2020-09-08T13:42:30.190855
|",
- "| 2020-09-08T12:42:29.190855 | 2020-09-08T12:42:30.190855
|",
- "| 2020-09-08T11:42:29.190855 | 2020-09-08T11:42:30.190855
|",
-
"+----------------------------+--------------------------------------+",
+
"+----------------------------+-------------------------------------------------+",
+ "| ts | table_b.ts +
IntervalMonthDayNano(\"1000000000\") |",
Review Comment:
This is pretty terrible formatting for the column name 🤮 but I don't think
it is fixable easily -- see #2027 for more discussion
##########
datafusion/sql/src/expr/value.rs:
##########
@@ -202,11 +203,55 @@ impl<'a, S: ContextProvider> SqlToRel<'a, S> {
}
};
- let leading_field = leading_field
- .as_ref()
- .map(|dt| dt.to_string())
- .unwrap_or_else(|| "second".to_string());
+ let value = if has_units(&value) {
Review Comment:
This is a pretty terrible hack but it seems to be necessary to get the tests
to pass (aka to have postgres compatible syntax) 🤷
--
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]
