Skip to content

Commit 8408a5d

Browse files
fix(expressions): bound the DNF expansion in rewrite_to_dnf (#3995)
Converting to DNF distributes every AND over the ORs below it, so a filter with n two-branch OR groups expands to 2**n terms. Nothing bounded that, so a small user-supplied filter could produce an expression no consumer can use: 20 groups reached 1,048,576 terms in ~17s and ~1 GiB. Cap the expansion at MAX_DNF_TERMS and raise before the product is built, so the same input now fails in 0.22s.
1 parent 7e7d340 commit 8408a5d

2 files changed

Lines changed: 27 additions & 0 deletions

File tree

‎pyiceberg/expressions/visitors.py‎

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -974,6 +974,12 @@ def extract_field_ids(expr: BooleanExpression) -> set[int]:
974974
return visit(expr, _ExpressionFieldIDs())
975975

976976

977+
# Converting to DNF distributes every AND over the ORs below it, so the number of
978+
# terms is exponential in the worst case. The expansion is bounded to keep a small
979+
# filter from producing an expression that no consumer can use.
980+
MAX_DNF_TERMS = 1 << 14
981+
982+
977983
class _RewriteToDNF(BooleanExpressionVisitor[tuple[BooleanExpression, ...]]):
978984
def visit_true(self) -> tuple[BooleanExpression, ...]:
979985
return (AlwaysTrue(),)
@@ -991,6 +997,8 @@ def visit_and(
991997
# ((P OR Q) AND (R OR S)) AND (((P AND R) OR (P AND S)) OR ((Q AND R) OR ((Q AND S)))
992998
# A AND (B OR C) = (A AND B) OR (A AND C)
993999
# (A OR B) AND C = (A AND C) OR (B AND C)
1000+
if len(left_result) * len(right_result) > MAX_DNF_TERMS:
1001+
raise ValueError(f"Expression expands to more than the maximum of {MAX_DNF_TERMS} DNF terms")
9941002
return tuple(And(le, re) for le in left_result for re in right_result)
9951003

9961004
def visit_or(
@@ -1008,6 +1016,7 @@ def visit_bound_predicate(self, predicate: BoundPredicate) -> tuple[BooleanExpre
10081016
def rewrite_to_dnf(expr: BooleanExpression) -> tuple[BooleanExpression, ...]:
10091017
# Rewrites an arbitrary boolean expression to disjunctive normal form (DNF):
10101018
# (A AND NOT(B) AND C) OR (NOT(D) AND E AND F) OR (G)
1019+
# Raises ValueError when the expansion exceeds MAX_DNF_TERMS terms.
10111020
expr_without_not = rewrite_not(expr)
10121021
return visit(expr_without_not, _RewriteToDNF())
10131022

‎tests/expressions/test_visitors.py‎

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -66,6 +66,7 @@
6666
)
6767
from pyiceberg.expressions.literals import Literal, literal
6868
from pyiceberg.expressions.visitors import (
69+
MAX_DNF_TERMS,
6970
BindVisitor,
7071
BooleanExpressionVisitor,
7172
BoundBooleanExpressionVisitor,
@@ -1594,6 +1595,23 @@ def test_to_dnf_not_and() -> None:
15941595
assert rewrite_to_dnf(expr) == (EqualTo("Q", "b"), NotEqualTo("R", "c"))
15951596

15961597

1598+
def _chained_or_pairs(groups: int) -> BooleanExpression:
1599+
# (a_0 = x OR b_0 = y) AND ... AND (a_n = x OR b_n = y), which expands to 2**groups DNF terms
1600+
expr: BooleanExpression = Or(EqualTo("a0", "x"), EqualTo("b0", "y"))
1601+
for idx in range(1, groups):
1602+
expr = And(expr, Or(EqualTo(f"a{idx}", "x"), EqualTo(f"b{idx}", "y")))
1603+
return expr
1604+
1605+
1606+
def test_to_dnf_at_expansion_limit() -> None:
1607+
assert len(rewrite_to_dnf(_chained_or_pairs(14))) == MAX_DNF_TERMS
1608+
1609+
1610+
def test_to_dnf_rejects_unbounded_expansion() -> None:
1611+
with pytest.raises(ValueError, match=f"maximum of {MAX_DNF_TERMS} DNF terms"):
1612+
rewrite_to_dnf(_chained_or_pairs(20))
1613+
1614+
15971615
def test_dnf_to_dask(table_schema_simple: Schema) -> None:
15981616
expr = (
15991617
BoundGreaterThan(

0 commit comments

Comments
 (0)