You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@parquet.apache.org by "wgtmac (via GitHub)" <gi...@apache.org> on 2023/02/27 09:01:30 UTC
[GitHub] [parquet-format] wgtmac opened a new pull request, #193: PARQUET-2222: RLE encoding spec incorrect for v2 data pages
wgtmac opened a new pull request, #193:
URL: https://github.com/apache/parquet-format/pull/193
Make sure you have checked _all_ steps below.
### Jira
- [ ] My PR addresses the following [Parquet Jira](https://issues.apache.org/jira/browse/PARQUET/) issues and references them in the PR title. For example, "PARQUET-1234: My Parquet PR"
- https://issues.apache.org/jira/browse/PARQUET-2222
- In case you are adding a dependency, check if the license complies with the [ASF 3rd Party License Policy](https://www.apache.org/legal/resolved.html#category-x).
### Commits
- [ ] My commits all reference Jira issues in their subject lines. In addition, my commits follow the guidelines from "[How to write a good git commit message](http://chris.beams.io/posts/git-commit/)":
1. Subject is separated from body by a blank line
1. Subject is limited to 50 characters (not including Jira issue reference)
1. Subject does not end with a period
1. Subject uses the imperative mood ("add", not "adding")
1. Body wraps at 72 characters
1. Body explains "what" and "why", not "how"
### Documentation
- [ ] In case of new functionality, my PR adds documentation that describes how to use it.
- All the public functions and the classes in the PR contain Javadoc that explain what it does
--
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: dev-unsubscribe@parquet.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [parquet-format] mapleFU commented on pull request #193: PARQUET-2222: RLE encoding spec incorrect for v2 data pages
Posted by "mapleFU (via GitHub)" <gi...@apache.org>.
mapleFU commented on PR #193:
URL: https://github.com/apache/parquet-format/pull/193#issuecomment-1482193618
No problem, I think that's RLE_DICTIONARY's problem. Just ignore it
--
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: dev-unsubscribe@parquet.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [parquet-format] pitrou commented on pull request #193: PARQUET-2222: RLE encoding spec incorrect for v2 data pages
Posted by "pitrou (via GitHub)" <gi...@apache.org>.
pitrou commented on PR #193:
URL: https://github.com/apache/parquet-format/pull/193#issuecomment-1474072383
Hmm, can you point me to the place where the length is written out for RLE-encoded boolean data in v2 data pages?
--
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: dev-unsubscribe@parquet.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [parquet-format] pitrou commented on pull request #193: PARQUET-2222: RLE encoding spec incorrect for v2 data pages
Posted by "pitrou (via GitHub)" <gi...@apache.org>.
pitrou commented on PR #193:
URL: https://github.com/apache/parquet-format/pull/193#issuecomment-1474186504
I think this should be more explicit, e.g.:
```
// The length-prepended version is used for:
// - in v1 data pages: definition levels, repetition levels, and RLE-encoded boolean data
// - in v2 data pages: RLE-encoded boolean data
length-prepended-rle-bit-packed-hybrid: <length> <encoded-data>
// The non-length-prepended version is used for:
// - in v1 data pages: dictionary indices
// - in v2 data pages: definition levels, repetition levels, and dictionary indices
rle-bit-packed-hybrid: <encoded-data>
length := length of the <encoded-data> in bytes stored as 4 bytes little endian (unsigned int32)
encoded-data := <run>*
```
--
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: dev-unsubscribe@parquet.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [parquet-format] pitrou commented on pull request #193: PARQUET-2222: RLE encoding spec incorrect for v2 data pages
Posted by "pitrou (via GitHub)" <gi...@apache.org>.
pitrou commented on PR #193:
URL: https://github.com/apache/parquet-format/pull/193#issuecomment-1474188220
Another possibility is a nice table:
```
+--------------+------------------------+-----------------+
| Page kind | RLE-encoded data kind | Prepend length? |
+--------------+------------------------+-----------------+
| Data page v1 | Definition levels | Y |
| | Repetition levels | Y |
| | Dictionary indices | N |
| | Boolean values | Y |
+--------------+------------------------+-----------------+
| Data page v2 | Definition levels | N |
| | Repetition levels | N |
| | Dictionary indices | N |
| | Boolean values | Y |
+--------------+------------------------+-----------------+
```
--
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: dev-unsubscribe@parquet.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [parquet-format] pitrou commented on pull request #193: PARQUET-2222: RLE encoding spec incorrect for v2 data pages
Posted by "pitrou (via GitHub)" <gi...@apache.org>.
pitrou commented on PR #193:
URL: https://github.com/apache/parquet-format/pull/193#issuecomment-1474188853
Also, please someone with better knowledge of parquet-mr comment on [https://github.com/apache/parquet-format/pull/193#issuecomment-1474171946].
--
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: dev-unsubscribe@parquet.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [parquet-format] wgtmac commented on pull request #193: PARQUET-2222: RLE encoding spec incorrect for v2 data pages
Posted by "wgtmac (via GitHub)" <gi...@apache.org>.
wgtmac commented on PR #193:
URL: https://github.com/apache/parquet-format/pull/193#issuecomment-1482199554
> No problem, I think that's RLE_DICTIONARY's problem. Just ignore it
Sounds good.
@pitrou Do you have any comment? I just want to make sure all reviewers are happy on this.
--
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: dev-unsubscribe@parquet.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [parquet-format] pitrou commented on pull request #193: PARQUET-2222: RLE encoding spec incorrect for v2 data pages
Posted by "pitrou (via GitHub)" <gi...@apache.org>.
pitrou commented on PR #193:
URL: https://github.com/apache/parquet-format/pull/193#issuecomment-1445971725
How about also stating it directly in the grammar:
```
// v1 data pages prepend the encoded length...
rle-bit-packed-hybrid-v1: <length> <encoded-data>
length := length of the <encoded-data> in bytes stored as 4 bytes little endian (unsigned int32)
// ... while v2 data pages don't
rle-bit-packed-hybrid-v2: <encoded-data>
encoded-data := <run>*
```
--
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: dev-unsubscribe@parquet.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [parquet-format] pitrou commented on pull request #193: PARQUET-2222: RLE encoding spec incorrect for v2 data pages
Posted by "pitrou (via GitHub)" <gi...@apache.org>.
pitrou commented on PR #193:
URL: https://github.com/apache/parquet-format/pull/193#issuecomment-1482461160
@wgtmac LGTM, thank you!
--
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: dev-unsubscribe@parquet.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [parquet-format] wgtmac commented on pull request #193: PARQUET-2222: RLE encoding spec incorrect for v2 data pages
Posted by "wgtmac (via GitHub)" <gi...@apache.org>.
wgtmac commented on PR #193:
URL: https://github.com/apache/parquet-format/pull/193#issuecomment-1475639855
> Also, please someone with better knowledge of parquet-mr comment on [#193 (comment)](https://github.com/apache/parquet-format/pull/193#issuecomment-1474171946).
I agree with you after a double-check @pitrou
--
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: dev-unsubscribe@parquet.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [parquet-format] wgtmac commented on pull request #193: PARQUET-2222: RLE encoding spec incorrect for v2 data pages
Posted by "wgtmac (via GitHub)" <gi...@apache.org>.
wgtmac commented on PR #193:
URL: https://github.com/apache/parquet-format/pull/193#issuecomment-1473382293
Gentle ping @gszadovszky
--
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: dev-unsubscribe@parquet.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [parquet-format] wgtmac commented on pull request #193: PARQUET-2222: RLE encoding spec incorrect for v2 data pages
Posted by "wgtmac (via GitHub)" <gi...@apache.org>.
wgtmac commented on PR #193:
URL: https://github.com/apache/parquet-format/pull/193#issuecomment-1477968033
> By the way, I think RLE_DICTIONARY is a bit wired, because it has a prepend byte. Should we mention this?
Do you mean dictionary indices are different between `PLAIN_DICTIONARY` and `RLE_DICTIONARY`? @mapleFU
--
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: dev-unsubscribe@parquet.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [parquet-format] wgtmac merged pull request #193: PARQUET-2222: RLE encoding spec incorrect for v2 data pages
Posted by "wgtmac (via GitHub)" <gi...@apache.org>.
wgtmac merged PR #193:
URL: https://github.com/apache/parquet-format/pull/193
--
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: dev-unsubscribe@parquet.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [parquet-format] gszadovszky commented on pull request #193: PARQUET-2222: RLE encoding spec incorrect for v2 data pages
Posted by "gszadovszky (via GitHub)" <gi...@apache.org>.
gszadovszky commented on PR #193:
URL: https://github.com/apache/parquet-format/pull/193#issuecomment-1473665965
@wgtmac is correct that `length` is left out only in case of `v2` `DL` and `RL`. Meanwhile I agree with @pitrou that the note is better to be mentioned at the grammar spec:
```
// length is not written for definition/repetition levels in v2 data pages since it is duplicated in the `DataPageHeaderV2`
length := length of the <encoded-data> in bytes stored as 4 bytes little endian (unsigned int32)
```
--
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: dev-unsubscribe@parquet.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [parquet-format] wgtmac commented on pull request #193: PARQUET-2222: RLE encoding spec incorrect for v2 data pages
Posted by "wgtmac (via GitHub)" <gi...@apache.org>.
wgtmac commented on PR #193:
URL: https://github.com/apache/parquet-format/pull/193#issuecomment-1473932634
Sounds good. Let me fix it.
--
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: dev-unsubscribe@parquet.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [parquet-format] pitrou commented on pull request #193: PARQUET-2222: RLE encoding spec incorrect for v2 data pages
Posted by "pitrou (via GitHub)" <gi...@apache.org>.
pitrou commented on PR #193:
URL: https://github.com/apache/parquet-format/pull/193#issuecomment-1474171946
Ok, so v2 data pages for RLE-encoded boolean do encode the length:
https://github.com/apache/parquet-mr/blob/1235003e742e6a76bf6cb8f7ed33e942fa12d0d5/parquet-column/src/main/java/org/apache/parquet/column/values/factory/DefaultV2ValuesWriterFactory.java#L77-L80
However, I don't think that RLE-encoded dictionary indices encode the length:
https://github.com/apache/parquet-mr/blob/1235003e742e6a76bf6cb8f7ed33e942fa12d0d5/parquet-column/src/main/java/org/apache/parquet/column/values/dictionary/DictionaryValuesWriter.java#L161-L162
... because `RunLengthBitPackingHybridValuesWriter` prepends the length, but `RunLengthBitPackingHybridEncoder` doesn't:
https://github.com/apache/parquet-mr/blob/1235003e742e6a76bf6cb8f7ed33e942fa12d0d5/parquet-column/src/main/java/org/apache/parquet/column/values/rle/RunLengthBitPackingHybridValuesWriter.java#L68-L70
--
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: dev-unsubscribe@parquet.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [parquet-format] mapleFU commented on pull request #193: PARQUET-2222: RLE encoding spec incorrect for v2 data pages
Posted by "mapleFU (via GitHub)" <gi...@apache.org>.
mapleFU commented on PR #193:
URL: https://github.com/apache/parquet-format/pull/193#issuecomment-1474140277
@pitrou I guess it's already within DataPage. It's the first 4B in deserialized data
--
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: dev-unsubscribe@parquet.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [parquet-format] wgtmac commented on pull request #193: PARQUET-2222: RLE encoding spec incorrect for v2 data pages
Posted by "wgtmac (via GitHub)" <gi...@apache.org>.
wgtmac commented on PR #193:
URL: https://github.com/apache/parquet-format/pull/193#issuecomment-1445975762
As the RLE encoding also applies to boolean values and dictionary-encoded indices in the data page v2, this may not be correct. @pitrou
--
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: dev-unsubscribe@parquet.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [parquet-format] wgtmac commented on pull request #193: PARQUET-2222: RLE encoding spec incorrect for v2 data pages
Posted by "wgtmac (via GitHub)" <gi...@apache.org>.
wgtmac commented on PR #193:
URL: https://github.com/apache/parquet-format/pull/193#issuecomment-1445956634
@pitrou @gszadovszky @shangxinli
--
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: dev-unsubscribe@parquet.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [parquet-format] mapleFU commented on pull request #193: PARQUET-2222: RLE encoding spec incorrect for v2 data pages
Posted by "mapleFU (via GitHub)" <gi...@apache.org>.
mapleFU commented on PR #193:
URL: https://github.com/apache/parquet-format/pull/193#issuecomment-1477713672
> Data page format: the bit width used to encode the entry ids stored as 1 byte (max bit width = 32), followed by the values encoded using RLE/Bit packed described above (with the given bit width).
By the way, I think RLE_DICTIONARY is a bit wired, because it has a prepend byte. Should we mention this?
--
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: dev-unsubscribe@parquet.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [parquet-format] pitrou commented on pull request #193: PARQUET-2222: RLE encoding spec incorrect for v2 data pages
Posted by "pitrou (via GitHub)" <gi...@apache.org>.
pitrou commented on PR #193:
URL: https://github.com/apache/parquet-format/pull/193#issuecomment-1474154058
I was alluding to this comment:
> @wgtmac is correct that `length` is left out only in case of `v2` `DL` and `RL`.
--
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: dev-unsubscribe@parquet.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org