Repository navigation
date_trunc(null) results in a panic #6701
Description
Describe the bug
Calling the date_trunc(null) function results in a panic
To Reproduce
DataFusion CLI v26.0.0
❯ select date_trunc('2021-02-01T12:01:02', 'hour');
Optimizer rule 'simplify_expressions' failed
caused by
Arrow error: Parser error: Error parsing timestamp from 'hour': timestamp must contain at least 10 characters
❯ select date_trunc('hour', '2021-02-01T12:01:02');
+------------------------------------------------------+
| date_trunc(Utf8("hour"),Utf8("2021-02-01T12:01:02")) |
+------------------------------------------------------+
| 2021-02-01T12:00:00 |
+------------------------------------------------------+
1 row in set. Query took 0.074 seconds.
❯ select date_trunc('hour', null);
thread 'main' panicked at 'called `Option::unwrap()` on a `None` value', /Users/alamb/Software/arrow-datafusion/datafusion/physical-expr/src/datetime_expressions.rs:314:35
note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace
Expected behavior
null should be returned
Additional context
Found while reviewing #6654 from @Weijun-H
The issue is the use of unwrap() in the date_trunc implementation
I think this is a good first issue for someone as it is localized (involves removing unwrap()) and writing some tests using .slt
Reacted by Alex Huang
Activity
BryanEmond commentedon Jun 16, 2023
I'll look at it.
BryanEmond commentedon Jun 16, 2023
I've tried to remove the unwrap() like specify in the issue, but nano is an Option, so removing the unwrap just creates errors. So is it better to catch the error and return null or replacing unwrap() by unwrap_or_default() which return "1970-01-01T00:00:00"?
I got this timestamp by running this test in timestamps.slt
query P
SELECT DATE_TRUNC('minute', TIMESTAMP '2022-08-03 14:38:50Z');
----2022-01-01T00:00:00
error:
[SQL] SELECT DATE_TRUNC('minute', NULL);
[Diff] (-expected|+actual)
at tests/sqllogictests/test_files/timestamps.slt:910
alamb commentedon Jun 18, 2023
I think what is needed is to ignore nulls (so that null comes out)
something like
nano.map(|nano| truncate(nano))So rather than
unwrapusemapto only do the calculation ofnanois non nullIf you make a PR with some example code I can probably offer some more specific advice