kumarUjjawal commented on code in PR #24916:
URL: https://github.com/apache/datafusion/pull/24916#discussion_r3930644972
##########
datafusion/optimizer/src/analyzer/type_coercion.rs:
##########
@@ -1110,7 +1130,19 @@ fn coerce_window_frame(
.map(|s| s.expr.get_type(schema))
.transpose()?;
if let Some(col_type) = current_types {
- let target_type = extract_window_frame_target_type(&col_type)?;
+ let target_type = match
extract_window_frame_target_type(&col_type) {
+ Ok(target_type) => target_type,
+ // A free range frame has no offsets to coerce, so an ORDER
+ // BY type without arithmetic is fine as long as its peer
+ // comparison is sound (see `supports_free_range_frame`).
+ Err(_)
+ if window_frame.free_range()
+ && supports_free_range_frame(&col_type) =>
Review Comment:
Free RANGE frames may contain multiple `ORDER BY` expressions, but this
check validates only the first expression’s type. For example, `ORDER BY
duration_col, map_col` is accepted because the first type is `Duration`, even
though the second `Map` key still has the peer-comparison mismatch discussed
earlier.
With equal duration values and maps that have the same key but different
values, sorting distinguishes the rows while the RANGE peer check treats them
as peers. `COUNT(*) OVER (...)` can therefore return `2, 2` instead of `1, 2`.
Could we validate every ORDER BY expression used for peer comparison, or
restrict this fallback to a single ORDER BY expression and add a regression
test?
--
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]
