Skip to content

Commit a6ccd6a

Browse files
committed
fix(iox_query_influxql): correct SPREAD when all values are negative
`SpreadAccumulator` initialised `max` to zero, so all-negative inputs produced `0 - min` instead of `max - min` (all -3.6 returned 3.6 instead of 0). Initialise `max` to NULL and check `is_null()` in `maybe_set_max`, mirroring the `min` path. Closes #27384
1 parent db092e1 commit a6ccd6a

1 file changed

Lines changed: 41 additions & 2 deletions

File tree

  • core/iox_query_influxql/src/aggregate

core/iox_query_influxql/src/aggregate/spread.rs

Lines changed: 41 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -86,7 +86,7 @@ struct SpreadAccumulator {
8686
impl SpreadAccumulator {
8787
fn new(data_type: DataType) -> Result<Self> {
8888
let min = ScalarValue::try_from(&data_type)?;
89-
let max = ScalarValue::new_zero(&data_type)?;
89+
let max = ScalarValue::try_from(&data_type)?;
9090
Ok(Self {
9191
data_type,
9292
min,
@@ -122,7 +122,7 @@ impl SpreadAccumulator {
122122
}
123123

124124
fn maybe_set_max(&mut self, v: ScalarValue) {
125-
if v > self.max {
125+
if self.max.is_null() || v > self.max {
126126
self.max = v
127127
}
128128
}
@@ -161,3 +161,42 @@ impl Accumulator for SpreadAccumulator {
161161
Ok(())
162162
}
163163
}
164+
165+
#[cfg(test)]
166+
mod tests {
167+
use super::*;
168+
use arrow::array::Float64Array;
169+
170+
fn spread_of(values: &[f64]) -> ScalarValue {
171+
let mut acc = SpreadAccumulator::new(DataType::Float64).unwrap();
172+
let array: ArrayRef = Arc::new(Float64Array::from_iter_values(values.iter().copied()));
173+
acc.update_batch(&[array]).unwrap();
174+
acc.evaluate().unwrap()
175+
}
176+
177+
#[test]
178+
fn all_negative_values_yield_zero_spread() {
179+
// Regression test for https://github.com/influxdata/influxdb/issues/27384:
180+
// when every observed value is negative, the initial `max` of 0 was never
181+
// overwritten, so spread evaluated to `0 - min` instead of `max - min`.
182+
assert_eq!(
183+
spread_of(&[-3.6, -3.6, -3.6]),
184+
ScalarValue::Float64(Some(0.0))
185+
);
186+
assert_eq!(spread_of(&[-5.0, -1.0]), ScalarValue::Float64(Some(4.0)));
187+
}
188+
189+
#[test]
190+
fn single_negative_value_yields_zero_spread() {
191+
assert_eq!(spread_of(&[-3.6]), ScalarValue::Float64(Some(0.0)));
192+
}
193+
194+
#[test]
195+
fn mixed_values_yield_correct_spread() {
196+
assert_eq!(
197+
spread_of(&[-2.0, 0.0, 3.0]),
198+
ScalarValue::Float64(Some(5.0))
199+
);
200+
assert_eq!(spread_of(&[1.0, 2.0, 3.0]), ScalarValue::Float64(Some(2.0)));
201+
}
202+
}

0 commit comments

Comments
 (0)