Skip to content

GH-51780: [C++][IPC] Remove the mislabeled null check on schema fields in GetSchema - #51811

Open
CaptainAni187 wants to merge 1 commit into
apache:mainfrom
CaptainAni187:GH-51780-schema-field-null-check
Open

CaptainAni187 wants to merge 1 commit into
apache:mainfrom
CaptainAni187:GH-51780-schema-field-null-check

Conversation

@CaptainAni187

@CaptainAni187 CaptainAni187 commented Oct 6, 2026 •

Copy link
Copy Markdown

Rationale for this change

In GetSchema, the loop over the schema's fields checked each Field for null with the label "DictionaryEncoding.indexType". That label belongs to a different check, so if this one ever failed, the error would point at the wrong field. As the XXX comment next to it said, the check also can't fail: for a vector of tables, fields()->Get(i) returns the element's address plus its stored offset, never null. Schema.fields itself is already checked for null just above.

What changes are included in this PR?

The check and its comment are removed, which is the first option in the issue. Child fields in FieldFromFlatbuffer are already read through children->Get(i) without a null check, so top-level fields now match.

Are these changes tested?

No new test, since the removed check couldn't fail. The arrow-ipc-* tests pass locally.

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

@github-actions

github-actions Bot commented Oct 6, 2026

Copy link
Copy Markdown

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

@CaptainAni187
CaptainAni187 marked this pull request as ready for review October 6, 2026 01:14
@github-actions

github-actions Bot commented Oct 6, 2026

Copy link
Copy Markdown

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant