edubraqd commented on code in PR #24916:
URL: https://github.com/apache/datafusion/pull/24916#discussion_r4036300691
##########
datafusion/optimizer/src/analyzer/type_coercion.rs:
##########
@@ -1099,7 +1099,9 @@ fn coerce_frame_bound(
}
}
-fn extract_window_frame_target_type(col_type: &DataType) -> Result<DataType> {
+/// The type that RANGE frame offsets are coerced to for an ORDER BY column of
+/// `col_type`, or `None` if the type does not support RANGE frames.
Review Comment:
Agreed, that is a real mismatch and the type check could not see it. Done in
d96eccbc7: lists are rejected in the fallback.
Concretely, `compare_rows` applies `nulls_first` to the top-level value only
and then calls `ScalarValue::partial_cmp`; for lists that ends in
`partial_cmp_list`, which always orders a NULL element after a non-NULL one.
The sorter's `make_comparator` applies the same option to the elements. So with
tied `d` and `ORDER BY d, l NULLS FIRST`, the sort puts `[NULL]` before `[1]`
and the peer check orders them the other way round.
`is_list_of_uncomparable` / `list_element_comparable` and their unit test
are gone; `is_list_type` (list behind Dictionary / RunEndEncoded included)
replaces them, with the reason in its doc comment. `window.slt` now has the
regression case you asked for (tied durations, `[NULL]` / `[1]`, `NULLS FIRST`,
planning error) plus a NULL-free list and a dictionary-encoded list, to show
the check is on the type.
The list cases with a list as the *first* ORDER BY key are unchanged: those
never enter the fallback and behave as on `main`. Net diff is smaller than
before (-70 lines).
--
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]
