goutamadwant commented on code in PR #25816:
URL: https://github.com/apache/datafusion/pull/25816#discussion_r4372281199
##########
datafusion/optimizer/src/analyzer/type_coercion.rs:
##########
@@ -1290,6 +1290,54 @@ fn coerce_frame_bound(
}
}
+/// Reject negative RANGE offsets. Numeric strings are checked only after a
+/// successful numeric coercion, before rounding can hide their original sign.
+/// Interval ordering alone is insufficient here:
+/// a positive month can mask a negative day in a lexicographic comparison.
+fn validate_range_frame_bound(bound: &WindowFrameBound) -> Result<()> {
+ let offset = match bound {
+ WindowFrameBound::Preceding(offset) |
WindowFrameBound::Following(offset)
+ if !offset.is_null() =>
+ {
+ offset
+ }
+ _ => return Ok(()),
+ };
+
+ let negative = match offset {
+ ScalarValue::Utf8(Some(value)) => value
+ .trim_start()
+ .strip_prefix('-')
+ .is_some_and(|magnitude| {
+ // Ignore exponent digits: -0e10 is zero, but -1e-1000 is
+ // negative even if floating-point coercion underflows to -0.
+ magnitude
+ .split(['e', 'E'])
+ .next()
+ .unwrap_or_default()
+ .bytes()
+ .any(|digit| matches!(digit, b'1'..=b'9'))
+ }),
+ ScalarValue::IntervalYearMonth(Some(months)) => *months < 0,
+ ScalarValue::IntervalDayTime(Some(interval)) => {
+ interval.days < 0 || interval.milliseconds < 0
+ }
+ ScalarValue::IntervalMonthDayNano(Some(interval)) => {
+ interval.months < 0 || interval.days < 0 || interval.nanoseconds < 0
+ }
+ ScalarValue::Float16(Some(value)) => value.to_f32() < 0.0,
+ ScalarValue::Float32(Some(value)) => *value < 0.0,
+ ScalarValue::Float64(Some(value)) => *value < 0.0,
+ _ => offset < &ScalarValue::new_zero(&offset.data_type())?,
+ };
+
+ if negative {
Review Comment:
sure @kosiew so the error now includes the offending bound, including
PRECEDING/FOLLOWING. The regression assertions cover both directions and
mixed-sign intervals. let me know if this looks good now.
--
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]
