wgtmac commented on code in PR #36510:
URL: https://github.com/apache/arrow/pull/36510#discussion_r1282676014
##########
cpp/src/parquet/properties.h:
##########
@@ -79,10 +93,45 @@ class PARQUET_EXPORT ReaderProperties {
/// Disable buffered stream reading.
void disable_buffered_stream() { buffered_stream_enabled_ = false; }
- /// Return the size of the buffered stream buffer.
- int64_t buffer_size() const { return buffer_size_; }
- /// Set the size of the buffered stream buffer in bytes.
- void set_buffer_size(int64_t size) { buffer_size_ = size; }
+ /// Return the default size of the buffered stream buffer.
+ int64_t buffer_size() const { return
default_column_reader_properties_.buffer_size(); }
+ /// Set the default size of the buffered stream buffer in bytes.
+ void set_buffer_size(int64_t size) {
+ default_column_reader_properties_.set_buffer_size(size);
+ }
+
+ /// Return the size of the buffered stream buffer for a column chunk.
+ int64_t buffer_size(int row_group_index, int column_index) const {
Review Comment:
The current change looks better than leaking the property thing into the
reader api. However, I'm not sure if it is appropriate to introduce concept
like `row_group_index` into ReaderProperties, especially when
`ColumnReaderPropertiesMap` is organized into a two-layer design. Sorry but I
would -1 on making such change on optimization like this.
WDYT? @pitrou @emkornfield @mapleFU
--
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]
