wgtmac commented on code in PR #40594:
URL: https://github.com/apache/arrow/pull/40594#discussion_r1858175561
##########
cpp/src/parquet/page_index.h:
##########
@@ -299,15 +315,14 @@ class PARQUET_EXPORT OffsetIndexBuilder {
virtual ~OffsetIndexBuilder() = default;
- /// \brief Add page location of a data page.
- virtual void AddPage(int64_t offset, int32_t compressed_page_size,
- int64_t first_row_index) = 0;
+ /// \brief Add page location and size stats of a data page.
+ virtual void AddPage(
+ int64_t offset, int32_t compressed_page_size, int64_t first_row_index,
+ std::optional<int64_t> unencoded_byte_array_length = std::nullopt) = 0;
- /// \brief Add page location of a data page.
- void AddPage(const PageLocation& page_location) {
- AddPage(page_location.offset, page_location.compressed_page_size,
- page_location.first_row_index);
- }
+ /// \brief Add page location and size stats of a data page.
+ void AddPage(const PageLocation& page_location,
+ const SizeStatistics* size_stats = NULLPTR);
Review Comment:
Let me change to use `const SizeStatistics&`. A caveat is that it must
include size_statistics.h in order to add a default parameter. To avoid this, I
think it does not hurt to make it a slightly breaking change.
--
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
