Move ParquetMetadataWriter to its own module, update documentation
#6202
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
alamb
commented
Aug 6, 2024
| @@ -730,281 +729,6 @@ impl<'a, W: Write + Send> PageWriter for SerializedPageWriter<'a, W> { | |||
| } | |||
| } | |||
|
|
|||
| /// Writes `crate::file::metadata` structures to a thrift encdoded byte streams | |||
There was a problem hiding this comment.
This code was mostly moved without modification - see the second commit to see what changed
This was referenced Aug 6, 2024
etseidl
reviewed
Aug 6, 2024
| /// | ||
| /// [`FileMetaData`]: crate::format::FileMetaData | ||
| /// | ||
| /// ```text |
There was a problem hiding this comment.
This ascii art will get interesting when the new footer extension work begins 😅.
| /// │┌ ─ ─ ─ ─ ─ ─ ─ ─ ─ ┐ │ │ FileMetadata | ||
| /// │ OffsetIndex │ contains embedded | ||
| /// ││ (Optional) │◀┼ ─ │ offsets to | ||
| /// │ ─ ─ ─ ─ ─ ─ ─ ─ ─ ─ │ │ ColumnIndex and |
There was a problem hiding this comment.
Where do Bloom filters go? I've never used them, so genuinely don't know.
There was a problem hiding this comment.
Bloom filters can theoretically be written anywhere, in practice they're either written after each row groups or just before the column index
There was a problem hiding this comment.
I tried to explain more how you would write BloomFilters in d51c697. Any corrections / comments would be appreciated
/// Note: this writer does not directly write BloomFilters. In order to write
/// BloomFilters, write the bloom filters into the buffer before creating the
/// metadata writer. Then set the corresponding `bloom_filter_offset` and
/// `bloom_filter_length` on [`ColumnChunkMetaData`] passed to this writer.
Co-authored-by: Ed Seidl <etseidl@users.noreply.github.com>
…o alamb/make_writer_pub
etseidl
reviewed
Aug 8, 2024
Co-authored-by: Ed Seidl <etseidl@users.noreply.github.com>
|
Thanks again @etseidl for the review |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Which issue does this PR close?
Closes #.
Rationale for this change
@AdrianB added
ParquetMetadataWriterin #6197 but the documentation was onThriftMetadataWriter.I also think this is a very important API and thus deserves special attention to documentation
What changes are included in this PR?
This PR sdds some documentation / examples (which also ensure
the code compiles and can be used by crate users), and makes a few tweaks:
ParquetMetadataWriter::write_page_index(the page index is written based on its presence)ParquetMetadataWriter::finish(&mut self)toParquetMetadataWriter::finish(mut self)so it can be used without beingmutnote I did the changes in two commits so it is clearer what I changed.
Are there any user-facing changes?
Yes there are some API changes in an as yet unreleased API