AntoinePrv commented on code in PR #50629:
URL: https://github.com/apache/arrow/pull/50629#discussion_r3656342831
##########
cpp/src/parquet/column_reader.cc:
##########
@@ -222,6 +254,161 @@ auto LevelDecoder::CountUpTo(int16_t value, int
batch_size) -> CountUpToResult {
};
}
+/**************************
+ * LevelToBitmapDecoder *
+ **************************/
+
+/// Decoder for definition levels that writes directly into a validity bitmap.
+///
+/// This is the bitmap counterpart of ``LevelDecoder``, specialized for levels
+/// encoded on a single bit (a max level of 1), such as the definition levels
of a
+/// flat, nullable column. Rather than decoding into an ``int16_t`` array and
+/// re-encoding into an Arrow validity bitmap, it decodes straight into the
bitmap.
+///
+/// @see LevelDecoder
+class LevelToBitmapDecoder {
Review Comment:
The issue is that `Decode` takes fundamentally different arguments:
- In the general case, an `int16_t*` array where the data type correspond to
one value
- In the bitmap case, a `uint8_t*`array + bit offset to find the exact
pointed bit.
So we'd need to also dynamically dispatch the output type here, which IMHO
brings extra complexity and performance hit (virtual polymorphism brings little
since this is fully private and used in separate classes).
If this is about code reuse, I would suggest:
- Make a de-pimpled templated `LevelDecoder`
- Either:
- Wrap it in a pimpl to make the existing `LevelDecoder`
- OR split `rle_encoding_internal.hpp` into a
`rle_decoding_fwd_internal.hpp`+`rle_decoding_internal.hpp` to have a light
class definition (without function definitions and other headers) to include in
`column_reader.h`.
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
