M-Tesla commented on code in PR #11003:
URL: https://github.com/apache/arrow-rs/pull/11003#discussion_r4394691926
##########
parquet/src/column/chunker/cdc.rs:
##########
@@ -1134,6 +1134,89 @@ mod arrow_tests {
buf
}
+ /// Chunk sizes that still force an empty CDC page flush.
+ fn empty_page_repro_cdc_options() -> CdcOptions {
+ CdcOptions {
+ min_chunk_size: 8 * 1024,
+ max_chunk_size: 16 * 1024,
+ norm_level: 0,
+ }
+ }
+
+ /// Write a single-column batch with CDC plus the page-size / row-count
+ /// limits that make a forced chunk boundary flush an already-empty page.
+ fn write_boolean_cdc_with_small_pages(num_rows: usize) -> Vec<u8> {
+ let values: Vec<bool> = (0..num_rows).map(|i| i % 7 == 0).collect();
+ let col = Arc::new(BooleanArray::from(values)) as ArrayRef;
+ let batch = RecordBatch::try_from_iter([("flag", col)]).unwrap();
+
+ let props = WriterProperties::builder()
+ .set_writer_version(WriterVersion::PARQUET_2_0)
+ .set_data_page_size_limit(1024)
+ .set_content_defined_chunking(Some(empty_page_repro_cdc_options()))
+ .build();
+
+ let mut buf = Vec::new();
+ let mut writer = ArrowWriter::try_new(&mut buf, batch.schema(),
Some(props)).unwrap();
+ writer.write(&batch).unwrap();
+ writer.close().unwrap();
+ buf
+ }
+
+ fn count_data_pages(data: &[u8]) -> (usize, usize) {
+ let info = get_column_info(data, 0);
+ let total = info.iter().map(|c| c.page_lengths.len()).sum();
+ let empty = info
+ .iter()
+ .flat_map(|c| c.page_lengths.iter())
+ .filter(|&&n| n == 0)
+ .count();
+ (total, empty)
+ }
+
+ #[test]
+ #[cfg_attr(miri, ignore)] // large enough to hit CDC chunk boundaries
+ fn cdc_boolean_small_pages_does_not_panic() {
+ let data = write_boolean_cdc_with_small_pages(32_768);
+ let (total, empty) = count_data_pages(&data);
+ assert!(
+ total > 1,
+ "CDC should produce multiple data pages, got {total}"
+ );
+ assert_eq!(empty, 0, "forced CDC page breaks must not emit empty
pages");
+
+ let readback = read_batches(&data);
+ let rows: usize = readback.iter().map(|b| b.num_rows()).sum();
+ assert_eq!(rows, 32_768);
+ }
+
+ #[test]
+ #[cfg_attr(miri, ignore)]
Review Comment:
Added the same miri note the BOOLEAN test already has. Both tests together
finished in 0.02s under cargo test, without miri.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
