Skip to content

Commit 3d75bb8

Browse files
authored
Update variance/stddev to work with single values (#4847)
* Update variance/stddev to work with single values Following Postgres: - var/stddev of single element is NULL - var_pop/stddev_pop of single element is 0 * Fix tests * matches! to if let * fix test_stddev_1_input test
1 parent dcd52ee commit 3d75bb8

3 files changed

Lines changed: 31 additions & 20 deletions

File tree

datafusion/core/tests/sqllogictests/test_files/aggregate.slt

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1091,3 +1091,15 @@ query U
10911091
SELECT ARRAY_AGG([1]);
10921092
----
10931093
[[1]]
1094+
1095+
# variance_single_value
1096+
query RRRR
1097+
select var(sq.column1), var_pop(sq.column1), stddev(sq.column1), stddev_pop(sq.column1) from (values (1.0)) as sq;
1098+
----
1099+
NULL 0 NULL 0
1100+
1101+
# variance_two_values
1102+
query RRRR
1103+
select var(sq.column1), var_pop(sq.column1), stddev(sq.column1), stddev_pop(sq.column1) from (values (1.0), (3.0)) as sq;
1104+
----
1105+
2 1 1.4142135623730951 1

datafusion/physical-expr/src/aggregate/stddev.rs

Lines changed: 4 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -307,8 +307,8 @@ mod tests {
307307
"bla".to_string(),
308308
DataType::Float64,
309309
));
310-
let actual = aggregate(&batch, agg);
311-
assert!(actual.is_err());
310+
let actual = aggregate(&batch, agg).unwrap();
311+
assert_eq!(actual, ScalarValue::Float64(None));
312312

313313
Ok(())
314314
}
@@ -341,9 +341,8 @@ mod tests {
341341
"bla".to_string(),
342342
DataType::Float64,
343343
));
344-
let actual = aggregate(&batch, agg);
345-
assert!(actual.is_err());
346-
344+
let actual = aggregate(&batch, agg).unwrap();
345+
assert_eq!(actual, ScalarValue::Float64(None));
347346
Ok(())
348347
}
349348

datafusion/physical-expr/src/aggregate/variance.rs

Lines changed: 15 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -286,17 +286,17 @@ impl Accumulator for VarianceAccumulator {
286286
}
287287
};
288288

289-
if count <= 1 {
290-
return Err(DataFusionError::Internal(
291-
"At least two values are needed to calculate variance".to_string(),
292-
));
293-
}
294-
295-
if self.count == 0 {
296-
Ok(ScalarValue::Float64(None))
297-
} else {
298-
Ok(ScalarValue::Float64(Some(self.m2 / count as f64)))
299-
}
289+
Ok(ScalarValue::Float64(match self.count {
290+
0 => None,
291+
1 => {
292+
if let StatsType::Population = self.stats_type {
293+
Some(0.0)
294+
} else {
295+
None
296+
}
297+
}
298+
_ => Some(self.m2 / count as f64),
299+
}))
300300
}
301301

302302
fn size(&self) -> usize {
@@ -382,8 +382,8 @@ mod tests {
382382
"bla".to_string(),
383383
DataType::Float64,
384384
));
385-
let actual = aggregate(&batch, agg);
386-
assert!(actual.is_err());
385+
let actual = aggregate(&batch, agg).unwrap();
386+
assert_eq!(actual, ScalarValue::Float64(None));
387387

388388
Ok(())
389389
}
@@ -416,8 +416,8 @@ mod tests {
416416
"bla".to_string(),
417417
DataType::Float64,
418418
));
419-
let actual = aggregate(&batch, agg);
420-
assert!(actual.is_err());
419+
let actual = aggregate(&batch, agg).unwrap();
420+
assert_eq!(actual, ScalarValue::Float64(None));
421421

422422
Ok(())
423423
}

0 commit comments

Comments
 (0)