Skip to content

Commit f54d48c

Browse files
committed
Fix: correct regex pattern in _extract_filter_columns for partition column safeguard
The regex pattern was looking for FieldRef.Name(x) format but PyArrow expressions actually stringify as (column_name == value). This caused the safeguard to return an empty set, making it always pass (empty set is subset of any set), which then pushed partition column filters to the scanner where they crashed. Changes: - Fixed regex to match actual PyArrow expression format: (column == value) - Added patterns for function-style expressions (is_in, is_null, etc.) - Added 7 unit tests for _extract_filter_columns Fixes test_identity_transform_column_projection and test_migrate_table failures.
1 parent 425d752 commit f54d48c

2 files changed

Lines changed: 86 additions & 4 deletions

File tree

pyiceberg/execution/backends/pyarrow_backend.py

Lines changed: 16 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -294,8 +294,12 @@ def read_parquet(
294294
def _extract_filter_columns(pa_filter: pc.Expression) -> set[str]:
295295
"""Extract column names referenced by a PyArrow compute expression.
296296
297-
Uses string representation parsing since PyArrow doesn't expose a clean API
298-
for expression introspection.
297+
Parses the string representation of the expression to find column names.
298+
PyArrow expressions have string formats like:
299+
- (column_name == value) for comparisons
300+
- ((a == 1) and (b == 2)) for compound expressions
301+
- is_in(column_name, ...) for isin expressions
302+
- is_null(column_name, ...) for null checks
299303
300304
Args:
301305
pa_filter: A PyArrow compute expression.
@@ -307,9 +311,17 @@ def _extract_filter_columns(pa_filter: pc.Expression) -> set[str]:
307311

308312
columns: set[str] = set()
309313
expr_str = str(pa_filter)
310-
# Find all field_ref('name') patterns in the expression string
311-
for match in re.finditer(r"field_ref\('([^']+)'", expr_str):
314+
315+
# Pattern 1: column before comparison operators (==, !=, <, >, <=, >=)
316+
# Matches: "column_name ==" or "column_name <" etc.
317+
for match in re.finditer(r"([a-zA-Z_][a-zA-Z0-9_]*)\s*(?:==|!=|<=|>=|<|>)", expr_str):
318+
columns.add(match.group(1))
319+
320+
# Pattern 2: function calls with column as first argument
321+
# Matches: "is_in(column_name," or "is_null(column_name," etc.
322+
for match in re.finditer(r"(?:is_in|is_null|is_nan|is_valid)\(([a-zA-Z_][a-zA-Z0-9_]*)", expr_str):
312323
columns.add(match.group(1))
324+
313325
return columns
314326

315327

tests/execution/test_pyarrow_backend.py

Lines changed: 70 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,7 @@
2121
- _anti_join_tables struct-array O(n+m) multi-column join correctness
2222
- NULL handling with IS NOT DISTINCT FROM semantics
2323
- Full InMemoryCatalog round-trip through pluggable backend
24+
- _extract_filter_columns regex correctness for filter column safeguard
2425
"""
2526

2627
from __future__ import annotations
@@ -38,6 +39,75 @@
3839
from pyiceberg.schema import Schema
3940
from pyiceberg.types import IntegerType, LongType, NestedField, StringType
4041

42+
# =============================================================================
43+
# _extract_filter_columns correctness (for partition column safeguard)
44+
# =============================================================================
45+
46+
47+
class TestExtractFilterColumns:
48+
"""_extract_filter_columns must correctly extract column names from PyArrow expressions.
49+
50+
This is critical for the safeguard in read_parquet() that prevents pushing filters
51+
to the scanner when they reference columns not in the file (e.g., partition columns).
52+
"""
53+
54+
def test_simple_equality(self) -> None:
55+
"""Extract column from simple equality expression."""
56+
from pyiceberg.execution.backends.pyarrow_backend import _extract_filter_columns
57+
58+
expr = pc.field("partition_id") == 1
59+
columns = _extract_filter_columns(expr)
60+
assert columns == {"partition_id"}
61+
62+
def test_simple_comparison(self) -> None:
63+
"""Extract column from comparison expression."""
64+
from pyiceberg.execution.backends.pyarrow_backend import _extract_filter_columns
65+
66+
expr = pc.field("id") > 5
67+
columns = _extract_filter_columns(expr)
68+
assert columns == {"id"}
69+
70+
def test_multiple_columns_and(self) -> None:
71+
"""Extract multiple columns from AND expression."""
72+
from pyiceberg.execution.backends.pyarrow_backend import _extract_filter_columns
73+
74+
expr = (pc.field("a") == 1) & (pc.field("b") == 2)
75+
columns = _extract_filter_columns(expr)
76+
assert columns == {"a", "b"}
77+
78+
def test_multiple_columns_or(self) -> None:
79+
"""Extract multiple columns from OR expression."""
80+
from pyiceberg.execution.backends.pyarrow_backend import _extract_filter_columns
81+
82+
expr = (pc.field("x") < 10) | (pc.field("y") > 20)
83+
columns = _extract_filter_columns(expr)
84+
assert columns == {"x", "y"}
85+
86+
def test_complex_nested(self) -> None:
87+
"""Extract columns from complex nested expression."""
88+
from pyiceberg.execution.backends.pyarrow_backend import _extract_filter_columns
89+
90+
expr = ((pc.field("a") == 1) & (pc.field("b") == 2)) | (pc.field("c") > 3)
91+
columns = _extract_filter_columns(expr)
92+
assert columns == {"a", "b", "c"}
93+
94+
def test_same_column_multiple_times(self) -> None:
95+
"""Same column referenced multiple times returns single entry."""
96+
from pyiceberg.execution.backends.pyarrow_backend import _extract_filter_columns
97+
98+
expr = (pc.field("id") > 5) & (pc.field("id") < 10)
99+
columns = _extract_filter_columns(expr)
100+
assert columns == {"id"}
101+
102+
def test_isin_expression(self) -> None:
103+
"""Extract column from is_in expression."""
104+
from pyiceberg.execution.backends.pyarrow_backend import _extract_filter_columns
105+
106+
expr = pc.field("category").isin(["a", "b", "c"])
107+
columns = _extract_filter_columns(expr)
108+
assert columns == {"category"}
109+
110+
41111
# =============================================================================
42112
# Multi-column anti-join correctness (O(n+m) struct-array approach)
43113
# =============================================================================

0 commit comments

Comments
 (0)