Skip to content

Commit eee2667

Browse files
Gayathri Srividya Rajavarapuclaude
authored andcommitted
fix: include NULL in delete predicate for evolved partition fields
dynamic_partition_overwrite builds a delete predicate from the current partition spec. After spec evolution, data files written under older specs may carry NULL for fields that were added later. The predicate category=A AND region=us never matches those files (region=NULL ≠ us), so they are silently left behind after the overwrite. Fix: detect which fields in the current spec are absent from at least one historical spec (i.e., were added via evolution), and extend the predicate for those fields to also accept NULL: category=A AND (region=us OR region IS NULL) This ensures pre-evolution data files are included in the delete pass while files from other partitions (region=eu, etc.) are correctly kept. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
1 parent 54d8f64 commit eee2667

2 files changed

Lines changed: 110 additions & 17 deletions

File tree

‎pyiceberg/table/__init__.py‎

Lines changed: 48 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -355,29 +355,46 @@ def _set_ref_snapshot(
355355
return updates, requirements
356356

357357
def _build_partition_predicate(
358-
self, partition_records: set[Record], spec: PartitionSpec, schema: Schema
358+
self,
359+
partition_records: set[Record],
360+
spec: PartitionSpec,
361+
schema: Schema,
362+
evolved_source_ids: set[int] | None = None,
359363
) -> BooleanExpression:
360364
"""Build a filter predicate matching any of the input partition records.
361365
362366
Args:
363367
partition_records: A set of partition records to match
364368
spec: An optional partition spec, if none then defaults to current
365369
schema: An optional schema, if none then defaults to current
370+
evolved_source_ids: Source IDs of partition fields that were added via spec
371+
evolution and therefore may be absent (NULL) in data written under older
372+
specs. When a field is in this set and the partition value is non-NULL,
373+
the predicate also accepts NULL so that pre-evolution files are included.
366374
Returns:
367375
A predicate matching any of the input partition records.
368376
"""
369-
partition_fields = [schema.find_field(field.source_id).name for field in spec.fields]
370-
if not partition_records or not partition_fields:
377+
spec_fields = spec.fields
378+
partition_field_names = [schema.find_field(field.source_id).name for field in spec_fields]
379+
if not partition_records or not partition_field_names:
371380
return AlwaysFalse()
372381

382+
nullable_source_ids: set[int] = evolved_source_ids or set()
383+
373384
per_record_exprs: list[BooleanExpression] = []
374385
for partition_record in partition_records:
375-
predicates: list[BooleanExpression] = [
376-
EqualTo(Reference(partition_field), partition_record[pos])
377-
if partition_record[pos] is not None
378-
else IsNull(Reference(partition_field))
379-
for pos, partition_field in enumerate(partition_fields)
380-
]
386+
predicates: list[BooleanExpression] = []
387+
for pos, (field_name, spec_field) in enumerate(zip(partition_field_names, spec_fields, strict=True)):
388+
value = partition_record[pos]
389+
if value is not None:
390+
field_pred: BooleanExpression = EqualTo(Reference(field_name), value)
391+
if spec_field.source_id in nullable_source_ids:
392+
# Also match NULL: pre-evolution files may have no value for this
393+
# field because it was not in the schema when the data was written.
394+
field_pred = Or(field_pred, IsNull(Reference(field_name)))
395+
else:
396+
field_pred = IsNull(Reference(field_name))
397+
predicates.append(field_pred)
381398
per_record_exprs.append(And(*predicates) if len(predicates) > 1 else predicates[0])
382399

383400
return Or(*per_record_exprs) if len(per_record_exprs) > 1 else per_record_exprs[0]
@@ -586,8 +603,24 @@ def dynamic_partition_overwrite(
586603
)
587604

588605
partitions_to_overwrite = {data_file.partition for data_file in data_files}
606+
607+
# Determine which partition fields were introduced via spec evolution. Data
608+
# written under older specs may have NULL for those fields even when the new
609+
# partition value is non-NULL, so the delete predicate must also match NULL.
610+
current_spec = self.table_metadata.spec()
611+
all_specs = self.table_metadata.specs()
612+
# A field is "evolved" if it is absent from at least one historical spec.
613+
evolved_source_ids = {
614+
f.source_id
615+
for f in current_spec.fields
616+
if not all(any(hf.source_id == f.source_id for hf in hs.fields) for hs in all_specs.values())
617+
}
618+
589619
delete_filter = self._build_partition_predicate(
590-
partition_records=partitions_to_overwrite, spec=self.table_metadata.spec(), schema=self.table_metadata.schema()
620+
partition_records=partitions_to_overwrite,
621+
spec=current_spec,
622+
schema=self.table_metadata.schema(),
623+
evolved_source_ids=evolved_source_ids,
591624
)
592625
self.delete(delete_filter=delete_filter, snapshot_properties=snapshot_properties, branch=branch)
593626

@@ -2072,13 +2105,11 @@ def _build_residual_evaluator(self, spec_id: int) -> Callable[[DataFile], Residu
20722105
# The lambda created here is run in multiple threads.
20732106
# So we avoid creating _EvaluatorExpression methods bound to a single
20742107
# shared instance across multiple threads.
2075-
return lambda datafile: (
2076-
residual_evaluator_of(
2077-
spec=spec,
2078-
expr=self.row_filter,
2079-
case_sensitive=self.case_sensitive,
2080-
schema=self.table_metadata.schema(),
2081-
)
2108+
return lambda datafile: residual_evaluator_of(
2109+
spec=spec,
2110+
expr=self.row_filter,
2111+
case_sensitive=self.case_sensitive,
2112+
schema=self.table_metadata.schema(),
20822113
)
20832114

20842115
@staticmethod

‎tests/table/test_init.py‎

Lines changed: 62 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -31,6 +31,8 @@
3131
And,
3232
EqualTo,
3333
In,
34+
IsNull,
35+
Or,
3436
)
3537
from pyiceberg.expressions.visitors import bind
3638
from pyiceberg.io import PY_IO_IMPL, load_file_io
@@ -1766,3 +1768,63 @@ def test_build_large_partition_predicate(table_v2: Table) -> None:
17661768
)
17671769

17681770
bind(table_v2.metadata.schema(), expr, case_sensitive=True)
1771+
1772+
1773+
def test_build_partition_predicate_with_evolved_source_ids(table_v2: Table) -> None:
1774+
"""Regression test for https://github.com/apache/iceberg-python/issues/3148.
1775+
1776+
When a partition field is added via spec evolution, data written under older specs
1777+
may have NULL for that field. _build_partition_predicate must include
1778+
``OR field IS NULL`` for evolved fields so that pre-evolution files are matched by
1779+
the delete predicate and are not silently left behind.
1780+
"""
1781+
from pyiceberg.transforms import IdentityTransform
1782+
1783+
schema = Schema(
1784+
NestedField(1, "category", StringType(), required=False),
1785+
NestedField(2, "region", StringType(), required=False),
1786+
)
1787+
spec = PartitionSpec(
1788+
PartitionField(source_id=1, field_id=1000, transform=IdentityTransform(), name="category"),
1789+
PartitionField(source_id=2, field_id=1001, transform=IdentityTransform(), name="region"),
1790+
)
1791+
1792+
# region (source_id=2) was added via spec evolution → it is an evolved field
1793+
with table_v2.transaction() as tx:
1794+
expr = tx._build_partition_predicate(
1795+
partition_records={Record("A", "us")},
1796+
spec=spec,
1797+
schema=schema,
1798+
evolved_source_ids={2},
1799+
)
1800+
1801+
# category=A AND (region=us OR region IS NULL)
1802+
expected = And(
1803+
EqualTo("category", "A"),
1804+
Or(EqualTo("region", "us"), IsNull("region")),
1805+
)
1806+
assert repr(expr) == repr(expected)
1807+
1808+
1809+
def test_build_partition_predicate_without_evolved_source_ids(table_v2: Table) -> None:
1810+
"""Without evolved_source_ids, the predicate matches exact values only."""
1811+
from pyiceberg.transforms import IdentityTransform
1812+
1813+
schema = Schema(
1814+
NestedField(1, "category", StringType(), required=False),
1815+
NestedField(2, "region", StringType(), required=False),
1816+
)
1817+
spec = PartitionSpec(
1818+
PartitionField(source_id=1, field_id=1000, transform=IdentityTransform(), name="category"),
1819+
PartitionField(source_id=2, field_id=1001, transform=IdentityTransform(), name="region"),
1820+
)
1821+
1822+
with table_v2.transaction() as tx:
1823+
expr = tx._build_partition_predicate(
1824+
partition_records={Record("A", "us")},
1825+
spec=spec,
1826+
schema=schema,
1827+
)
1828+
1829+
expected = And(EqualTo("category", "A"), EqualTo("region", "us"))
1830+
assert repr(expr) == repr(expected)

0 commit comments

Comments
 (0)