Skip to content

Commit 48af0f0

Browse files
jackylee-chclaude
andcommitted
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 <noreply@anthropic.com>
1 parent c0768b1 commit 48af0f0

1 file changed

Lines changed: 18 additions & 35 deletions

File tree

‎pyiceberg/expressions/parser.py‎

Lines changed: 18 additions & 35 deletions
Original file line numberDiff line numberDiff line change
@@ -121,15 +121,6 @@ def _(result: ParseResults) -> Literal[bool]:
121121
return BooleanLiteral(False)
122122

123123

124-
# As an operand a bare boolean has to fold to AlwaysTrue/AlwaysFalse. This needs its own
125-
# copy because `literal` and `literal_set` keep the raw BooleanLiteral for `foo = true`.
126-
always_boolean = boolean.copy()
127-
128-
129-
@always_boolean.set_parse_action
130-
def _(result: ParseResults) -> BooleanExpression:
131-
return AlwaysTrue() if strtobool(result[0]) else AlwaysFalse()
132-
133124
@string.set_parse_action
134125
def _(result: ParseResults) -> Literal[str]:
135126
return StringLiteral(result.raw_quoted_string[1:-1].replace("''", "'"))
@@ -274,9 +265,16 @@ def _evaluate_like_statement(result: ParseResults) -> BooleanExpression:
274265
return EqualTo(result.column, StringLiteral(literal_like.value.replace("\\%", "%")))
275266

276267

277-
predicate = (between | comparison | in_check | null_check | nan_check | starts_check | always_boolean).set_results_name(
278-
"predicate"
279-
)
268+
predicate = (between | comparison | in_check | null_check | nan_check | starts_check | boolean).set_results_name("predicate")
269+
270+
271+
@predicate.add_parse_action
272+
def _(result: ParseResults) -> BooleanExpression:
273+
# A bare "true" or "false" in an operand position folds to AlwaysTrue or AlwaysFalse
274+
expr = result[0]
275+
if isinstance(expr, BooleanLiteral):
276+
return AlwaysTrue() if expr.value else AlwaysFalse()
277+
return expr
280278

281279

282280
def handle_not(result: ParseResults) -> Not:
@@ -291,29 +289,14 @@ def handle_or(result: ParseResults) -> Or:
291289
return Or(*result[0])
292290

293291

294-
def handle_always_expression(result: ParseResults) -> BooleanExpression:
295-
# If the entire result is "true" or "false", return AlwaysTrue or AlwaysFalse
296-
expr = result[0]
297-
if isinstance(expr, BooleanLiteral):
298-
if expr.value:
299-
return AlwaysTrue()
300-
else:
301-
return AlwaysFalse()
302-
return result[0]
303-
304-
305-
boolean_expression = (
306-
infix_notation(
307-
predicate,
308-
[
309-
(Suppress(NOT), 1, opAssoc.RIGHT, handle_not),
310-
(Suppress(AND), 2, opAssoc.LEFT, handle_and),
311-
(Suppress(OR), 2, opAssoc.LEFT, handle_or),
312-
],
313-
)
314-
.set_name("expr")
315-
.add_parse_action(handle_always_expression)
316-
)
292+
boolean_expression = infix_notation(
293+
predicate,
294+
[
295+
(Suppress(NOT), 1, opAssoc.RIGHT, handle_not),
296+
(Suppress(AND), 2, opAssoc.LEFT, handle_and),
297+
(Suppress(OR), 2, opAssoc.LEFT, handle_or),
298+
],
299+
).set_name("expr")
317300

318301

319302
def parse(expr: str) -> BooleanExpression:

0 commit comments

Comments
 (0)