emecii commented on code in PR #51277:
URL: https://github.com/apache/arrow/pull/51277#discussion_r4095769650
##########
c_glib/parquet-glib/arrow-file-reader.cpp:
##########
@@ -25,25 +25,191 @@
#include <parquet/file_reader.h>
+namespace {
+ GParquetArrowFileReader *
+ open_reader_with_properties(std::shared_ptr<arrow::io::RandomAccessFile>
source,
+ GArrowSeekableInputStream *source_object,
+ GParquetReaderProperties *properties,
+ GError **error,
+ const char *tag)
+ {
+ auto parquet_properties = properties ?
gparquet_reader_properties_get_raw(properties)
+ :
parquet::default_reader_properties();
+ parquet::arrow::FileReaderBuilder builder;
+ if (!garrow::check(error, builder.Open(source, parquet_properties), tag)) {
+ return NULL;
+ }
+ if (parquet_properties.is_buffered_stream_enabled()) {
+ // Read-ahead would bypass the buffered stream by caching whole column
chunks.
+ auto arrow_properties = parquet::default_arrow_reader_properties();
+ arrow_properties.set_pre_buffer(false);
+ builder.properties(arrow_properties);
+ }
+ auto result = builder.Build();
+ if (!garrow::check(error, result, tag)) {
+ return NULL;
+ }
+ return
GPARQUET_ARROW_FILE_READER(g_object_new(GPARQUET_TYPE_ARROW_FILE_READER,
+ "arrow-file-reader",
+ result->release(),
+ "source",
+ source_object,
+ NULL));
Review Comment:
Updated in 3b468be4fd22e12e89a28f904cb5794841c1239f: `open_reader()` now
constructs the wrapper through `gparquet_arrow_file_reader_new_raw(reader,
source)`, which sets the construct-only source property. The existing
one-argument raw constructor delegates with a null source. Native C checks pass
for source retention and final release.
##########
c_glib/parquet-glib/arrow-file-reader.cpp:
##########
@@ -25,25 +25,191 @@
#include <parquet/file_reader.h>
+namespace {
+ GParquetArrowFileReader *
+ open_reader_with_properties(std::shared_ptr<arrow::io::RandomAccessFile>
source,
Review Comment:
Renamed the helper to `open_reader()` and reused it for both `new_arrow()`
and `new_arrow_full()` in 3b468be4fd22e12e89a28f904cb5794841c1239f. The legacy
constructor retains its error tag and default properties. The native I/O probe
confirms identical default reads; a separate C check verifies legacy source
retention.
##########
c_glib/parquet-glib/arrow-file-reader.cpp:
##########
@@ -25,25 +25,191 @@
#include <parquet/file_reader.h>
+namespace {
+ GParquetArrowFileReader *
+ open_reader_with_properties(std::shared_ptr<arrow::io::RandomAccessFile>
source,
+ GArrowSeekableInputStream *source_object,
+ GParquetReaderProperties *properties,
+ GError **error,
+ const char *tag)
+ {
+ auto parquet_properties = properties ?
gparquet_reader_properties_get_raw(properties)
+ :
parquet::default_reader_properties();
+ parquet::arrow::FileReaderBuilder builder;
+ if (!garrow::check(error, builder.Open(source, parquet_properties), tag)) {
+ return NULL;
+ }
+ if (parquet_properties.is_buffered_stream_enabled()) {
+ // Read-ahead would bypass the buffered stream by caching whole column
chunks.
+ auto arrow_properties = parquet::default_arrow_reader_properties();
+ arrow_properties.set_pre_buffer(false);
Review Comment:
Added `parquet::ArrowReaderProperties` to the private properties struct and
exposed `set_pre_buffer()` / `get_pre_buffer()` in
3b468be4fd22e12e89a28f904cb5794841c1239f. Buffered-stream toggles no longer
change pre-buffering; Ruby callers explicitly use `properties.pre_buffer =
false`. Both native properties are copied into the reader. Updated
documentation and tests pass: 17 focused GLib tests, 74 GLib Parquet tests, 18
Red Parquet tests, and 14 native I/O cases covering defaults, independent
settings and snapshots after mutation/destruction.
--
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]
