Skip to content

Commit fc8953a

Browse files
committed
Aggregating bounded min/max values has to not ignore null values
Signed-off-by: Robert Kruszewski <github@robertk.io>
1 parent 35e4d72 commit fc8953a

2 files changed

Lines changed: 117 additions & 2 deletions

File tree

vortex-file/src/tests.rs

Lines changed: 97 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1986,3 +1986,100 @@ async fn test_can_prune_composite_predicates() -> VortexResult<()> {
19861986

19871987
Ok(())
19881988
}
1989+
1990+
// Regression test for https://github.com/vortex-data/vortex/issues/8166.
1991+
//
1992+
// A binary column whose per-zone max value has an all-0xff prefix (longer than the
1993+
// statistics-truncation length) cannot be upper-bounded, so the max stat is written as NULL.
1994+
// Filtering with `col > literal` must still return the row, but a pruning bug drops it.
1995+
#[tokio::test]
1996+
#[cfg_attr(miri, ignore)]
1997+
async fn repro_8166_binary_gt_all_ff_max() -> VortexResult<()> {
1998+
use vortex_buffer::ByteBuffer;
1999+
let mut ctx = SESSION.create_execution_ctx();
2000+
2001+
let empty: Vec<u8> = vec![];
2002+
let chunk0: Vec<Vec<u8>> = vec![
2003+
vec![0x1d, 0x00],
2004+
empty.clone(),
2005+
vec![0x1d, 0x10, 0x9d, 0x08],
2006+
empty.clone(),
2007+
empty.clone(),
2008+
empty.clone(),
2009+
empty.clone(),
2010+
empty.clone(),
2011+
empty.clone(),
2012+
];
2013+
let chunk1: Vec<Vec<u8>> = vec![
2014+
empty.clone(),
2015+
empty.clone(),
2016+
vec![0x40],
2017+
empty.clone(),
2018+
empty.clone(),
2019+
empty.clone(),
2020+
empty.clone(),
2021+
empty.clone(),
2022+
vec![0x24],
2023+
vec![0x43, 0xff],
2024+
];
2025+
// Row 27 (local index 8 in chunk2): 112-byte value, all 0xff except a 0x03 at byte 89.
2026+
let mut big = vec![0xffu8; 112];
2027+
big[89] = 0x03;
2028+
let mut chunk2: Vec<Vec<u8>> = vec![empty.clone(); 10];
2029+
chunk2[8] = big;
2030+
2031+
let bin = DType::Binary(Nullability::NonNullable);
2032+
// Chunk at the struct level: the zoned writer maps one input chunk to one zone, so this
2033+
// produces 3 zones. Zones 0/1 have small, valid maxes (< literal, pruned); zone 2's max is
2034+
// the all-0xff value, whose upper bound overflows and is written as a NULL max stat.
2035+
let mk_struct = |vals: Vec<Vec<u8>>| -> VortexResult<ArrayRef> {
2036+
let yyw = VarBinArray::from_vec(vals, bin.clone()).into_array();
2037+
Ok(StructArray::from_fields(&[("yyw", yyw)])?.into_array())
2038+
};
2039+
let array =
2040+
ChunkedArray::from_iter([mk_struct(chunk0)?, mk_struct(chunk1)?, mk_struct(chunk2)?])
2041+
.into_array();
2042+
2043+
let mut buf = ByteBufferMut::empty();
2044+
SESSION
2045+
.write_options()
2046+
.write(&mut buf, array.to_array_stream())
2047+
.await
2048+
.unwrap();
2049+
2050+
// literal = 0x6f x5 + 0xff x57 + 0x98 (63 bytes). Only row 27 is > literal.
2051+
let mut literal = vec![0x6fu8; 5];
2052+
literal.extend(iter::repeat_n(0xffu8, 57));
2053+
literal.push(0x98);
2054+
assert_eq!(literal.len(), 63);
2055+
2056+
let filter = gt(
2057+
get_item("yyw", root()),
2058+
lit(Scalar::binary(
2059+
ByteBuffer::from(literal),
2060+
Nullability::NonNullable,
2061+
)),
2062+
);
2063+
2064+
let result = SESSION
2065+
.open_options()
2066+
.open_buffer(buf)
2067+
.unwrap()
2068+
.scan()
2069+
.unwrap()
2070+
.with_filter(filter)
2071+
.into_array_stream()
2072+
.unwrap()
2073+
.read_all()
2074+
.await
2075+
.unwrap()
2076+
.execute::<StructArray>(&mut ctx)
2077+
.unwrap();
2078+
2079+
assert_eq!(
2080+
result.len(),
2081+
1,
2082+
"expected exactly row 27 to match the filter"
2083+
);
2084+
Ok(())
2085+
}

vortex-layout/src/layouts/zoned/builder.rs

Lines changed: 20 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -147,8 +147,26 @@ impl StatsAccumulator {
147147

148148
// Different stats need different aggregations
149149
match stat {
150-
// For stats that are associative, we can just compute them over the stat column
151-
Stat::Min | Stat::Max | Stat::Sum => {
150+
// Min/Max are associative, so we aggregate them over the per-chunk stat column.
151+
// For variable-width (utf8/binary) types these are *bounded* aggregates: a chunk's
152+
// bound is null when its value could not be represented within
153+
// `max_variable_length_statistics_size` (an all-0xFF prefix for Max), i.e. the
154+
// bound is unknown. A plain Min/Max aggregate would silently skip that null and
155+
// emit a bound derived from the remaining chunks, under-estimating Max /
156+
// over-estimating Min and pruning rows that actually match
157+
// (https://github.com/vortex-data/vortex/issues/8166). Treat a null bound as
158+
// "no aggregate stat" for these types instead.
159+
Stat::Min | Stat::Max => {
160+
let bound_unknown = matches!(array.dtype(), DType::Utf8(_) | DType::Binary(_))
161+
&& !array.all_valid(ctx)?;
162+
if !bound_unknown
163+
&& let Some(s) = array.statistics().compute_stat(stat, ctx)?
164+
&& let Some(v) = s.into_value()
165+
{
166+
stats_set.set(stat, Precision::exact(v))
167+
}
168+
}
169+
Stat::Sum => {
152170
if let Some(s) = array.statistics().compute_stat(stat, ctx)?
153171
&& let Some(v) = s.into_value()
154172
{

0 commit comments

Comments
 (0)