From 2953aab30b1237ab8c2601a836b221d7115da218 Mon Sep 17 00:00:00 2001 From: jackylee-ch Date: Mon, 7 Sep 2026 11:22:00 +0800 Subject: [PATCH 1/3] fix(expressions): accept a bare true / false as an AND / OR / NOT operand The BooleanLiteral to AlwaysTrue/AlwaysFalse conversion was attached only to the whole expression, so a bare boolean reached And/Or/Not as a BooleanLiteral and failed pydantic validation: parse("(false) or foo = 1") -> EqualTo(foo, 1) parse("false or foo = 1") -> ValidationError: 1 validation error for Or Seeding a filter with `true` and appending clauses is a common way to build a row_filter, so `row_filter="true and status = 'x'"` raised instead of scanning. Give `predicate` its own copy of the boolean element that folds to AlwaysTrue/AlwaysFalse; `literal` and `literal_set` keep the raw BooleanLiteral, so `foo = true` and `foo in (true, false)` are unchanged. Co-Authored-By: Claude Code --- pyiceberg/expressions/parser.py | 14 +++++++++++++- tests/expressions/test_parser.py | 22 ++++++++++++++++++++++ 2 files changed, 35 insertions(+), 1 deletion(-) diff --git a/pyiceberg/expressions/parser.py b/pyiceberg/expressions/parser.py index 6b7c8d47f3..20548a7990 100644 --- a/pyiceberg/expressions/parser.py +++ b/pyiceberg/expressions/parser.py @@ -121,6 +121,16 @@ def _(result: ParseResults) -> Literal[bool]: return BooleanLiteral(False) +# As an operand a bare boolean has to fold to AlwaysTrue/AlwaysFalse. This needs its own +# copy because `literal` and `literal_set` keep the raw BooleanLiteral for `foo = true`. +always_boolean = boolean.copy() + + +@always_boolean.add_parse_action +def _(result: ParseResults) -> BooleanExpression: + return AlwaysTrue() if result[0].value else AlwaysFalse() + + @string.set_parse_action def _(result: ParseResults) -> Literal[str]: return StringLiteral(result.raw_quoted_string[1:-1].replace("''", "'")) @@ -265,7 +275,9 @@ def _evaluate_like_statement(result: ParseResults) -> BooleanExpression: return EqualTo(result.column, StringLiteral(literal_like.value.replace("\\%", "%"))) -predicate = (between | comparison | in_check | null_check | nan_check | starts_check | boolean).set_results_name("predicate") +predicate = (between | comparison | in_check | null_check | nan_check | starts_check | always_boolean).set_results_name( + "predicate" +) def handle_not(result: ParseResults) -> Not: diff --git a/tests/expressions/test_parser.py b/tests/expressions/test_parser.py index 2f0c444dc4..581ee90b72 100644 --- a/tests/expressions/test_parser.py +++ b/tests/expressions/test_parser.py @@ -24,6 +24,7 @@ AlwaysFalse, AlwaysTrue, And, + BooleanExpression, EqualTo, GreaterThan, GreaterThanOrEqual, @@ -272,3 +273,24 @@ def test_valid_between_with_numerics() -> None: ) == parser.parse("foo between '2025-01-01T00:00:00.000000' and '2025-01-10T12:00:00.000000'") assert parser.parse("foo between 1 and 3") == parser.parse("1 <= foo and foo <= 3") + + +@pytest.mark.parametrize( + "expression, expected", + [ + ("true and foo = 1", EqualTo(Reference("foo"), literal(1))), + ("foo = 1 and true", EqualTo(Reference("foo"), literal(1))), + ("foo = 1 or false", EqualTo(Reference("foo"), literal(1))), + ("foo = 1 or true", AlwaysTrue()), + ("foo = 1 and false", AlwaysFalse()), + ("not true", AlwaysFalse()), + ("not false", AlwaysTrue()), + ], +) +def test_boolean_as_operand(expression: str, expected: BooleanExpression) -> None: + assert parser.parse(expression) == expected + + +def test_boolean_as_literal_is_unchanged() -> None: + assert parser.parse("foo = true") == EqualTo(Reference("foo"), literal(True)) + assert parser.parse("foo in (true, false)") == In(Reference("foo"), {literal(True), literal(False)}) From c0768b1847d1eb02b9097b78c127291b19021a54 Mon Sep 17 00:00:00 2001 From: Fokko Driesprong Date: Mon, 7 Sep 2026 08:30:36 +0200 Subject: [PATCH 2/3] Fix parse action for always_boolean to use strtobool Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com> --- pyiceberg/expressions/parser.py | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/pyiceberg/expressions/parser.py b/pyiceberg/expressions/parser.py index 20548a7990..6ae0b26671 100644 --- a/pyiceberg/expressions/parser.py +++ b/pyiceberg/expressions/parser.py @@ -126,10 +126,9 @@ def _(result: ParseResults) -> Literal[bool]: always_boolean = boolean.copy() -@always_boolean.add_parse_action +@always_boolean.set_parse_action def _(result: ParseResults) -> BooleanExpression: - return AlwaysTrue() if result[0].value else AlwaysFalse() - + return AlwaysTrue() if strtobool(result[0]) else AlwaysFalse() @string.set_parse_action def _(result: ParseResults) -> Literal[str]: From 48af0f034e55b14b0024df86ad519439218198f7 Mon Sep 17 00:00:00 2001 From: jackylee-ch Date: Mon, 7 Sep 2026 15:09:10 +0800 Subject: [PATCH 3/3] refactor(expressions): fold a bare boolean at the predicate level The extra `boolean.copy()` is not needed. `handle_always_expression` already did this fold; it was only attached to the whole expression, which is why `(true) and foo = 1` worked and `true and foo = 1` did not -- infix_notation returns the same Forward that a parenthesized sub-expression recurses through, so the fold ran only when an operand happened to take that path. Attach it to `predicate` instead so every operand folds. `literal` and `literal_set` consume the boolean further in, so `foo = true` and `foo in (true, false)` keep the raw `BooleanLiteral`. The top-level attachment is now unreachable and is removed. Tests are unchanged. Co-Authored-By: Claude Code --- pyiceberg/expressions/parser.py | 53 +++++++++++---------------------- 1 file changed, 18 insertions(+), 35 deletions(-) diff --git a/pyiceberg/expressions/parser.py b/pyiceberg/expressions/parser.py index 6ae0b26671..d813bcaa07 100644 --- a/pyiceberg/expressions/parser.py +++ b/pyiceberg/expressions/parser.py @@ -121,15 +121,6 @@ def _(result: ParseResults) -> Literal[bool]: return BooleanLiteral(False) -# As an operand a bare boolean has to fold to AlwaysTrue/AlwaysFalse. This needs its own -# copy because `literal` and `literal_set` keep the raw BooleanLiteral for `foo = true`. -always_boolean = boolean.copy() - - -@always_boolean.set_parse_action -def _(result: ParseResults) -> BooleanExpression: - return AlwaysTrue() if strtobool(result[0]) else AlwaysFalse() - @string.set_parse_action def _(result: ParseResults) -> Literal[str]: return StringLiteral(result.raw_quoted_string[1:-1].replace("''", "'")) @@ -274,9 +265,16 @@ def _evaluate_like_statement(result: ParseResults) -> BooleanExpression: return EqualTo(result.column, StringLiteral(literal_like.value.replace("\\%", "%"))) -predicate = (between | comparison | in_check | null_check | nan_check | starts_check | always_boolean).set_results_name( - "predicate" -) +predicate = (between | comparison | in_check | null_check | nan_check | starts_check | boolean).set_results_name("predicate") + + +@predicate.add_parse_action +def _(result: ParseResults) -> BooleanExpression: + # A bare "true" or "false" in an operand position folds to AlwaysTrue or AlwaysFalse + expr = result[0] + if isinstance(expr, BooleanLiteral): + return AlwaysTrue() if expr.value else AlwaysFalse() + return expr def handle_not(result: ParseResults) -> Not: @@ -291,29 +289,14 @@ def handle_or(result: ParseResults) -> Or: return Or(*result[0]) -def handle_always_expression(result: ParseResults) -> BooleanExpression: - # If the entire result is "true" or "false", return AlwaysTrue or AlwaysFalse - expr = result[0] - if isinstance(expr, BooleanLiteral): - if expr.value: - return AlwaysTrue() - else: - return AlwaysFalse() - return result[0] - - -boolean_expression = ( - infix_notation( - predicate, - [ - (Suppress(NOT), 1, opAssoc.RIGHT, handle_not), - (Suppress(AND), 2, opAssoc.LEFT, handle_and), - (Suppress(OR), 2, opAssoc.LEFT, handle_or), - ], - ) - .set_name("expr") - .add_parse_action(handle_always_expression) -) +boolean_expression = infix_notation( + predicate, + [ + (Suppress(NOT), 1, opAssoc.RIGHT, handle_not), + (Suppress(AND), 2, opAssoc.LEFT, handle_and), + (Suppress(OR), 2, opAssoc.LEFT, handle_or), + ], +).set_name("expr") def parse(expr: str) -> BooleanExpression: