AntoinePrv commented on code in PR #47294:
URL: https://github.com/apache/arrow/pull/47294#discussion_r2349510714


##########
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:
   Looking only at the Run classes, they do feel like they could be simple 
classes.
   However, in the much other functions in this file, they bring a much 
welcomed (small) reduction of complexity:
   - The two run classes have a similar interface;
   - They manage the conversion from bit size and number of values to a ready 
to use ptr+len;
   - `RleRun` hides the unused bytes.
   
   Perhaps we could instead put the member functions definition within the 
class (as suggested by another comment).



-- 
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: github-unsubscr...@arrow.apache.org

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org

Reply via email to