GH-51228: [Python] Raise instead of crashing on an invalid registry by 1fanwang · Pull Request #51247 · apache/arrow

1fanwang · 2026-09-08T19:52:37Z

Rationale for this change

Passing an invalid registry to a tabular call or UDF registration can terminate Python. Applications should receive TypeError instead.

Fixes #51228.

What changes are included in this PR?

Validate the registry before reading its native pointer. The shared registration check covers scalar, vector, aggregate, and tabular functions. Default and explicit registries retain their behavior.

Are these changes tested?

Built from source on macOS arm64 with Python 3.12.14. Six invalid calls crashed in separate processes on upstream main, then raised TypeError after the fix. Default and explicit registries still returned the same one-row table.

Raw logs

$ python -c "import pyarrow as pa; import pyarrow.compute as pc; pc.call_tabular_function('', None, 0.0)"
Before:
tabular_call_float: returncode=-10
After:
TypeError: func_registry must be a FunctionRegistry
tabular_call_float: returncode=1

$ python -c "import pyarrow as pa; import pyarrow.compute as pc; pc.register_scalar_function(func=lambda context: None, function_name='example', function_doc={'summary': '', 'description': ''}, in_types={}, out_type=pa.struct([]), func_registry=1)"
Before:
scalar: returncode=-11
After:
TypeError: func_registry must be a FunctionRegistry
scalar: returncode=1

$ python -m pytest pyarrow/tests/test_compute.py pyarrow/tests/test_udf.py pyarrow/tests/test_csv.py::TestThreadedCSVTableRead::test_cancellation -q --tb=short --basetemp=build/pytest-51247
652 passed, 4 skipped, 7 warnings in 3.35s

Are there any user-facing changes?

Invalid registry arguments raise TypeError instead of terminating Python.

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 Issue: [Python] call_tabular_function() segfaults for some wrong typed func_registry values #51228

github-actions · 2026-09-08T19:53:37Z

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

Copilot

🟡 Changes recommended

The new subprocess-based test should be marked with @pytest.mark.processes so it can be skipped on platforms/configurations where process creation is disabled.

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

Pull request overview

Fixes a crash in the PyArrow compute bindings by validating the optional func_registry argument passed to pyarrow.compute.call_tabular_function() before performing the Cython cast, so invalid types raise a catchable TypeError instead of segfaulting.

Changes:

Add a runtime type check for func_registry in call_tabular_function() and raise TypeError on invalid values.
Add a regression test that runs the call in a subprocess to assert the failure mode is a Python exception (not a process crash).

File summaries

File	Description
python/pyarrow/_compute.pyx	Adds `func_registry` type validation before casting to a native registry pointer.
python/pyarrow/tests/test_compute.py	Adds a subprocess-based regression test to ensure invalid registries raise `TypeError` rather than crashing.

Review details

Files reviewed: 2/2 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.

Copilot

🟢 Approval recommended

The fix is narrowly scoped, matches the stated user-facing behavior, and is covered by a regression test that would fail under the prior segfaulting implementation.

Review details

Files reviewed: 2/2 changed files
Comments generated: 0 new
Review effort level: Lite

AlenkaF

Would it make sense to also add a check in _register_user_defined_function as every register_* function uses that?

Copilot

🟡 Changes recommended

The added regression test should be executed in a subprocess (as described) to remain safe if the segfault regresses and to avoid crashing the test runner.

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

Review details

Files reviewed: 2/2 changed files
Comments generated: 1
Review effort level: Lite

AlenkaF · 2026-09-11T06:52:38Z

Two things left:

comment GH-51228: [Python] Raise instead of crashing on an invalid registry #51247 (review) is not yet addressed
PR description needs an update on the testing section.

Copilot

🟢 Approval recommended

The fix is covered by regression tests and prevents the reported process crash.

Review details

Files reviewed: 2/2 changed files
Comments generated: 0 new
Review effort level: Lite

github-actions · 2026-09-12T05:01:25Z

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

1fanwang · 2026-09-12T05:05:04Z

Done in 388ac7b.

github-actions · 2026-09-22T01:25:27Z

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

github-actions · 2026-09-22T02:02:34Z

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

Copilot

Copilot review overview

🟢 Approval recommended

The validation consistently addresses the reported crash path and is covered by regression tests.

Review effort: Lite
Findings: None

Copilot

Copilot review overview

🟢 Approval recommended

The crash fix and regression coverage are complete, with no unresolved review issues.

Review effort: Lite
Findings: None

Validate call_tabular_function registry inputs before casting them to the native FunctionRegistry pointer. Signed-off-by: Stefan Wang <1fannnw@gmail.com> Signed-off-by: 1fanwang <1fannnw@gmail.com>

The test spawns a subprocess, which Emscripten does not support. Without the marker the test runs there anyway and fails. Signed-off-by: 1fanwang <1fannnw@gmail.com>

The subprocess wrapper guarded against the crash this change removes, so the in-process form reads better now. Signed-off-by: 1fanwang <1fannnw@gmail.com>

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

Copilot

Copilot review overview

🟢 Approval recommended

Invalid registries are validated and regression coverage is included.

Review effort: Lite
Findings: None

github-actions · 2026-09-25T20:53:59Z

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

Copilot AI lite review requested due to automatic review settings September 8, 2026 19:52

1fanwang requested review from AlenkaF, raulcd and rok as code owners September 8, 2026 19:52

github-actions Bot added the awaiting review Awaiting review label Sep 8, 2026

github-actions Bot added the Component: Python label Sep 8, 2026

Copilot started reviewing on behalf of 1fanwang September 8, 2026 19:53 View session

Copilot AI reviewed Sep 8, 2026

View reviewed changes

Comment thread python/pyarrow/tests/test_compute.py

Copilot AI review requested due to automatic review settings September 8, 2026 22:39

Copilot started reviewing on behalf of 1fanwang September 8, 2026 22:40 View session

github-actions Bot added awaiting committer review Awaiting committer review and removed awaiting review Awaiting review labels Sep 8, 2026

Copilot AI reviewed Sep 8, 2026

View reviewed changes

AlenkaF reviewed Sep 9, 2026

View reviewed changes

AlenkaF reviewed Sep 10, 2026

View reviewed changes

Comment thread python/pyarrow/tests/test_compute.py

Copilot AI review requested due to automatic review settings September 10, 2026 11:54

Copilot started reviewing on behalf of 1fanwang September 10, 2026 11:55 View session

Copilot AI reviewed Sep 10, 2026

View reviewed changes

Comment thread python/pyarrow/tests/test_compute.py

Copilot AI review requested due to automatic review settings September 12, 2026 04:57

Copilot started reviewing on behalf of 1fanwang September 12, 2026 04:58 View session

Copilot AI reviewed Sep 12, 2026

View reviewed changes

Copilot AI review requested due to automatic review settings September 22, 2026 02:08

Copilot started reviewing on behalf of 1fanwang September 22, 2026 02:08 View session

Copilot AI reviewed Sep 22, 2026

View reviewed changes

Copilot AI review requested due to automatic review settings September 24, 2026 06:16

1fanwang force-pushed the 1fannnw/gh-51228-call-tabular-registry branch from 10eed8b to 80261c3 Compare September 24, 2026 06:16

Copilot started reviewing on behalf of 1fanwang September 24, 2026 06:16 View session

Copilot AI reviewed Sep 24, 2026

View reviewed changes

1fanwang added 4 commits September 25, 2026 13:51

apacheGH-51228: [Python] Reject invalid tabular function registry

49757dc

Validate call_tabular_function registry inputs before casting them to the native FunctionRegistry pointer. Signed-off-by: Stefan Wang <1fannnw@gmail.com> Signed-off-by: 1fanwang <1fannnw@gmail.com>

apacheGH-51228: Mark the subprocess test with pytest.mark.processes

590a742

The test spawns a subprocess, which Emscripten does not support. Without the marker the test runs there anyway and fails. Signed-off-by: 1fanwang <1fannnw@gmail.com>

apacheGH-51228: Assert the type error with pytest.raises

29192c5

The subprocess wrapper guarded against the crash this change removes, so the in-process form reads better now. Signed-off-by: 1fanwang <1fannnw@gmail.com>

apacheGH-51228: [Python] Reject invalid UDF registration registries

000992c

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

1fanwang force-pushed the 1fannnw/gh-51228-call-tabular-registry branch from 80261c3 to 000992c Compare September 25, 2026 20:51

Copilot AI review requested due to automatic review settings September 25, 2026 20:51

Copilot started reviewing on behalf of 1fanwang September 25, 2026 20:51 View session

Copilot AI reviewed Sep 25, 2026

View reviewed changes

Conversation

1fanwang commented Sep 8, 2026 • edited by github-actions Bot 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?

Was AI used for this PR?

Uh oh!

github-actions Bot commented Sep 8, 2026

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

🟡 Changes recommended

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

🟢 Approval recommended

Uh oh!

AlenkaF left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

🟡 Changes recommended

Uh oh!

Uh oh!

AlenkaF commented Sep 11, 2026

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

🟢 Approval recommended

Uh oh!

github-actions Bot commented Sep 12, 2026

Uh oh!

1fanwang commented Sep 12, 2026

Uh oh!

github-actions Bot commented Sep 22, 2026

Uh oh!

github-actions Bot commented Sep 22, 2026

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Copilot review overview

🟢 Approval recommended

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Copilot review overview

🟢 Approval recommended

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Copilot review overview

🟢 Approval recommended

Uh oh!

github-actions Bot commented Sep 25, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

1fanwang commented Sep 8, 2026 •

edited by github-actions Bot

Loading