Skip to content

Commit d9050b6

Browse files
committed
add priority_map test covering "null id and value"
1 parent 347c2ba commit d9050b6

2 files changed

Lines changed: 29 additions & 3 deletions

File tree

‎datafusion/physical-plan/src/aggregates/topk/hash_table.rs‎

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -36,11 +36,12 @@ use std::fmt::Debug;
3636
use std::hash::BuildHasher;
3737
use std::sync::Arc;
3838

39-
/// `heap_idx` assigned to groups whose aggregate values are all NULL. Such
40-
/// groups are tracked in the hash table only (they never enter the heap), so
41-
/// they can be emitted with a NULL aggregate value at the end.
39+
/// `heap_idx` assigned to groups whose aggregate values are all NULL.
40+
/// Such groups are tracked in the hash table only, without entering heap,
41+
/// so they can be emitted with a NULL aggregate value at the end.
4242
const NULL_HEAP_IDX: usize = usize::MAX - 1;
4343

44+
/// `heap_idx` needs to distinguish vacant slots from items that have a NULL key and value.
4445
const VACANT_HEAP_IDX: usize = usize::MAX;
4546

4647
/// An entry in our hash table that:

‎datafusion/physical-plan/src/aggregates/topk/priority_map.rs‎

Lines changed: 25 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -615,6 +615,31 @@ mod tests {
615615
Ok(())
616616
}
617617

618+
#[test]
619+
fn should_emit_null_id_with_all_null_values() -> Result<()> {
620+
let ids: ArrayRef = Arc::new(StringArray::from(vec![None, Some("1")]));
621+
let vals: ArrayRef = Arc::new(Int64Array::from(vec![None, Some(5)]));
622+
let mut agg = PriorityMap::new(DataType::Utf8, DataType::Int64, 2, true)?;
623+
agg.set_batch(ids, vals);
624+
agg.insert_null(0);
625+
agg.insert_with_null_groups(1)?;
626+
627+
let cols = agg.emit()?;
628+
let batch = RecordBatch::try_new(test_schema(), cols)?;
629+
let actual = format!("{}", pretty_format_batches(&[batch])?);
630+
assert_snapshot!(actual, @r"
631+
+----------+--------------+
632+
| trace_id | timestamp_ms |
633+
+----------+--------------+
634+
| 1 | 5 |
635+
| | |
636+
+----------+--------------+
637+
"
638+
);
639+
640+
Ok(())
641+
}
642+
618643
#[test]
619644
fn should_cap_null_groups_at_limit() -> Result<()> {
620645
let ids: ArrayRef = Arc::new(StringArray::from(vec!["1", "2", "3", "4", "5"]));

0 commit comments

Comments
 (0)