Skip to content

GH-50678: [C++][Parquet] Remove unused member null_slot_usage in struct LevelInfo - #50679

Merged
pitrou merged 2 commits into
apache:mainfrom
HuaHuaY:remove_null_slot_usage
Jul 28, 2026
Merged

GH-50678: [C++][Parquet] Remove unused member null_slot_usage in struct LevelInfo#50679
pitrou merged 2 commits into
apache:mainfrom
HuaHuaY:remove_null_slot_usage

Conversation

@HuaHuaY

@HuaHuaY HuaHuaY commented Jul 28, 2026

Copy link
Copy Markdown
Contributor

Rationale for this change

Remove unused member null_slot_usage in struct LevelInfo which is in the parquet::internal namespace.

What changes are included in this PR?

Remove member null_slot_usage in struct LevelInfo.

Are these changes tested?

Yes.

Are there any user-facing changes?

No.

@HuaHuaY
HuaHuaY requested review from pitrou and wgtmac as code owners July 28, 2026 07:58
@github-actions

Copy link
Copy Markdown

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

Comment on lines -122 to -126
if (output->null_count > 0 && level_info.null_slot_usage > 1) {
throw ParquetException(
"Null values with null_slot_usage > 1 not supported."
"(i.e. FixedSizeLists with null values are not supported)");
}

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.

Are we ok removing this? Is it handled elsewhere?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

In the current code, null_slot_usage is not assigned a value, so it will never be greater than 1.

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.

Oh, I see. Thanks!

Comment thread cpp/src/parquet/level_conversion.h Outdated
@HuaHuaY

HuaHuaY commented Jul 28, 2026

Copy link
Copy Markdown
Contributor Author

I forgot I had written this comment #48587 (comment). Some CI compilers will reject the <format> header.

@pitrou pitrou removed CI: Extra: C++ Run extra C++ CI CI: Extra: R Run extra R CI labels Jul 28, 2026
@github-actions github-actions Bot added the awaiting review Awaiting review label Jul 28, 2026
@github-actions github-actions Bot added awaiting committer review Awaiting committer review and removed awaiting review Awaiting review labels Jul 28, 2026
@pitrou

pitrou commented Jul 28, 2026

Copy link
Copy Markdown
Member

CI failures are unrelated. Thank you @HuaHuaY !

@pitrou
pitrou merged commit 6f6051b into apache:main Jul 28, 2026
53 of 57 checks passed
@pitrou pitrou removed the awaiting committer review Awaiting committer review label Jul 28, 2026
@HuaHuaY
HuaHuaY deleted the remove_null_slot_usage branch July 28, 2026 08:57
@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 6f6051b.

There were no benchmark performance regressions. 🎉

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