AntoinePrv commented on code in PR #47294:
URL: https://github.com/apache/arrow/pull/47294#discussion_r2355590946
##########
cpp/src/arrow/util/rle_encoding_internal.h:
##########
@@ -84,32 +85,278 @@ namespace util {
/// (total 26 bytes, 1 byte overhead)
//
+class RleRun {
+ public:
+ using byte = uint8_t;
+ /// Enough space to store a 64bit value
+ using raw_data_storage = std::array<byte, 8>;
+ using raw_data_const_pointer = const byte*;
+ using raw_data_size_type = int32_t;
+ /// The type of the size of either run, between 1 and 2^31-1 as per Parquet
spec
+ using values_count_type = int32_t;
+ /// The type to represent a size in bits
+ using bit_size_type = int32_t;
+
+ constexpr RleRun() noexcept = default;
Review Comment:
I still believe the `RleRun` is not trivial. It requires partially filling
the local array (and also opportunities for some checks). It acts as a cheap
read-only run with an interface uniform with `BitPackedRun`.
Now with the function definition within the class and reduced number of
aliases, it reads clearly within one screen height. I don't think exposing the
member variable would do more than save a few lines.
--
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]
