Skip to content

GH-55124: [C++][Parquet] Allow masked nulls in required fields when writing - #55136

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

HuaHuaY wants to merge 1 commit into
apache:mainfrom
HuaHuaY:masked_null

Conversation

@HuaHuaY

@HuaHuaY HuaHuaY commented Oct 23, 2026 •

Copy link
Copy Markdown
Contributor

Rationale for this change

The Parquet writer checks the leaf array's physical null count without accounting for the parent struct's validity bitmap. As a result, it rejects nulls in required child fields even when they are masked by null parent rows.

What changes are included in this PR?

Corrected the logic used by the column writer to validate the null values for required leaf fields.

Are these changes tested?

Yes.

Are there any user-facing changes?

No.

Was AI used for this PR?

In accordance to the AI generation guidelines, please disclose below whether and how AI was used in this PR.

PR code and description written by:

  • Human
  • AI

Reviewed before submission by:

  • Human
  • AI
  • Not reviewed

Copilot AI balanced review requested due to automatic review settings October 23, 2026 11:00
@HuaHuaY
HuaHuaY requested review from pitrou and wgtmac as code owners October 23, 2026 11:00
@github-actions github-actions Bot added the awaiting review Awaiting review label Oct 23, 2026

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🟢 Approval recommended

The focused validation change follows definition-level semantics and includes regression coverage for both accepted and rejected cases.

No open issues 🎉

What changed in this PR

Fixes Parquet writing of required child fields whose null values are masked by null ancestors, addressing GH-55124 while preserving rejection of unmasked nulls.

Changes:

  • Validate required leaf nulls against ancestor definition levels.
  • Preserve definition levels for required leaves so invalid nulls remain detectable.
  • Add regression coverage for masked nulls, null and empty lists, and invalid unmasked nulls.
File Description
cpp/​src/​parquet/​column_writer.cc Distinguishes masked from unmasked required-leaf nulls.
cpp/​src/​parquet/​arrow/​path_internal.cc Prevents required-leaf nulls from lowering definition levels.
cpp/​src/​parquet/​arrow/​arrow_reader_writer_test.cc Tests valid masked nulls and rejection of unmasked nulls.

Review effort: Balanced

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