Skip to content

GH-50737: [C++][Parquet] mark MakeStatistics method without ColumnDescriptor as deprecated - #50738

Merged
wgtmac merged 1 commit into
apache:mainfrom
HuaHuaY:deprecated_make_statistics
Aug 3, 2026
Merged

GH-50737: [C++][Parquet] mark MakeStatistics method without ColumnDescriptor as deprecated#50738
wgtmac merged 1 commit into
apache:mainfrom
HuaHuaY:deprecated_make_statistics

Conversation

@HuaHuaY

@HuaHuaY HuaHuaY commented Jul 30, 2026

Copy link
Copy Markdown
Contributor

Rationale for this change

I offer two reasons for marking this method as deprecated:

  1. It accepts a distinct_count parameter but lacks has_distinct_count, making it impossible to represent missing distinct counts, which could lead to misuse.
  2. When I try to add nan_count, I found that it lacks a ColumnDescriptor parameter, making it impossible to obtain the logical type and determine the value of has_nan_count based on Parquet's logical type (FLOAT16).

What changes are included in this PR?

Mark MakeStatistics method without ColumnDescriptor as deprecated.

Are these changes tested?

Yes.

Are there any user-facing changes?

Yes.

Mark MakeStatistics method without ColumnDescriptor as deprecated.

@HuaHuaY
HuaHuaY requested review from pitrou and wgtmac as code owners July 30, 2026 10:47
@github-actions

Copy link
Copy Markdown

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

@HuaHuaY

HuaHuaY commented Jul 30, 2026

Copy link
Copy Markdown
Contributor Author

@pitrou Please take a look. What do you think about marking this method as deprecated?

@github-actions github-actions Bot added the awaiting review Awaiting review label Jul 30, 2026
@pitrou

pitrou commented Jul 30, 2026

Copy link
Copy Markdown
Member

I'm ok with it, what do you think @wgtmac @mapleFU ?

@github-actions github-actions Bot added awaiting committer review Awaiting committer review and removed awaiting review Awaiting review labels Jul 30, 2026

@wgtmac wgtmac left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Make sense. +1

@wgtmac
wgtmac merged commit 8743501 into apache:main Aug 3, 2026
59 checks passed
@wgtmac wgtmac removed the awaiting committer review Awaiting committer review label Aug 3, 2026
@HuaHuaY
HuaHuaY deleted the deprecated_make_statistics branch August 3, 2026 02:40
@conbench-apache-arrow

Copy link
Copy Markdown

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

There were no benchmark performance regressions. 🎉

The full Conbench report has more details. It also includes information about 1 possible false positive 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.

4 participants