Skip to content

Commit 444a67b

Browse files
committed
Fix post-filter to use task.residual and preserve dictionary encoding
Two fixes for pluggable backend regressions: 1. Post-filter now uses task.residual instead of row_filter - The residual is the part of the filter not handled by partition pruning - If residual is AlwaysTrue, partition pruning handled everything (no post-filter needed) - The original row_filter may reference columns not in projected schema - Fixes test_partitioned_tables which filters by 'ts' but only selects 'number' 2. Pass dictionary_columns to PyArrow ParquetFileFormat - The PyArrow backend wasn't passing dictionary_columns to the parquet reader - Without this, dictionary encoding was silently dropped - Fixes test_to_arrow_batch_reader_preserves_dictionary_columns
1 parent f461d36 commit 444a67b

2 files changed

Lines changed: 29 additions & 17 deletions

File tree

pyiceberg/execution/_orchestrate.py

Lines changed: 21 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -343,29 +343,34 @@ def _execute_task(task: FileScanTask) -> list[pa.RecordBatch]:
343343
if puffin_positions:
344344
result_batches = _apply_puffin_positions(result_batches, puffin_positions)
345345

346-
# Post-filter guarantees correctness; read_parquet pushdown is best-effort only.
347-
# Use the row_filter parameter (not task.residual) for post-filtering.
348-
# This allows callers like Transaction.delete's _read_live_rows to override
349-
# the task's residual (e.g., pass AlwaysTrue() to read all rows).
346+
# Post-filter uses the task's residual, NOT the original row_filter.
347+
# The residual is what remains after partition pruning - it only references
348+
# columns that are either:
349+
# 1. In the projected schema (user selected them)
350+
# 2. Partition columns projected via identity transform
350351
#
351-
# The residual from ManifestGroupPlanner may contain unbound predicates
352-
# (e.g., for unpartitioned tables or predicates not involving partition columns).
353-
# We must bind to the projected schema before converting to PyArrow expression.
354-
# However, some residuals may already be bound (from tests or REST scan planning),
355-
# so we catch the TypeError from bind() and use the residual as-is.
352+
# If residual is AlwaysTrue, partition pruning handled the entire filter
353+
# and no row-level filtering is needed. If the original row_filter references
354+
# columns not in the projection, the residual should be AlwaysTrue (because
355+
# partition pruning eliminated all non-matching files) or the filter should
356+
# have been pushed down during read_parquet.
356357
#
357358
# Post-filter happens AFTER reconciliation so that:
358359
# - Projected partition columns are available for filtering
359-
# - Renamed columns use their current names
360-
if not isinstance(row_filter, AlwaysTrue):
360+
# - Renamed columns use their current names (for schema evolution)
361+
if not isinstance(task.residual, AlwaysTrue):
361362
from pyiceberg.expressions.visitors import bind
362363

363364
try:
364-
bound_filter = bind(projected_schema, row_filter, case_sensitive)
365-
except TypeError:
366-
# Predicate is already bound
367-
bound_filter = row_filter
368-
result_batches = list(backends.compute.filter(iter(result_batches), bound_filter))
365+
bound_filter = bind(projected_schema, task.residual, case_sensitive)
366+
except (TypeError, ValueError):
367+
# Predicate is already bound, or references columns not in projected schema.
368+
# The latter can happen if the filter references non-partition columns that
369+
# weren't selected - in this case we skip post-filter and rely on pushdown.
370+
bound_filter = None
371+
372+
if bound_filter is not None:
373+
result_batches = list(backends.compute.filter(iter(result_batches), bound_filter))
369374

370375
return result_batches
371376

pyiceberg/execution/backends/pyarrow_backend.py

Lines changed: 8 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -253,7 +253,14 @@ def read_parquet(
253253
pa_filter = None
254254

255255
filesystem, path = _resolve_filesystem(location, io_properties)
256-
dataset = ds.dataset(path, format="parquet", filesystem=filesystem)
256+
257+
# Configure parquet format with dictionary columns if specified
258+
format_kwargs: dict[str, Any] = {}
259+
if dictionary_columns:
260+
format_kwargs["dictionary_columns"] = list(dictionary_columns)
261+
parquet_format = ds.ParquetFileFormat(**format_kwargs) if format_kwargs else "parquet"
262+
263+
dataset = ds.dataset(path, format=parquet_format, filesystem=filesystem)
257264

258265
# Only request columns that exist in the file. Missing columns (from schema
259266
# evolution) will be filled with NULLs by the schema reconciliation layer in

0 commit comments

Comments
 (0)