namanjain24-sudo opened a new pull request, #25498:
URL: https://github.com/apache/datafusion/pull/25498
## Which issue does this PR close?
- Closes #25432.
## Rationale for this change
`process_scalar` always went through `general_date_trunc`, which converts
the value to nanoseconds before truncating. A `Timestamp(Second)`,
`Timestamp(Millisecond)` or `Timestamp(Microsecond)` value outside the
nanosecond range (after 2262 or before 1677) therefore failed with `Timestamp
... out of range`.
`process_array` does not convert for the fine granularities: when
`granularity.is_fine_granularity()`, or the value has no timezone and the
granularity is `hour` or `day`, it truncates in the input's own unit with plain
arithmetic. So the same value succeeded in a column and failed as a scalar:
```sql
SELECT date_trunc('second', to_timestamp_seconds(10000000000));
-- Execution error: Timestamp 10000000000 out of range
SELECT date_trunc('second', to_timestamp_seconds(ts)) FROM (VALUES
(10000000000)) AS t(ts);
-- 2286-11-20T17:46:40
```
## What changes are included in this PR?
The scalar path now takes the same route as the array path:
- `truncates_in_input_unit` is the condition `process_array` already used,
now shared by both paths.
- `fine_granularity_unit` is the unit table from
`general_date_trunc_array_fine_granularity`, moved out so both paths read it.
- `date_trunc_fine_granularity` truncates one value in its own unit, with
the checked subtraction and error message the array path already used for
values near `i64::MIN`. The array path's slow branch now calls it too.
For those granularities, a scalar and a one-row column now run the same
function, so they accept the same values and give the same result. Everything
else (`week`, `month`, `quarter`, `year`, and `hour`/`day` with a timezone)
still goes through `general_date_trunc` on both paths, so it is unchanged and
still limited to the nanosecond range for both scalars and columns.
One existing test changes. `date_trunc('hour',
arrow_cast(9223372036854775807, 'Timestamp(Second, None)'))` was expected to
fail with `Timestamp 9223372036854775807 out of range`. That test comes from
#22262, whose goal was an error instead of a panic, and a column holding that
value already succeeds. As a scalar it now succeeds too, so the test uses
`week`, which still converts to nanoseconds on both paths and so still covers
the error.
## Are these changes tested?
- New unit test `scalar_and_array_accept_timestamps_beyond_nanosecond_range`
evaluates `date_trunc` on a scalar and on a one-row column with the same value,
for all three coarser-than-nanosecond units, with and without a timezone, and
for values before 1677 and after 2262, and checks that both succeed with the
same result.
- New sqllogictest queries in `datetime/timestamps.slt` cover the issue's
scalar and column queries, plus millisecond and microsecond scalars.
- With the scalar change disabled, the new unit test fails, and the three
new scalar queries fail with the error from the issue (`Timestamp 10000000000
out of range`, etc.), while the column query passes as it does on `main`.
- `cargo test -p datafusion-functions` (384 passed), the full sqllogictest
suite (520 files), `cargo fmt` and `cargo clippy -p datafusion-functions
--all-targets --all-features -- -D warnings` pass.
## Are there any user-facing changes?
`date_trunc` on a scalar timestamp outside the nanosecond range now returns
the same result as on a column, instead of an out-of-range error, for
`microsecond`, `millisecond`, `second` and `minute`, and for `hour` and `day`
without a timezone. No API changes.
--
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]
