alamb commented on code in PR #7987:
URL: https://github.com/apache/arrow-rs/pull/7987#discussion_r2231689129
##########
parquet-variant/src/builder.rs:
##########
@@ -2861,8 +2888,7 @@ mod tests {
// Only the second attempt should appear in the final variant
let (metadata, value) = builder.finish();
let metadata = VariantMetadata::try_new(&metadata).unwrap();
- assert_eq!(metadata.len(), 1);
- assert_eq!(&metadata[0], "name"); // not rolled back
+ assert!(metadata.is_empty()); // rolled back
Review Comment:
nice!
##########
parquet-variant/src/builder.rs:
##########
@@ -1216,24 +1211,45 @@ impl<'a> ListBuilder<'a> {
/// Finalizes this list and appends it to its parent, which otherwise
remains unmodified.
pub fn finish(mut self) {
- let data_size = self.buffer.offset();
+ let buffer = self.parent_state.buffer();
+
+ let data_size = buffer.offset() - self.parent_value_offset_base;
Review Comment:
should we do a checked sub here to avoid underflow? An underflow would only
happen with a bug in the implementation so this is probably fine
##########
parquet-variant/src/builder.rs:
##########
@@ -1121,16 +1099,27 @@ impl VariantBuilder {
pub struct ListBuilder<'a> {
parent_state: ParentState<'a>,
offsets: Vec<usize>,
- buffer: ValueBuffer,
+ /// The starting offset in the parent's buffer where this list starts
+ parent_value_offset_base: usize,
+ /// The starting offset in the parent's metadata buffer where this list
starts
+ /// used to truncate the written fields in `drop` if the current list has
not been finished
+ parent_metadata_offset_base: usize,
Review Comment:
this is a good idea
--
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]
