From 64adf4dfa9a2ebf692edf3816987683b38edb00f Mon Sep 17 00:00:00 2001 From: 1fanwang <1fannnw@gmail.com> Date: Thu, 10 Sep 2026 11:07:03 -0700 Subject: [PATCH 1/6] GH-51293: [Python] Reject a null partition expression get_partition_keys took an Expression without rejecting None, so passing None dereferenced a null pointer and terminated the interpreter. Signed-off-by: 1fanwang <1fannnw@gmail.com> --- python/pyarrow/_dataset.pyx | 2 +- python/pyarrow/tests/test_dataset.py | 3 +++ 2 files changed, 4 insertions(+), 1 deletion(-) diff --git a/python/pyarrow/_dataset.pyx b/python/pyarrow/_dataset.pyx index d40614a61fc0..f5666d69e1dd 100644 --- a/python/pyarrow/_dataset.pyx +++ b/python/pyarrow/_dataset.pyx @@ -4052,7 +4052,7 @@ cdef class Scanner(_Weakrefable): return reader -def get_partition_keys(Expression partition_expression): +def get_partition_keys(Expression partition_expression not None): """ Extract partition keys (equality constraints between a field and a scalar) from an expression as a dict mapping the field's name to its value. diff --git a/python/pyarrow/tests/test_dataset.py b/python/pyarrow/tests/test_dataset.py index 0a94c0bd9875..707d937da201 100644 --- a/python/pyarrow/tests/test_dataset.py +++ b/python/pyarrow/tests/test_dataset.py @@ -946,6 +946,9 @@ def test_partition_keys(): null = ds.field('a').is_null() assert ds.get_partition_keys(null) == {'a': None} + with pytest.raises(TypeError, match="Argument 'partition_expression'"): + ds.get_partition_keys(None) + @pytest.mark.parquet def test_parquet_read_options(): From aebbc7d09d2893468f7cae34643a52641c51a1a9 Mon Sep 17 00:00:00 2001 From: 1fanwang <1fannnw@gmail.com> Date: Thu, 10 Sep 2026 11:50:19 -0700 Subject: [PATCH 2/6] GH-51293: [Python] Add a guard against None crashing public callables Sweeps public module-level callables with None in a child process and fails with the offending name if the interpreter dies by signal, so a typed Cython parameter missing "not None" is caught as a test failure rather than a segfault in someone else's code. Signed-off-by: 1fanwang <1fannnw@gmail.com> --- python/pyarrow/tests/test_misc.py | 47 +++++++++++++++++++++++++++++++ 1 file changed, 47 insertions(+) diff --git a/python/pyarrow/tests/test_misc.py b/python/pyarrow/tests/test_misc.py index 856873f441b4..4ac393c1a225 100644 --- a/python/pyarrow/tests/test_misc.py +++ b/python/pyarrow/tests/test_misc.py @@ -270,3 +270,50 @@ def test_extension_type_constructor_errors(klass): msg = f"Do not call {klass.__name__}'s constructor directly, use .* instead." with pytest.raises(TypeError, match=msg): klass() + + +@pytest.mark.processes +def test_public_callables_reject_none_without_crashing(): + # GH-51293: a typed Cython parameter that is not declared "not None" + # lets None reach code that dereferences it, killing the interpreter + # instead of raising. Passing None to any public callable must produce + # a Python exception, never a fatal signal. + code = """if 1: + import importlib, inspect + mods = ["pyarrow", "pyarrow.compute", "pyarrow.dataset", + "pyarrow.parquet", "pyarrow.fs", "pyarrow.ipc", + "pyarrow.csv", "pyarrow.json", "pyarrow.feather"] + names = [] + for mn in mods: + try: + m = importlib.import_module(mn) + except Exception: + continue + for n in dir(m): + if n.startswith("_"): + continue + try: + obj = getattr(m, n) + except Exception: + continue + if callable(obj) and not inspect.isclass(obj): + names.append((mn, n)) + names.sort() + for mn, n in names: + print("%s.%s" % (mn, n), flush=True) + try: + getattr(importlib.import_module(mn), n)(None) + except BaseException: + pass + print("DONE", flush=True) + """ + res = subprocess.run([sys.executable, "-c", code], + capture_output=True, text=True) + lines = res.stdout.splitlines() + if not lines or lines[-1] != "DONE": + culprit = lines[-1] if lines else "" + raise AssertionError( + f"passing None to {culprit} terminated the interpreter " + f"(returncode {res.returncode}); declare its typed parameter " + f"'not None'" + ) From fdd398ae287fd754ab6d3931de43e17781d5533c Mon Sep 17 00:00:00 2001 From: 1fanwang <1fannnw@gmail.com> Date: Thu, 10 Sep 2026 13:17:33 -0700 Subject: [PATCH 3/6] GH-51293: [Python] Reject a null Expression in Expression.equals The sweep in the previous commit only covered module-level functions. Extending it to methods reached from a live object found Expression.equals taking a typed Expression without rejecting None, which segfaulted. Signed-off-by: 1fanwang <1fannnw@gmail.com> --- python/pyarrow/_compute.pyx | 2 +- python/pyarrow/tests/test_misc.py | 67 +++++++++++++++++++++++++++++++ 2 files changed, 68 insertions(+), 1 deletion(-) diff --git a/python/pyarrow/_compute.pyx b/python/pyarrow/_compute.pyx index 1c0c779f6037..b166bc9bf7cd 100644 --- a/python/pyarrow/_compute.pyx +++ b/python/pyarrow/_compute.pyx @@ -2708,7 +2708,7 @@ cdef class Expression(_Weakrefable): cdef inline CExpression unwrap(self): return self.expr - def equals(self, Expression other): + def equals(self, Expression other not None): """ Parameters ---------- diff --git a/python/pyarrow/tests/test_misc.py b/python/pyarrow/tests/test_misc.py index 4ac393c1a225..8774778a7207 100644 --- a/python/pyarrow/tests/test_misc.py +++ b/python/pyarrow/tests/test_misc.py @@ -317,3 +317,70 @@ def test_public_callables_reject_none_without_crashing(): f"(returncode {res.returncode}); declare its typed parameter " f"'not None'" ) + + +@pytest.mark.processes +def test_public_methods_reject_none_without_crashing(): + # Same contract as the function sweep above, applied to methods reached + # from a live object. Expression.equals crashed this way before GH-51293. + code = """if 1: + import inspect + import pyarrow as pa + import pyarrow.dataset as ds + + tbl = pa.table({"a": [1, 2], "b": ["x", "y"]}) + instances = [ + ("Table", tbl), + ("RecordBatch", tbl.to_batches()[0]), + ("Array", pa.array([1, 2, 3])), + ("ChunkedArray", tbl.column("a")), + ("Schema", tbl.schema), + ("Field", tbl.schema.field(0)), + ("DataType", pa.int64()), + ("Scalar", pa.scalar(1)), + ("Dataset", ds.dataset(tbl)), + ("Expression", ds.field("a")), + ("Buffer", pa.py_buffer(b"abc")), + ] + targets = [] + for label, obj in instances: + for name in dir(obj): + if name.startswith("_"): + continue + try: + attr = getattr(obj, name) + except BaseException: + continue + if not callable(attr): + continue + try: + sig = inspect.signature(attr) + except BaseException: + continue + required = [ + p for p in sig.parameters.values() + if p.default is inspect._empty + and p.kind in (p.POSITIONAL_ONLY, p.POSITIONAL_OR_KEYWORD) + ] + if len(required) == 1: + targets.append((label, name)) + targets.sort() + lookup = dict(instances) + for label, name in targets: + print("%s.%s" % (label, name), flush=True) + try: + getattr(lookup[label], name)(None) + except BaseException: + pass + print("DONE", flush=True) + """ + res = subprocess.run([sys.executable, "-c", code], + capture_output=True, text=True) + lines = res.stdout.splitlines() + if not lines or lines[-1] != "DONE": + culprit = lines[-1] if lines else "" + raise AssertionError( + f"passing None to {culprit} terminated the interpreter " + f"(returncode {res.returncode}); declare its typed parameter " + f"'not None'" + ) From 1e54f895ae944eedc5ad09dbf49c61fdec4415be Mon Sep 17 00:00:00 2001 From: 1fanwang <1fannnw@gmail.com> Date: Thu, 10 Sep 2026 13:26:49 -0700 Subject: [PATCH 4/6] GH-51293: [Python] Bound the sweeps and state the invariant precisely Add a timeout so a blocking callable cannot hang CI, fail on a non-zero returncode so a crash during interpreter shutdown is caught, and correct the comment: some public APIs accept None, so the invariant is only that none of them terminate the interpreter. Signed-off-by: 1fanwang <1fannnw@gmail.com> --- python/pyarrow/tests/test_misc.py | 13 +++++++------ 1 file changed, 7 insertions(+), 6 deletions(-) diff --git a/python/pyarrow/tests/test_misc.py b/python/pyarrow/tests/test_misc.py index 8774778a7207..f2d14734d31d 100644 --- a/python/pyarrow/tests/test_misc.py +++ b/python/pyarrow/tests/test_misc.py @@ -276,8 +276,9 @@ def test_extension_type_constructor_errors(klass): def test_public_callables_reject_none_without_crashing(): # GH-51293: a typed Cython parameter that is not declared "not None" # lets None reach code that dereferences it, killing the interpreter - # instead of raising. Passing None to any public callable must produce - # a Python exception, never a fatal signal. + # instead of raising. Some public APIs accept None legitimately, so the + # invariant asserted here is only that none of them terminate the + # interpreter. code = """if 1: import importlib, inspect mods = ["pyarrow", "pyarrow.compute", "pyarrow.dataset", @@ -308,9 +309,9 @@ def test_public_callables_reject_none_without_crashing(): print("DONE", flush=True) """ res = subprocess.run([sys.executable, "-c", code], - capture_output=True, text=True) + capture_output=True, text=True, timeout=300) lines = res.stdout.splitlines() - if not lines or lines[-1] != "DONE": + if res.returncode != 0 or not lines or lines[-1] != "DONE": culprit = lines[-1] if lines else "" raise AssertionError( f"passing None to {culprit} terminated the interpreter " @@ -375,9 +376,9 @@ def test_public_methods_reject_none_without_crashing(): print("DONE", flush=True) """ res = subprocess.run([sys.executable, "-c", code], - capture_output=True, text=True) + capture_output=True, text=True, timeout=300) lines = res.stdout.splitlines() - if not lines or lines[-1] != "DONE": + if res.returncode != 0 or not lines or lines[-1] != "DONE": culprit = lines[-1] if lines else "" raise AssertionError( f"passing None to {culprit} terminated the interpreter " From 35d2142c1eb3600f783b8de2c4caebb72efa271b Mon Sep 17 00:00:00 2001 From: 1fanwang <1fannnw@gmail.com> Date: Fri, 11 Sep 2026 21:47:33 -0700 Subject: [PATCH 5/6] GH-51293: [Python] Test equality with a null expression Signed-off-by: 1fanwang <1fannnw@gmail.com> --- python/pyarrow/tests/test_compute.py | 3 +++ 1 file changed, 3 insertions(+) diff --git a/python/pyarrow/tests/test_compute.py b/python/pyarrow/tests/test_compute.py index 797fbc220ec3..2ccef3ac6eb3 100644 --- a/python/pyarrow/tests/test_compute.py +++ b/python/pyarrow/tests/test_compute.py @@ -4249,6 +4249,9 @@ def test_expression_construction(): with pytest.raises(TypeError): field.isin(1) + with pytest.raises(TypeError, match="Argument 'other' has incorrect type"): + field.equals(None) + with pytest.raises(pa.ArrowInvalid): field != object() From 94e0f8972f02674660eaf64b469e438f28ab5cc0 Mon Sep 17 00:00:00 2001 From: 1fanwang <1fannnw@gmail.com> Date: Mon, 21 Sep 2026 17:05:59 -0700 Subject: [PATCH 6/6] GH-51293: [Python] Limit null-expression regression coverage The two Expression guards should not depend on unrelated APIs accepting None. Signed-off-by: 1fanwang <1fannnw@gmail.com> --- python/pyarrow/tests/test_misc.py | 121 +++--------------------------- 1 file changed, 12 insertions(+), 109 deletions(-) diff --git a/python/pyarrow/tests/test_misc.py b/python/pyarrow/tests/test_misc.py index f2d14734d31d..0d755e25297b 100644 --- a/python/pyarrow/tests/test_misc.py +++ b/python/pyarrow/tests/test_misc.py @@ -273,115 +273,18 @@ def test_extension_type_constructor_errors(klass): @pytest.mark.processes -def test_public_callables_reject_none_without_crashing(): - # GH-51293: a typed Cython parameter that is not declared "not None" - # lets None reach code that dereferences it, killing the interpreter - # instead of raising. Some public APIs accept None legitimately, so the - # invariant asserted here is only that none of them terminate the - # interpreter. - code = """if 1: - import importlib, inspect - mods = ["pyarrow", "pyarrow.compute", "pyarrow.dataset", - "pyarrow.parquet", "pyarrow.fs", "pyarrow.ipc", - "pyarrow.csv", "pyarrow.json", "pyarrow.feather"] - names = [] - for mn in mods: - try: - m = importlib.import_module(mn) - except Exception: - continue - for n in dir(m): - if n.startswith("_"): - continue - try: - obj = getattr(m, n) - except Exception: - continue - if callable(obj) and not inspect.isclass(obj): - names.append((mn, n)) - names.sort() - for mn, n in names: - print("%s.%s" % (mn, n), flush=True) - try: - getattr(importlib.import_module(mn), n)(None) - except BaseException: - pass - print("DONE", flush=True) - """ - res = subprocess.run([sys.executable, "-c", code], - capture_output=True, text=True, timeout=300) - lines = res.stdout.splitlines() - if res.returncode != 0 or not lines or lines[-1] != "DONE": - culprit = lines[-1] if lines else "" - raise AssertionError( - f"passing None to {culprit} terminated the interpreter " - f"(returncode {res.returncode}); declare its typed parameter " - f"'not None'" - ) - - -@pytest.mark.processes -def test_public_methods_reject_none_without_crashing(): - # Same contract as the function sweep above, applied to methods reached - # from a live object. Expression.equals crashed this way before GH-51293. - code = """if 1: - import inspect - import pyarrow as pa +@pytest.mark.dataset +@pytest.mark.parametrize(("call", "argument"), [ + ("ds.get_partition_keys(None)", "partition_expression"), + ("ds.field('a').equals(None)", "other"), +]) +def test_expression_apis_reject_none_without_crashing( + call: str, argument: str) -> None: + code = f"""if 1: + import pytest import pyarrow.dataset as ds - tbl = pa.table({"a": [1, 2], "b": ["x", "y"]}) - instances = [ - ("Table", tbl), - ("RecordBatch", tbl.to_batches()[0]), - ("Array", pa.array([1, 2, 3])), - ("ChunkedArray", tbl.column("a")), - ("Schema", tbl.schema), - ("Field", tbl.schema.field(0)), - ("DataType", pa.int64()), - ("Scalar", pa.scalar(1)), - ("Dataset", ds.dataset(tbl)), - ("Expression", ds.field("a")), - ("Buffer", pa.py_buffer(b"abc")), - ] - targets = [] - for label, obj in instances: - for name in dir(obj): - if name.startswith("_"): - continue - try: - attr = getattr(obj, name) - except BaseException: - continue - if not callable(attr): - continue - try: - sig = inspect.signature(attr) - except BaseException: - continue - required = [ - p for p in sig.parameters.values() - if p.default is inspect._empty - and p.kind in (p.POSITIONAL_ONLY, p.POSITIONAL_OR_KEYWORD) - ] - if len(required) == 1: - targets.append((label, name)) - targets.sort() - lookup = dict(instances) - for label, name in targets: - print("%s.%s" % (label, name), flush=True) - try: - getattr(lookup[label], name)(None) - except BaseException: - pass - print("DONE", flush=True) + with pytest.raises(TypeError, match="Argument '{argument}'"): + {call} """ - res = subprocess.run([sys.executable, "-c", code], - capture_output=True, text=True, timeout=300) - lines = res.stdout.splitlines() - if res.returncode != 0 or not lines or lines[-1] != "DONE": - culprit = lines[-1] if lines else "" - raise AssertionError( - f"passing None to {culprit} terminated the interpreter " - f"(returncode {res.returncode}); declare its typed parameter " - f"'not None'" - ) + subprocess.check_call(args=[sys.executable, "-c", code], timeout=30)