etseidl commented on code in PR #11297: URL: https://github.com/apache/arrow-rs/pull/11297#discussion_r4300210319
########## parquet/src/file/metadata/mod.rs: ########## @@ -116,15 +116,18 @@ pub(crate) use writer::ThriftMetadataWriter; /// This structure is read by the various readers in this crate or can be read /// directly from a file using the [`ParquetMetaDataReader`] struct. /// +/// The individual fields of this structure are stored in a way that make cloning +/// this structure low-cost. +/// /// See the [`ParquetMetaDataBuilder`] to create and modify this structure. /// /// [`parquet.thrift`]: https://github.com/apache/parquet-format/blob/master/src/main/thrift/parquet.thrift #[derive(Debug, Clone)] pub struct ParquetMetaData { /// File level metadata - file_metadata: FileMetaData, + file_metadata: Arc<FileMetaData>, /// Row group metadata - row_groups: Vec<RowGroupMetaData>, + row_groups: Arc<Vec<RowGroupMetaData>>, Review Comment: > I think we can make this Arc<Vec<Arc<RowGroupMetaData>>> in the future if that turns out to be a problem Yes. I'd like to eventually pull the column chunk metadata out of the row group and instead offer it up the same way as page indexes or bloom filters...they're all indexed by `(row_group_idx, column_idx)` anyway, why pay all the allocation pain for all of these vectors of metadata buried in vectors of structs? >Another minor thing here to save a few bytes (maybe 16 bytes per instance) is to instead of >``` > row_groups: Arc<Vec<RowGroupMetaData>>,` >``` > >remove the Vec part: >``` > row_groups: Arc<[RowGroupMetaData]>, >``` Can do, but that has a few implications. Burned a few tokens to work around those, and it does save 16 bytes per row group. Adds a bit of complexity to `ParquetMetaDataBuilder`. I'll push what I've got now, and let you decide if you want to roll it back. Thanks for the review! ❤️ -- 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]
