Skip to content

GH-51043: [Python] Reject null required Arrow objects - #51161

Open
1fanwang wants to merge 4 commits into
apache:mainfrom
1fanwang:1fannnw/fix-pyarrow-none-segfaults
Open

1fanwang wants to merge 4 commits into
apache:mainfrom
1fanwang:1fannnw/fix-pyarrow-none-segfaults

Conversation

@1fanwang

@1fanwang 1fanwang commented Sep 4, 2026 •

Copy link
Copy Markdown
Contributor

Rationale for this change

Passing None for a required Arrow object can abort the Python process or raise an unrelated AttributeError. Later comments on #51043 added ScanNodeOptions, ParquetSchema, SubTreeFileSystem, ProjectNodeOptions and ColumnSchema.

Fixes #51043.

get_partition_keys(None) is in #51294.

What changes are included in this PR?

Required schema, format, type, dictionary, fragment, filesystem, and child-array arguments reject None. ScanNodeOptions, ParquetSchema, ColumnSchema, SubTreeFileSystem, and ProjectNodeOptions do the same. Array buffer constructors require a data type. String constructors keep existing buffer validation.

Are these changes tested?

A native Arrow 26.0.0-SNAPSHOT build reproduced the failures. The same commands then raised TypeError.

$ python -c 'from pyarrow import acero; acero.ScanNodeOptions(None)'
Before: process aborted (SIGBUS)
After: TypeError: Argument 'dataset' has incorrect type (expected pyarrow._dataset.Dataset, got NoneType)

$ python -c 'import pyarrow.parquet as pq; pq.ParquetSchema(None)'
Before: process aborted (SIGSEGV)
After: TypeError: Argument 'container' has incorrect type (expected pyarrow._parquet.FileMetaData, got NoneType)

$ python -c 'import pyarrow as pa; pa.Array.from_buffers(type=None, length=0, buffers=[])'
Before: AttributeError: 'NoneType' object has no attribute 'num_fields'
After: TypeError: Argument 'type' has incorrect type (expected pyarrow.lib.DataType, got NoneType)

ProjectNodeOptions([None]) and SubTreeFileSystem("", None) aborted with SIGBUS. ColumnSchema(None, 0) aborted on .name with SIGSEGV. All three now raise TypeError.

Are there any user-facing changes?

Missing required objects raise TypeError. Valid null buffers keep their existing behavior.

New Contributor's Guide |
Contributing Overview |
AI-generated Code Guidance

Was AI used for this PR?

AI assisted with the code, tests, and description.

PR code and description written by:

  • Human
  • AI

Reviewed before submission by:

  • Human
  • AI
  • Not reviewed

Copilot AI lite review requested due to automatic review settings September 4, 2026 21:47
@1fanwang
1fanwang requested a review from rok as a code owner September 4, 2026 21:47
@github-actions

github-actions Bot commented Sep 4, 2026

Copy link
Copy Markdown

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

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.

🟡 Changes recommended

DictionaryArray.from_buffers() still allows dictionary=None while dereferencing it unconditionally, so a None argument can still trigger a crash.

Once you've addressed the issues Copilot identified, you can request another Copilot review.

Pull request overview

This PR hardens several PyArrow Cython entry points by rejecting None for required Arrow extension objects, preventing null C++ pointer dereferences that can segfault the Python interpreter (GH-51043).

Changes:

  • Mark required Schema, FileFormat, and DataType parameters as non-nullable (not None) at the Cython boundary to raise TypeError instead of crashing.
  • Add an explicit None check for FileSystemDataset fragments before unwrapping.
  • Add focused regression tests covering the newly rejected None arguments.
File summaries
File Description
python/pyarrow/_dataset.pyx Reject None fragments and make schema/format non-nullable in FileSystemDataset.
python/pyarrow/_parquet.pyx Make SortingColumn conversion helpers reject schema=None at the boundary.
python/pyarrow/array.pxi Make DictionaryArray.from_buffers reject type=None at the boundary.
python/pyarrow/tests/test_dataset.py Add regression assertions for FileSystemDataset(..., schema=None/format=None) and [None] fragments.
python/pyarrow/tests/parquet/test_metadata.py Add regression assertions for SortingColumn.* with schema=None.
python/pyarrow/tests/test_array.py Add regression assertion for DictionaryArray.from_buffers(type=None, ...).
Review details
  • Files reviewed: 6/6 changed files
  • Comments generated: 1
  • Review effort level: Lite

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

Comment thread python/pyarrow/array.pxi
@github-actions

github-actions Bot commented Sep 5, 2026

Copy link
Copy Markdown

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

Copilot AI review requested due to automatic review settings September 5, 2026 13:10

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 changes directly address the null-dereference crash at the Cython boundary and are covered by targeted regression tests for the reported cases.

Review details
  • Files reviewed: 6/6 changed files
  • Comments generated: 0 new
  • Review effort level: Lite

@AlenkaF AlenkaF 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.

Are there any other instances of this pattern anywhere else in the Cython bindings? They might produce other errors (like AttributeError) but would still benefit from similar change.

Comment thread python/pyarrow/array.pxi
@AlenkaF

AlenkaF commented Sep 10, 2026

Copy link
Copy Markdown
Member

Are there any other instances of this pattern anywhere else in the Cython bindings?

Ah, ok, there is another PR aiming at other files in PyArrow: #51163

For future work we could try keeping the number of PRs down and tackle similar issues in one.

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

No unresolved issues were identified, and focused regression tests cover the changes.

Review details
  • Files reviewed: 6/6 changed files
  • Comments generated: 0 new
  • Review effort level: Lite

@github-actions

Copy link
Copy Markdown

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

1 similar comment
@github-actions

Copy link
Copy Markdown

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

Copilot AI review requested due to automatic review settings September 22, 2026 01:38

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.

Copilot review overview

🟢 Approval recommended

No unresolved review comments remain.

Review effort: Lite
Findings: None

Signed-off-by: 1fanwang <1fannnw@gmail.com>
Signed-off-by: 1fanwang <1fannnw@gmail.com>
Signed-off-by: 1fanwang <1fannnw@gmail.com>
…tors

Raise TypeError instead of aborting the interpreter when ScanNodeOptions,
ProjectNodeOptions, ParquetSchema, ColumnSchema, or SubTreeFileSystem
receive None for a required argument.

Signed-off-by: 1fanwang <1fannnw@gmail.com>
Copilot AI review requested due to automatic review settings September 24, 2026 06:42
@1fanwang
1fanwang force-pushed the 1fannnw/fix-pyarrow-none-segfaults branch from 846a3ac to bb913b6 Compare September 24, 2026 06:42

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.

Copilot review overview

🔵 Needs a closer look

RunEndEncodedArray.from_buffers still lacks validation for null child arrays and needs a regression test.

Review effort: Lite
Findings: None

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.

[Python] Several APIs segfault when required Arrow object arguments are None

3 participants