This is an automated email from the ASF dual-hosted git repository. github-merge-queue[bot] pushed a commit to branch gh-readonly-queue/main/pr-11828-b858a1f951e5599ec639e9e27e0bd293fd304b6f in repository https://gitbox.apache.org/repos/asf/arrow-rs.git
commit 6038304119027e550189d3fb4de5e98d68431c63 Author: mightsleep <[email protected]> AuthorDate: Wed Oct 28 15:13:08 2026 +0000 fix(parquet): count levels, not rows, when marking null pages in the column index (#11828) # Which issue does this PR close? - Closes #11963 found while checking level histograms against the page index for nested columns. # Rationale for this change The writer marks a null page when `num_buffered_rows == num_page_nulls`, but `num_page_nulls` counts levels, not rows. For repeated columns this marks pages like `[null, 7]` per row as null pages with empty min/max, and drops the whole column index for pages like `[null, null]`. DataFusion does not prune nested columns yet, but pages always start at a record boundary, so pruning e.g. `array_has` on list elements is possible and would rely on `null_pages` being right. # What changes are included in this PR? Compare `num_page_nulls` with the number of levels (`num_buffered_values`). Non-repeated columns are unchanged. # Are these changes tested? Two new tests for both cases; both fail without the fix. # Are there any user-facing changes? No. Co-authored-by: Ed Seidl <[email protected]> --- parquet/src/column/writer/mod.rs | 48 +++++++++++++++++++++++++++++++++++++++- 1 file changed, 47 insertions(+), 1 deletion(-) diff --git a/parquet/src/column/writer/mod.rs b/parquet/src/column/writer/mod.rs index 923f0a4599..bab0a1b91e 100644 --- a/parquet/src/column/writer/mod.rs +++ b/parquet/src/column/writer/mod.rs @@ -1258,8 +1258,10 @@ impl<'a, E: ColumnValueEncoder> GenericColumnWriter<'a, E> { page_variable_length_bytes: Option<i64>, ) { // update the column index + // `num_page_nulls` counts levels, not rows, so compare it with the number of levels: + // with repetition a page can hold as many nulls as rows and still have values. let null_page = - (self.page_metrics.num_buffered_rows as u64) == self.page_metrics.num_page_nulls; + (self.page_metrics.num_buffered_values as u64) == self.page_metrics.num_page_nulls; // a page contains only null values, // and writers have to set the corresponding entries in min_values and max_values to byte[0] if null_page && self.column_index_builder.valid() { @@ -4354,6 +4356,50 @@ mod tests { assert_eq!(col_idx.definition_level_histogram(0).unwrap(), &[4, 0]); } + #[test] + fn test_column_index_repeated_page_with_values_not_null() { + // four lists of [null, 7]: as many null levels as rows, but the page has values + let page_writer = get_test_page_writer(); + let props = Default::default(); + let mut writer = get_test_column_writer::<Int32Type>(page_writer, 2, 1, props); + writer + .write_batch( + &[7, 7, 7, 7], + Some(&[1, 2, 1, 2, 1, 2, 1, 2]), + Some(&[0, 1, 0, 1, 0, 1, 0, 1]), + ) + .unwrap(); + + let r = writer.close().unwrap(); + let ColumnIndexMetaData::INT32(col_idx) = r.column_index.unwrap() else { + panic!("wrong stats type") + }; + assert!(!col_idx.is_null_page(0)); + assert_eq!(col_idx.min_value(0), Some(&7)); + assert_eq!(col_idx.max_value(0), Some(&7)); + assert_eq!(col_idx.null_count(0), Some(4)); + } + + #[test] + fn test_column_index_repeated_page_of_nulls() { + // two lists of [null, null]: no values, more null levels than rows + let page_writer = get_test_page_writer(); + let props = Default::default(); + let mut writer = get_test_column_writer::<Int32Type>(page_writer, 2, 1, props); + writer + .write_batch(&[], Some(&[1, 1, 1, 1]), Some(&[0, 1, 0, 1])) + .unwrap(); + + let r = writer.close().unwrap(); + let ColumnIndexMetaData::INT32(col_idx) = r.column_index.unwrap() else { + panic!("wrong stats type") + }; + assert!(col_idx.is_null_page(0)); + assert!(col_idx.min_value(0).is_none()); + assert!(col_idx.max_value(0).is_none()); + assert_eq!(col_idx.null_count(0), Some(4)); + } + #[test] fn test_column_offset_index_metadata() { // write data
