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

Open

1fanwang wants to merge 2 commits into

apache:main from

1fanwang:1fannnw/fix-pyarrow-none-segfaults

Open

GH-51043: [Python] Reject null required Arrow objects #51161
1fanwang wants to merge 2 commits into
apache:main from
1fanwang:1fannnw/fix-pyarrow-none-segfaults

Conversation

@1fanwang

@1fanwang 1fanwang commented Sep 4, 2026 •

edited

Loading

Copy link

Copy Markdown

Contributor

Rationale for this change

Passing null values to required PyArrow object arguments can dereference a null C++ pointer and terminate the Python process. These calls should reject invalid input with a type error, consistent with other typed PyArrow APIs.

Fixes #51043.

What changes are included in this PR?

Required schema, file-format, data-type, and dictionary array arguments now reject null at the Cython boundary. FileSystemDataset also rejects a null fragment before unwrapping it.

Are these changes tested?

The reported calls were run in separate Python processes against PyArrow 25.0.1, then covered by focused tests against the patched source.

Raw logs

$ python -c 'import pyarrow.parquet as pq; pq.SortingColumn.from_ordering(None, [])'
Segmentation fault: 11
$ echo $?
139
$ python -c 'import pyarrow as pa; import pyarrow.dataset as ds; ds.FileSystemDataset([None], pa.schema([]), ds.ParquetFileFormat())'
Segmentation fault: 11
$ echo $?
139
$ python -c 'import pyarrow as pa; d=pa.array([], type=pa.string()); pa.DictionaryArray.from_buffers(None, 0, [None, pa.py_buffer(b"")], d)'
Segmentation fault: 11
$ echo $?
139
$ python -c 'import pyarrow as pa; pa.DictionaryArray.from_buffers(pa.dictionary(pa.int8(), pa.string()), 1, [None, pa.py_buffer(b"\x00")], None)'
AttributeError: 'NoneType' object has no attribute 'sp_array'
$ echo $?
1
$ cd python/pyarrow/tests
$ pytest -q parquet/test_metadata.py::test_parquet_sorting_column test_dataset.py::test_filesystem_dataset test_array.py::test_dictionary_from_buffers
.... [100%]
4 passed

Are there any user-facing changes?

Yes. Invalid null arguments now raise a type error instead of crashing the interpreter or failing with an unrelated attribute error.

GitHub Issue: [Python] Several APIs segfault when required Arrow object arguments are None #51043

@1fanwang


 apacheGH-51043 : Reject null required PyArrow objects

1a331e1

Generated-by: GitHub Copilot CLI (GPT-5.6 Sol)
Signed-off-by: 1fanwang <1fannnw@gmail.com>

@1fanwang

1fanwang requested review from AlenkaF and raulcd as code owners

September 4, 2026 21:47

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.

@github-actions github-actions Bot added Component: Python awaiting review labels

Sep 4, 2026

Copilot started reviewing on behalf of 1fanwang

September 4, 2026 21:48

View session

Copilot AI reviewed

Sep 4, 2026

View reviewed changes

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.

python/pyarrow/array.pxi

@1fanwang


 apacheGH-51043 : Reject null dictionary arrays

6b429e8

Generated-by: GitHub Copilot CLI (GPT-5.6 Sol)
Signed-off-by: 1fanwang <1fannnw@gmail.com>

@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 started reviewing on behalf of 1fanwang

September 5, 2026 13:11

View session

Copilot AI reviewed

Sep 5, 2026

View reviewed changes

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

@github-actions github-actions Bot added awaiting committer review and removed awaiting review labels

Sep 8, 2026

@marinelay marinelay mentioned this pull request

Sep 8, 2026

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

Open

AlenkaF

AlenkaF reviewed

Sep 9, 2026

View reviewed changes

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

python/pyarrow/array.pxi

@staticmethod

def from_buffers(DataType type, int64_t length, buffers, Array dictionary,

int64_t null_count=-1, int64_t offset=0):

def from_buffers(DataType type not None, int64_t length, buffers,

@AlenkaF AlenkaF Sep 9, 2026

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.

Would it make sense to also update other from_buffer() methods (base Array class too)?

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

@AlenkaF AlenkaF mentioned this pull request

Sep 10, 2026

GH-51042: [Python] Reject invalid Arrow wrapper types #51163

Open

Labels

awaiting committer review Component: Python

3 participants

@1fanwang @AlenkaF

Uh oh!

Conversation

@1fanwang 1fanwang commented Sep 4, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Rationale for this change

What changes are included in this PR?

Are these changes tested?

Are there any user-facing changes?

Uh oh!

github-actions Bot commented Sep 4, 2026

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

🟡 Changes recommended

Uh oh!

Uh oh!

github-actions Bot commented Sep 5, 2026

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

🟢 Approval recommended

Uh oh!

@AlenkaF AlenkaF left a comment

Choose a reason for hiding this comment

Uh oh!

@AlenkaF AlenkaF Sep 9, 2026

Choose a reason for hiding this comment

Uh oh!

AlenkaF commented Sep 10, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

@1fanwang 1fanwang commented Sep 4, 2026 •

edited

Loading