PARQUET-2369: Clarify GZIP Support for Multiple GZIP Members #218
Conversation
tustvold
changed the title
tustvold
force-pushed
the
clarify-gzip-members
branch
from
f7b9766
to
d3a7421
Compare
|
Posted to the mailing list - https://lists.apache.org/thread/zzdh20j8fhrrt3dmx6ow4dbbccmvh277 |
|
+1 from me. |
@tustvold I don't see much progress in the mailing list, do you have a deadline for waiting for responds on it and moving forward? |
|
LGTM, thanks for working on it. |
|
I will merge this if there is no objection by the end of this week. |
| @@ -58,6 +58,10 @@ formats) defined by [RFC 1952](https://tools.ietf.org/html/rfc1952). | |||
| If any ambiguity arises when implementing this format, the implementation | |||
| provided by the [zlib compression library](https://zlib.net/) is authoritative. | |||
|
|
|||
| Readers should support reading pages containing multiple GZIP members, however, | |||
There was a problem hiding this comment.
Might be worth linking to the RFC of GZIP members: https://datatracker.ietf.org/doc/html/rfc1952#section-2.2
There was a problem hiding this comment.
its already mentioned:
GZIP
A codec based on the GZIP format (not the closely-related "zlib" or "deflate"
formats) defined by [RFC 1952](https://tools.ietf.org/html/rfc1952).
There was a problem hiding this comment.
Right to clarify I'm suggesting that we link "members" to its specific section where it is defined.
|
@wgtmac are you planning to merge? I see all required approved were achieved |
I thought there is a minor suggestion: #218 (comment). Would you mind adding that? |
@emkornfield @wgtmac |
Agreed, let's not overdo this. |
I'm ok with this. |
|
I just merged this. Thanks everyone! |
See apache/parquet-testing#41 for more context