Enable RLE boolean encoding for v2 Parquet files #13886
Conversation
|
Pull requests from external contributors require approval from a |
|
/ok to test |
|
/ok to test |
| if (dict_bits >= 0 && physical_type != BOOLEAN) { | ||
| dst[0] = dict_bits; | ||
| s->rle_out = dst + 1; | ||
| } else if (is_v2 && physical_type == BOOLEAN) { | ||
| // save space for RLE length. we don't know the total length yet. |
There was a problem hiding this comment.
why is such adjustment only needed for bool RLE?
There was a problem hiding this comment.
So RLE is only used for 1) rep/def levels, 2) dictionary indexes, and 3) booleans. So booleans are the only data type that will be RLE encoded...everything else will be plain or dictionary (or eventually delta).
There's actually a table in the spec that lists when a length is prepended and when it is not.
+--------------+------------------------+-----------------+
| 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 |
+--------------+------------------------+-----------------+
There was a problem hiding this comment.
I think I get it, thanks.
And this is something that does not make sense to include in fragment size?
I'm asking because this "max RLE page size" is implemented as a util function in the ORC writer, so this manual padding looked odd in comparison.
BTW, must be nice not having RLE in almost every data type (cries in ORC).
There was a problem hiding this comment.
And this is something that does not make sense to include in fragment size?
yeah, because it's just once per page, not every fragment. although, yeah, maybe it should be included in the calculation at line 451. I'll ponder that.
There was a problem hiding this comment.
ok, I think that fixed it. nice catch! That would have been a lot of fun tracking down later :P
|
/ok to test |
|
/merge |
Description
While working on #13707 it was noticed that RLE encoding of booleans had been implemented and then disabled (see this comment for details). This PR re-enables RLE encoding for booleans, but only when V2 headers are being used.
Part of #13501.
Checklist