You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@iceberg.apache.org by gi...@apache.org on 2026/09/15 08:34:50 UTC
(iceberg-python) branch gh-readonly-queue/main/pr-3917-4a0eeeafbd8dae6dc2e8f0ccffd5ff24b0a196aa created (now 1312c5509)
This is an automated email from the ASF dual-hosted git repository.
github-merge-queue[bot] pushed a change to branch gh-readonly-queue/main/pr-3917-4a0eeeafbd8dae6dc2e8f0ccffd5ff24b0a196aa
in repository https://gitbox.apache.org/repos/asf/iceberg-python.git
at 1312c5509 fix(expressions): accept a bare true / false as an AND / OR / NOT operand (#3917)
This branch includes the following new commits:
new 1312c5509 fix(expressions): accept a bare true / false as an AND / OR / NOT operand (#3917)
The 1 revisions listed above as "new" are entirely new to this
repository and will be described in separate emails. The revisions
listed as "add" were already present in the repository and have only
been added to this reference.
(iceberg-python) 01/01: fix(expressions): accept a bare true / false as an AND / OR / NOT operand (#3917)
Posted by gi...@apache.org.
This is an automated email from the ASF dual-hosted git repository.
github-merge-queue[bot] pushed a commit to branch gh-readonly-queue/main/pr-3917-4a0eeeafbd8dae6dc2e8f0ccffd5ff24b0a196aa
in repository https://gitbox.apache.org/repos/asf/iceberg-python.git
commit 1312c5509e5459a41d6eff633185b8b8d5f48295
Author: jackylee <qc...@gmail.com>
AuthorDate: Tue Sep 15 08:34:26 2026 +0000
fix(expressions): accept a bare true / false as an AND / OR / NOT operand (#3917)
* 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 <no...@anthropic.com>
* Fix parse action for always_boolean to use strtobool
Co-authored-by: Copilot Autofix powered by AI <17...@users.noreply.github.com>
* 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 <no...@anthropic.com>
---------
Co-authored-by: Claude Code <no...@anthropic.com>
Co-authored-by: Fokko Driesprong <fo...@apache.org>
Co-authored-by: Copilot Autofix powered by AI <17...@users.noreply.github.com>
---
pyiceberg/expressions/parser.py | 40 +++++++++++++++++-----------------------
tests/expressions/test_parser.py | 22 ++++++++++++++++++++++
2 files changed, 39 insertions(+), 23 deletions(-)
diff --git a/pyiceberg/expressions/parser.py b/pyiceberg/expressions/parser.py
index 6b7c8d47f..d813bcaa0 100644
--- a/pyiceberg/expressions/parser.py
+++ b/pyiceberg/expressions/parser.py
@@ -268,6 +268,15 @@ def _evaluate_like_statement(result: ParseResults) -> BooleanExpression:
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:
return Not(result[0][0])
@@ -280,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:
diff --git a/tests/expressions/test_parser.py b/tests/expressions/test_parser.py
index 2f0c444dc..581ee90b7 100644
--- a/tests/expressions/test_parser.py
+++ b/tests/expressions/test_parser.py
@@ -24,6 +24,7 @@ from pyiceberg.expressions import (
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)})