viirya opened a new issue, #25812: URL: https://github.com/apache/datafusion/issues/25812
### Is your feature request related to a problem or challenge? #25692 (fixing #25690) made `date_bin` report `Unordered` output when its source argument is `Timestamp(Second | Millisecond | Microsecond, _)`, `Time64(Microsecond)`, or has an unknown (`DataType::Null`) range type. The reason is that `date_bin` scales these values to nanoseconds, and a per-row scaling overflow becomes `NULL`, so sorted input can turn into `[NULL, value, NULL]`. As discussed in https://github.com/apache/datafusion/pull/25692#discussion_r4103331668, this forces extra work for common queries over data that never gets near the overflow range: - `ORDER BY date_bin(..., time)` over data sorted by `time` re-sorts the whole input. - `GROUP BY date_bin(..., time)` over sorted input can no longer use sorted/streaming aggregation. Affected today: - Direct s/ms/us timestamp and `Time64(Microsecond)` columns. - Nested expressions whose range type is unknown, including nanosecond ones, e.g. `date_bin(i, date_trunc('hour', ts))`. #25668 would give these a typed range, which recovers the nanosecond case. Not affected: `date_bin(i, col)` where `col` is `Timestamp(Nanosecond, _)`. The guard is also incomplete in the other direction. Nanosecond inputs can still produce per-row `NULL`s in the binning step, for example `compute_distance` computes `time_delta - stride`, which overflows for `time_diff` near `i64::MIN`; `source - origin` can overflow with an explicit origin; and month strides can go out of range. So the current rule adds sorts for coarse precisions without fully guaranteeing NULL placement for nanoseconds. ### Describe the solution you'd like Stop producing `NULL` for valid in-range source values, so that `date_bin` is monotone over non-null input for every unit, and restore ordering propagation for all timestamp/time representations: - For s/ms/us sources, compute the bin in wider arithmetic (i128) or directly in the source precision, instead of scaling to nanoseconds first and mapping overflow to `NULL`. - Decide on the remaining edge cases (bins that fall outside the representable range near `i64::MIN`/`i64::MAX`, explicit origins, month strides) consistently for all units. They need either an explicit ordering argument (e.g. only the extreme bins can fail, so `NULL`s cluster at one end) or proven input bounds. ### Describe alternatives you've considered - Only give up ordering when the input range is proven to possibly overflow. Column ranges are currently unbounded in `ExprProperties`, so this needs bounds propagation from statistics first. - A config flag to disable the guard. A flag that trades correctness for performance is hard to reason about, so I'd prefer fixing the arithmetic. - Reverting the guard from #25692 until the arithmetic fix lands. ### Additional context - #25690, #25692: original bug and conservative fix. - #25668: typed ranges for scalar UDFs with default `evaluate_bounds`. - #25575: typed `date_bin` bounds, which made the original bug visible through `date_trunc`. -- 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]
