Skip to content

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

Open
HuaHuaY wants to merge 1 commit into
apache:mainfrom
HuaHuaY:deprecated_make_statistics
Open

GH-50737: [C++][Parquet] mark MakeStatistics method without ColumnDescriptor as deprecated#50738
HuaHuaY wants to merge 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

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.

3 participants