Skip to content

GH-44810: [C++][Parquet] Add arrow::Result version of parquet::arrow::FileReader::Make()#48285

Merged
kou merged 24 commits intoapache:mainfrom
hiroyuki-sato:topic/parquet-result-reader
Dec 11, 2025
Merged

GH-44810: [C++][Parquet] Add arrow::Result version of parquet::arrow::FileReader::Make()#48285
kou merged 24 commits intoapache:mainfrom
hiroyuki-sato:topic/parquet-result-reader

Conversation

@hiroyuki-sato
Copy link
Copy Markdown
Collaborator

@hiroyuki-sato hiroyuki-sato commented Nov 28, 2025

Rationale for this change

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

What changes are included in this PR?

Introduce a Result<std::unique_ptr<FileReader>> returning API to allow clearer error propagation

Are these changes tested?

Yes.

Are there any user-facing changes?

Yes.

Status version FileReader::Make has been deprecated.

  static ::arrow::Status Make(::arrow::MemoryPool* pool,
                              std::unique_ptr<ParquetFileReader> reader,
                              const ArrowReaderProperties& properties,
                              std::unique_ptr<FileReader>* out);

  static ::arrow::Status Make(::arrow::MemoryPool* pool,
                              std::unique_ptr<ParquetFileReader> reader,
                              std::unique_ptr<FileReader>* out);

@github-actions
Copy link
Copy Markdown

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

@github-actions github-actions bot added awaiting changes Awaiting changes awaiting change review Awaiting change review and removed awaiting review Awaiting review awaiting changes Awaiting changes awaiting change review Awaiting change review labels Nov 28, 2025
@hiroyuki-sato hiroyuki-sato force-pushed the topic/parquet-result-reader branch from 86eba06 to 7ef530c Compare November 29, 2025 12:47
@github-actions github-actions bot added awaiting change review Awaiting change review and removed awaiting changes Awaiting changes labels Nov 29, 2025
Copy link
Copy Markdown
Collaborator Author

@hiroyuki-sato hiroyuki-sato left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

@github-actions github-actions bot added awaiting changes Awaiting changes awaiting change review Awaiting change review and removed awaiting change review Awaiting change review awaiting changes Awaiting changes labels Dec 1, 2025
@github-actions github-actions bot added awaiting changes Awaiting changes Component: Gandiva awaiting change review Awaiting change review and removed awaiting change review Awaiting change review awaiting changes Awaiting changes labels Dec 2, 2025
@hiroyuki-sato hiroyuki-sato force-pushed the topic/parquet-result-reader branch 2 times, most recently from 39cc9ea to 45249dd Compare December 8, 2025 00:50
@hiroyuki-sato hiroyuki-sato force-pushed the topic/parquet-result-reader branch from f1abda7 to 4045dc8 Compare December 9, 2025 05:20
@hiroyuki-sato
Copy link
Copy Markdown
Collaborator Author

@kou Thank you for your review. I add auto parameter. Please take a look when you get a chance.

@github-actions github-actions bot added awaiting changes Awaiting changes awaiting change review Awaiting change review and removed awaiting change review Awaiting change review awaiting changes Awaiting changes labels Dec 11, 2025
Copy link
Copy Markdown
Member

@kou kou left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

+1

@kou kou merged commit 7a36fcc into apache:main Dec 11, 2025
44 of 45 checks passed
@kou kou removed the awaiting change review Awaiting change review label Dec 11, 2025
@github-actions github-actions bot added the awaiting merge Awaiting merge label Dec 11, 2025
@hiroyuki-sato hiroyuki-sato deleted the topic/parquet-result-reader branch December 11, 2025 12:09
@conbench-apache-arrow
Copy link
Copy Markdown

After merging your PR, Conbench analyzed the 2 benchmarking runs that have been run so far on merge-commit 7a36fcc.

There weren't enough matching historic benchmark results to make a call on whether there were regressions.

The full Conbench report has more details.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants