Core: Support view metadata compression#8552
Merged
Merged
Conversation
311bb6b to
8553226
Compare
rdblue
reviewed
Oct 2, 2023
rdblue
reviewed
Oct 2, 2023
| @ValueSource(strings = {"none", "gzip"}) | ||
| public void metadataCompression(String codecName) throws IOException { | ||
| Codec codec = Codec.fromName(codecName); | ||
| String location = Paths.get(tmp.toString(), "v1" + getFileExtension(codec)).toString(); |
Contributor
There was a problem hiding this comment.
I don't think that this should rely on getFileExtension. It's fine that it is used by BaseViewOperations to name the file, but there's no need to do that here. Instead, I think this should be parameterized by file name (v1.metadata.json and v1.gz.metadata.json) so that it is obvious what happens based on the file name in the test. Otherwise this is testing that the extension produced by getFileExtension triggers some behavior in the parser, rather than a specific extension triggers behavior in the parser.
Contributor
Author
There was a problem hiding this comment.
I was aligning this test with how compression was tested for table metadata in
but I've changed it to have explicit file names8553226 to
2eb49de
Compare
2eb49de to
243a4b9
Compare
rdblue
approved these changes
Oct 11, 2023
Contributor
|
Thanks, @nastra! |
zhongyujiang
pushed a commit
to zhongyujiang/iceberg
that referenced
this pull request
Apr 16, 2025
(cherry picked from commit b7e5d68)
This file contains hidden or 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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
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.
No description provided.