Skip to content

Commit 33e0e11

Browse files
authored
Fix deepcopy for And, Or, and Not expressions (#79)
This PR addresses failures when `copy.deepcopy()` is applied to PyIceberg boolean expression trees under Pydantic v2, particularly for compound expressions (`And`/`Or`/`Not`) with `__new__` signatures requiring arguments, and for `Singleton`-based expressions that must not be reconstructed.
1 parent e1ab55e commit 33e0e11

2 files changed

Lines changed: 86 additions & 0 deletions

File tree

pyiceberg/expressions/__init__.py

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,7 @@
1717

1818
from __future__ import annotations
1919

20+
import copy
2021
from abc import ABC, abstractmethod
2122
from collections.abc import Callable, Iterable, Sequence
2223
from functools import cached_property
@@ -339,6 +340,10 @@ def __invert__(self) -> BooleanExpression:
339340
# De Morgan's law: not (A and B) = (not A) or (not B)
340341
return Or(~self.left, ~self.right)
341342

343+
def __deepcopy__(self, memo: dict[int, Any]) -> And:
344+
"""Return a deep copy of the And expression."""
345+
return And(copy.deepcopy(self.left, memo), copy.deepcopy(self.right, memo))
346+
342347
def __getnewargs__(self) -> tuple[BooleanExpression, BooleanExpression]:
343348
"""Pickle the And class."""
344349
return (self.left, self.right)
@@ -386,6 +391,10 @@ def __invert__(self) -> BooleanExpression:
386391
# De Morgan's law: not (A or B) = (not A) and (not B)
387392
return And(~self.left, ~self.right)
388393

394+
def __deepcopy__(self, memo: dict[int, Any]) -> Or:
395+
"""Return a deep copy of the Or expression."""
396+
return Or(copy.deepcopy(self.left, memo), copy.deepcopy(self.right, memo))
397+
389398
def __getnewargs__(self) -> tuple[BooleanExpression, BooleanExpression]:
390399
"""Pickle the Or class."""
391400
return (self.left, self.right)
@@ -428,6 +437,10 @@ def __invert__(self) -> BooleanExpression:
428437
"""Transform the Expression into its negated version."""
429438
return self.child
430439

440+
def __deepcopy__(self, memo: dict[int, Any]) -> Not:
441+
"""Return a deep copy of the Not expression."""
442+
return Not(copy.deepcopy(self.child, memo))
443+
431444
def __getnewargs__(self) -> tuple[BooleanExpression]:
432445
"""Pickle the Not class."""
433446
return (self.child,)

tests/expressions/test_expressions.py

Lines changed: 73 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,7 @@
1616
# under the License.
1717
# pylint:disable=redefined-outer-name,eval-used
1818

19+
import copy
1920
import pickle
2021
import uuid
2122
from decimal import Decimal
@@ -1292,6 +1293,78 @@ def test_bind_ambiguous_name() -> None:
12921293
assert "Invalid schema, multiple fields for name foo.bar: 2 and 3" in str(exc_info)
12931294

12941295

1296+
# --- deepcopy tests ---
1297+
1298+
1299+
def test_deepcopy_and() -> None:
1300+
expr = And(EqualTo("x", 1), EqualTo("y", 2))
1301+
copied = copy.deepcopy(expr)
1302+
assert copied == expr
1303+
assert copied is not expr
1304+
1305+
1306+
def test_deepcopy_or() -> None:
1307+
expr = Or(EqualTo("x", 1), EqualTo("y", 2))
1308+
copied = copy.deepcopy(expr)
1309+
assert copied == expr
1310+
assert copied is not expr
1311+
1312+
1313+
def test_deepcopy_not() -> None:
1314+
expr = Not(EqualTo("x", 1))
1315+
copied = copy.deepcopy(expr)
1316+
assert copied == expr
1317+
assert copied is not expr
1318+
1319+
1320+
def test_deepcopy_equal_to() -> None:
1321+
expr = EqualTo("x", 1)
1322+
copied = copy.deepcopy(expr)
1323+
assert copied == expr
1324+
assert copied is not expr
1325+
1326+
1327+
def test_deepcopy_always_true() -> None:
1328+
copied = copy.deepcopy(AlwaysTrue())
1329+
assert copied is AlwaysTrue()
1330+
1331+
1332+
def test_deepcopy_always_false() -> None:
1333+
copied = copy.deepcopy(AlwaysFalse())
1334+
assert copied is AlwaysFalse()
1335+
1336+
1337+
def test_deepcopy_always_true_then_pickle() -> None:
1338+
copied = copy.deepcopy(AlwaysTrue())
1339+
restored = pickle.loads(pickle.dumps(copied))
1340+
assert restored is AlwaysTrue()
1341+
1342+
1343+
def test_deepcopy_balanced_and() -> None:
1344+
expr = And(EqualTo("a", 1), EqualTo("b", 2), EqualTo("c", 3), EqualTo("d", 4))
1345+
copied = copy.deepcopy(expr)
1346+
assert copied == expr
1347+
1348+
1349+
def test_deepcopy_balanced_or() -> None:
1350+
expr = Or(EqualTo("a", 1), EqualTo("b", 2), EqualTo("c", 3), EqualTo("d", 4))
1351+
copied = copy.deepcopy(expr)
1352+
assert copied == expr
1353+
1354+
1355+
def test_deepcopy_nested_expression() -> None:
1356+
expr = And(Or(EqualTo("a", 1), EqualTo("b", 2)), Not(EqualTo("c", 3)))
1357+
copied = copy.deepcopy(expr)
1358+
assert copied == expr
1359+
1360+
1361+
def test_deepcopy_then_pickle() -> None:
1362+
expr = And(EqualTo("x", 1), EqualTo("y", 2))
1363+
copied = copy.deepcopy(expr)
1364+
restored = pickle.loads(pickle.dumps(copied))
1365+
assert restored == expr
1366+
1367+
12951368
# __ __ ___
12961369
# | \/ |_ _| _ \_ _
12971370
# | |\/| | || | _/ || |

0 commit comments

Comments
 (0)