GH-48394: [C++][Parquet] Add arrow::Result version of parquet::arrow::FileReader::ReadTable() by fenfeng9 · Pull Request #48939 · apache/arrow

fenfeng9 · 2026-01-22T07:38:51Z

Rationale for this change

FileReader::ReadTable previously returned Status and required callers to pass an out parameter.

What changes are included in this PR?

Introduce Result<std::shared_ptr<Table>> returning API to allow clearer error propagation:

Add new Result-returning ReadTable() methods
Deprecate the old Status versions
Migrate all callers to use the new API

Are these changes tested?

Yes.

Are there any user-facing changes?

Yes.

Status version FileReader::ReadTable has been deprecated.

virtual ::arrow::Status ReadTable(std::shared_ptr<::arrow::Table>* out);

virtual ::arrow::Status ReadTable(const std::vector<int>& column_indices,
                                  std::shared_ptr<::arrow::Table>* out);

GitHub Issue: [C++][Parquet] Add arrow::Result version of parquet::arrow::FileReader::ReadTable() #48394

github-actions · 2026-01-22T07:39:16Z

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

fenfeng9 · 2026-01-22T07:43:30Z

Hi @kou, Could you please review it? Thanks!

cpp/examples/arrow/parquet_read_write.cc

kou · 2026-01-22T07:57:33Z

cpp/examples/tutorial_examples/file_access_example.cc

-  std::shared_ptr<arrow::Table> parquet_table;
  // Read the table.
-  PARQUET_THROW_NOT_OK(reader->ReadTable(&parquet_table));
+  PARQUET_ASSIGN_OR_THROW(auto parquet_table, reader->ReadTable());


Could you keep using explicit type instead of auto for easy to understand?

kou · 2026-01-22T07:59:38Z

cpp/src/arrow/filesystem/s3fs_benchmark.cc

+      auto table_result = reader->ReadTable(column_indices);
+      ASSERT_OK(table_result.status());


Suggested change

auto table_result = reader->ReadTable(column_indices);

ASSERT_OK(table_result.status());

ASSERT_OK_AND_ASSIGN(auto table, reader->ReadTable(column_indices));

If this reports the "variable is not used" warning, we can use ASSERT_OK(reader->ReadTable(column_indices).status());

kou · 2026-01-22T08:02:19Z

cpp/src/parquet/arrow/arrow_reader_writer_test.cc

    std::shared_ptr<Table> result;
-    ASSERT_OK(reader->ReadTable(column_subset, &result));
+    ASSERT_OK_AND_ASSIGN(result, reader->ReadTable(column_subset));


Could you use ASSERT_OK_AND_ASSIGN(auto result, reader->ReadTable(column_subset)); here?

kou · 2026-01-22T08:02:32Z

cpp/src/parquet/arrow/arrow_reader_writer_test.cc

  std::shared_ptr<Table> result;
-  ASSERT_OK_NO_THROW(reader->ReadTable(&result));
+  ASSERT_OK_AND_ASSIGN(result, reader->ReadTable());


Ditto. (There are more similar codes.)

kou · 2026-01-22T08:05:30Z

cpp/src/parquet/arrow/reader.cc

+    RETURN_NOT_OK(ReadRowGroups(Iota(reader_->metadata()->num_row_groups()),
+                                column_indices, &table));


Oh... We need to add arrow::Result version of ReadRowGroups()...

Let's work on it as a separated task. Could you open an issue for it?

Sure, I'll open a new issue for the ReadRowGroups refactoring. I'll get to the code updates soon.

fenfeng9 · 2026-01-22T11:26:50Z

I've created #48949 to track add arrow::Result version of ReadRowGroups()...

…Table

Co-authored-by: Sutou Kouhei <kou@cozmixng.org>

fenfeng9 · 2026-01-23T09:06:03Z

Hi, @kou, I have a question: For R / Python / GLib bindings, can we switch directly to the new API?

kou · 2026-01-23T09:12:14Z

Yes. If you can't do it, I can help it.

fenfeng9 · 2026-01-23T09:15:55Z

Thanks! I’ll try it first.

…o Result API

fenfeng9 · 2026-01-23T13:39:44Z

@kou CI passed, Please take a look when you get a chance.

kou

+1

conbench-apache-arrow · 2026-01-24T02:22:57Z

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

There was 1 benchmark result with an error:

Commit Run on arm64-t4g-2xlarge-linux at 2026-01-24 00:25:27Z
- dataset-read (Python) with async=True, dataset=nyctaxi_multi_ipc_s3

There were no benchmark performance regressions. 🎉

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

fenfeng9 requested a review from wgtmac as a code owner January 22, 2026 07:38

github-actions bot added Component: Parquet Component: C++ awaiting review Awaiting review labels Jan 22, 2026

kou reviewed Jan 22, 2026

View reviewed changes

fenfeng9 force-pushed the refactor/parquet-readtable-result branch from afe13a4 to cf88ec1 Compare January 22, 2026 10:57

fenfeng9 requested a review from kou January 22, 2026 11:38

github-actions bot added awaiting committer review Awaiting committer review and removed awaiting review Awaiting review labels Jan 23, 2026

fenfeng9 and others added 3 commits January 23, 2026 13:59

apacheGH-48394: [C++][Parquet] Add Result version of FileReader::Read…

b81b631

…Table

Update cpp/examples/arrow/parquet_read_write.cc

d204bbd

Co-authored-by: Sutou Kouhei <kou@cozmixng.org>

address review comments for apacheGH-48394

dddef6d

apacheGH-48394: [R][Python][GLib] Switch Parquet ReadTable bindings t…

6ae9eb5

…o Result API

wgtmac approved these changes Jan 23, 2026

View reviewed changes

fenfeng9 force-pushed the refactor/parquet-readtable-result branch from cf88ec1 to 6ae9eb5 Compare January 23, 2026 12:28

fenfeng9 requested review from AlenkaF, jonkeane, raulcd, rok and thisisnic as code owners January 23, 2026 12:28

github-actions bot added Component: R Component: Python Component: GLib labels Jan 23, 2026

kou approved these changes Jan 23, 2026

View reviewed changes

kou merged commit 75ef031 into apache:main Jan 23, 2026
51 checks passed

kou removed the awaiting committer review Awaiting committer review label Jan 23, 2026

kou mentioned this pull request Jan 23, 2026

[C++][Parquet] Add arrow::Result version of parquet::arrow::FileReader::ReadTable() #48394

Closed

		auto table_result = reader->ReadTable(column_indices);
		ASSERT_OK(table_result.status());

	auto table_result = reader->ReadTable(column_indices);
	ASSERT_OK(table_result.status());
	ASSERT_OK_AND_ASSIGN(auto table, reader->ReadTable(column_indices));

		RETURN_NOT_OK(ReadRowGroups(Iota(reader_->metadata()->num_row_groups()),
		column_indices, &table));

Conversation

fenfeng9 commented Jan 22, 2026 • edited by github-actions bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Rationale for this change

What changes are included in this PR?

Are these changes tested?

Are there any user-facing changes?

Uh oh!

github-actions bot commented Jan 22, 2026

Uh oh!

fenfeng9 commented Jan 22, 2026

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

fenfeng9 commented Jan 22, 2026

Uh oh!

fenfeng9 commented Jan 23, 2026

Uh oh!

kou commented Jan 23, 2026

Uh oh!

fenfeng9 commented Jan 23, 2026

Uh oh!

fenfeng9 commented Jan 23, 2026

Uh oh!

kou left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

conbench-apache-arrow bot commented Jan 24, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

fenfeng9 commented Jan 22, 2026 •

edited by github-actions bot

Loading