GH-44784: [C++][Parquet] Add arrow::Result version of parquet::arrow::OpenFile()

[kou]

Rationale for this change

We're migrating arrow::Status + output variable API to arrow::Result API.

What changes are included in this PR?

• Add arrow::Result<std::unique_ptr<FileReader>> parquet::arrow::OpenFile()
• Deprecate arrow::Status parquet::arrow::OpenFile()
• Use the added arrow::Result version in our code base

Are these changes tested?

Yes.

Are there any user-facing changes?

Yes.

• GitHub Issue: [C++][Parquet] Add arrow::Result version of parquet::arrow::OpenFile() #44784

[github-actions]

⚠️ GitHub issue #44784 has been automatically assigned in GitHub to PR creator.

[mapleFU]

LGTM though I think it's just style issue

[raulcd]

The error on CGLib Ruby is related:

FAILED: parquet-glib/libparquet-glib.1900.dylib.p/arrow-file-reader.cpp.o 
ccache c++ -Iparquet-glib/libparquet-glib.1900.dylib.p -I. -I../../c_glib -I/tmp/local/include -I/opt/homebrew/Cellar/glib/2.82.2/include -I/opt/homebrew/Cellar/glib/2.82.2/include/glib-2.0 -I/opt/homebrew/Cellar/glib/2.82.2/lib/glib-2.0/include -I/opt/homebrew/opt/gettext/include -I/opt/homebrew/Cellar/pcre2/10.44/include -I/Library/Developer/CommandLineTools/SDKs/MacOSX14.sdk/usr/include/ffi -fdiagnostics-color=always -D_LIBCPP_ENABLE_ASSERTIONS=1 -Wall -Winvalid-pch -Werror -std=c++17 -O0 -g -Wmissing-declarations -DGPARQUET_COMPILATION -MD -MQ parquet-glib/libparquet-glib.1900.dylib.p/arrow-file-reader.cpp.o -MF parquet-glib/libparquet-glib.1900.dylib.p/arrow-file-reader.cpp.o.d -o parquet-glib/libparquet-glib.1900.dylib.p/arrow-file-reader.cpp.o -c ../../c_glib/parquet-glib/arrow-file-reader.cpp
../../c_glib/parquet-glib/arrow-file-reader.cpp:138:33: error: 'OpenFile' is deprecated: Deprecated in 19.0.0. Use arrow::Result version instead. [-Werror,-Wdeprecated-declarations]
  auto status = parquet::arrow::OpenFile(arrow_random_access_file,
                                ^
/tmp/local/include/parquet/arrow/reader.h:362:1: note: 'OpenFile' has been explicitly marked deprecated here
ARROW_DEPRECATED("Deprecated in 19.0.0. Use arrow::Result version instead.")
^
/tmp/local/include/arrow/util/macros.h:140:35: note: expanded from macro 'ARROW_DEPRECATED'
#  define ARROW_DEPRECATED(...) [[deprecated(__VA_ARGS__)]]
                                  ^
../../c_glib/parquet-glib/arrow-file-reader.cpp:172:33: error: 'OpenFile' is deprecated: Deprecated in 19.0.0. Use arrow::Result version instead. [-Werror,-Wdeprecated-declarations]
  auto status = parquet::arrow::OpenFile(arrow_random_access_file,
                                ^
/tmp/local/include/parquet/arrow/reader.h:362:1: note: 'OpenFile' has been explicitly marked deprecated here
ARROW_DEPRECATED("Deprecated in 19.0.0. Use arrow::Result version instead.")
^
/tmp/local/include/arrow/util/macros.h:140:35: note: expanded from macro 'ARROW_DEPRECATED'
#  define ARROW_DEPRECATED(...) [[deprecated(__VA_ARGS__)]]
                                  ^
2 errors generated.

I tried something locally and something like this should work:

diff --git a/c_glib/parquet-glib/arrow-file-reader.cpp b/c_glib/parquet-glib/arrow-file-reader.cpp
index 4996d7862..e3d707ab5 100644
--- a/c_glib/parquet-glib/arrow-file-reader.cpp
+++ b/c_glib/parquet-glib/arrow-file-reader.cpp
@@ -134,12 +134,10 @@ gparquet_arrow_file_reader_new_arrow(GArrowSeekableInputStream *source, GError *
 {
   auto arrow_random_access_file = garrow_seekable_input_stream_get_raw(source);
   auto arrow_memory_pool = arrow::default_memory_pool();
-  std::unique_ptr<parquet::arrow::FileReader> parquet_arrow_file_reader;
-  auto status = parquet::arrow::OpenFile(arrow_random_access_file,
-                                         arrow_memory_pool,
-                                         &parquet_arrow_file_reader);
-  if (garrow_error_check(error, status, "[parquet][arrow][file-reader][new-arrow]")) {
-    return gparquet_arrow_file_reader_new_raw(parquet_arrow_file_reader.release());
+  auto parquet_arrow_open_file_result = parquet::arrow::OpenFile(arrow_random_access_file,
+                                         arrow_memory_pool);
+  if (garrow::check(error, parquet_arrow_open_file_result, "[parquet][arrow][file-reader][new-arrow]")) {
+    return gparquet_arrow_file_reader_new_raw(parquet_arrow_open_file_result->release());
   } else {
     return NULL;
   }
@@ -168,12 +166,10 @@ gparquet_arrow_file_reader_new_path(const gchar *path, GError **error)
   std::shared_ptr<arrow::io::RandomAccessFile> arrow_random_access_file =
     arrow_memory_mapped_file.ValueOrDie();
   auto arrow_memory_pool = arrow::default_memory_pool();
-  std::unique_ptr<parquet::arrow::FileReader> parquet_arrow_file_reader;
-  auto status = parquet::arrow::OpenFile(arrow_random_access_file,
-                                         arrow_memory_pool,
-                                         &parquet_arrow_file_reader);
-  if (garrow::check(error, status, "[parquet][arrow][file-reader][new-path]")) {
-    return gparquet_arrow_file_reader_new_raw(parquet_arrow_file_reader.release());
+  auto parquet_arrow_open_file_result = parquet::arrow::OpenFile(arrow_random_access_file,
+                                         arrow_memory_pool);
+  if (garrow::check(error, parquet_arrow_open_file_result, "[parquet][arrow][file-reader][new-path]")) {
+    return gparquet_arrow_file_reader_new_raw(parquet_arrow_open_file_result->release());
   } else {
     return NULL;
   }

[kou]

Good catch! I should have changed the GLib part too...

I've applied your patch with some modifications.

[kou]

+1

[conbench-apache-arrow]

After merging your PR, Conbench analyzed the 3 benchmarking runs that have been run so far on merge-commit 33e8cbb.

There were no benchmark performance regressions. 🎉

The full Conbench report has more details. It also includes information about 4 possible false positives for unstable benchmarks that are known to sometimes produce them.