adriangb commented on code in PR #10505:
URL: https://github.com/apache/arrow-rs/pull/10505#discussion_r3723322099
##########
parquet/src/column/writer/mod.rs:
##########
@@ -1030,6 +1042,39 @@ impl<'a, E: ColumnValueEncoder> GenericColumnWriter<'a,
E> {
}
}
+ /// Exempt a page's mandatory first value from the data page byte limit,
+ /// when that value alone already exceeds it.
+ ///
+ /// Parquet requires every data page to hold at least one value, so such a
+ /// value cannot be split out no matter how the limit is set. Counting it
+ /// against the limit makes the limit unsatisfiable, and
+ /// `should_add_data_page` then cuts a page after every single value. For
+ /// `DELTA_BYTE_ARRAY` that is destructive rather than merely wasteful:
+ /// each page boundary discards the previous value, so a column of large
+ /// values sharing long prefixes degenerates to `PLAIN` (#10489).
+ ///
+ /// Recording the value's encoded size here makes the limit apply to what
+ /// follows it — the bytes we *can* still place elsewhere. Only encodings
+ /// that compress against the preceding value opt in, so `PLAIN` and
+ /// `DELTA_LENGTH_BYTE_ARRAY` keep their tighter one-value page bound.
+ ///
+ /// Known limitation: the caller's trigger keys on a page-opening
Review Comment:
I think we can do it in https://github.com/apache/arrow-rs/pull/10554. Happy
to incorporate into one larger PR or do it in stages. I think we can do it in
stages since this PR doesn't make things any worse than the status quo.
--
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]
