Skip to content

GH-49539: [C++][Parquet] Fix argument count check in parquet_scan#49540

Merged
raulcd merged 1 commit intoapache:mainfrom
domibel:domibel-patch-fix-parquet_scan
Apr 7, 2026
Merged

GH-49539: [C++][Parquet] Fix argument count check in parquet_scan#49540
raulcd merged 1 commit intoapache:mainfrom
domibel:domibel-patch-fix-parquet_scan

Conversation

@domibel
Copy link
Copy Markdown
Contributor

@domibel domibel commented Mar 18, 2026

Rationale for this change

Running parquet-scan without arguments currently triggers a confusing IOError instead of showing how to use the tool.

What changes are included in this PR?

Updated the argument validation from argc < 1 (always false) to argc < 2.

Are these changes tested?

Yes.

Are there any user-facing changes?

Yes.

Old behavior:
$ parquet-scan
Parquet error: IOError: Failed to open local file ''. Detail: [errno 2] No such file or directory

New behavior:
$ parquet-scan
Usage: parquet-scan [--batch-size=] [--columns=...]

@github-actions
Copy link
Copy Markdown

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

@raulcd raulcd changed the title GH-49539: [C++] Fix argument count check in parquet_scan GH-49539: [C++][Parquet] Fix argument count check in parquet_scan Mar 18, 2026
Copy link
Copy Markdown
Member

@raulcd raulcd left a comment

Choose a reason for hiding this comment

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

Thanks for the PR! Looks good to me, I've triggered CI. Instead of removing the PR template, could you fill it with the required sections? The message goes directly into the commit history and it's helpful for us to be consistent.

@github-actions github-actions bot added awaiting changes Awaiting changes and removed awaiting review Awaiting review labels Mar 26, 2026
@domibel
Copy link
Copy Markdown
Contributor Author

domibel commented Apr 3, 2026

The PR description has been updated.

Copy link
Copy Markdown
Member

@raulcd raulcd left a comment

Choose a reason for hiding this comment

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

Thanks!

@raulcd raulcd merged commit da6870d into apache:main Apr 7, 2026
53 checks passed
@raulcd raulcd removed the awaiting changes Awaiting changes label Apr 7, 2026
@conbench-apache-arrow
Copy link
Copy Markdown

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

There were no benchmark performance regressions. 🎉

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

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