AntoinePrv commented on code in PR #50422:
URL: https://github.com/apache/arrow/pull/50422#discussion_r3552621875
##########
cpp/src/parquet/column_reader.cc:
##########
@@ -95,52 +96,73 @@ constexpr std::string_view
kErrorRepDefLevelNotMatchesNumValues =
} // namespace
-LevelDecoder::LevelDecoder() : num_values_remaining_(0) {}
+/******************
+ * LevelDecoder *
+ ******************/
+
+struct LevelDecoder::Impl {
+ using RleBitPackedDecoder = ::arrow::util::RleBitPackedDecoder<int16_t>;
+ using BitPackedDecoder = ::arrow::util::BitPackedDecoder<int16_t>;
+
+ std::variant<RleBitPackedDecoder, BitPackedDecoder> decoder = {};
Review Comment:
Here the set is of vaiants is closed: users do not have access to it to
derive theirs (and adding extras impl in Arrow is still possible).
In these cases, I have a bias for `std::variant` since it does not require
an extra alloc and deriving ABCs.
Here with the `Pimpl` it is probably the same. We could also consider having
the variant directly in the `LevelDecoder` with well defined `_fwd` headers, or
with hard coded opaque buffer to reduce an extra indirection.
--
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]
