Skip to content

Commit d9bdf8e

Browse files
authored
fix: Serialize AlwaysTrue/AlwaysFalse as boolean literals (#2780)
related to #2775 # Rationale for this change This PR aligns the `AlwaysTrue` and `AlwaysFalse` expression serialization to use the boolean primitives `true`/`false` instead of string representation, matching the Iceberg [ExpressionParser](https://github.com/apache/iceberg/blob/main/core/src/main/java/org/apache/iceberg/expressions/ExpressionParser.java#L101-L108). I know the spec defines the true/false expressions as string but today we can't hit the scan planning client endpoint following these models. I know the spec defines the true/false expressions as a string representation, but currently we can't successfully use the scan planning client endpoint with those models. Java's actual implementation serializes these as boolean literals. For instance, with what we have today we would throw an illegal argument exception: ``` { "snapshot-id": 2540284336700708540, "filter": "true", "case-sensitive": true } ``` Throws: `java.lang.IllegalArgumentException: argument "content" is null` But, this works just fine (notice no quotes): ``` { "snapshot-id": 2540284336700708540, "filter": true, "case-sensitive": true } ``` ## Are these changes tested? Yes ## Are there any user-facing changes? No, this only affects the JSON serialization format to match Java Cores behavior.
1 parent 59dc8d1 commit d9bdf8e

File tree

2 files changed

+6
-6
lines changed

2 files changed

+6
-6
lines changed

pyiceberg/expressions/__init__.py

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -370,10 +370,10 @@ def __getnewargs__(self) -> tuple[BooleanExpression]:
370370
return (self.child,)
371371

372372

373-
class AlwaysTrue(BooleanExpression, Singleton, IcebergRootModel[str]):
373+
class AlwaysTrue(BooleanExpression, Singleton, IcebergRootModel[bool]):
374374
"""TRUE expression."""
375375

376-
root: str = "true"
376+
root: bool = True
377377

378378
def __invert__(self) -> AlwaysFalse:
379379
"""Transform the Expression into its negated version."""
@@ -388,10 +388,10 @@ def __repr__(self) -> str:
388388
return "AlwaysTrue()"
389389

390390

391-
class AlwaysFalse(BooleanExpression, Singleton, IcebergRootModel[str]):
391+
class AlwaysFalse(BooleanExpression, Singleton, IcebergRootModel[bool]):
392392
"""FALSE expression."""
393393

394-
root: str = "false"
394+
root: bool = False
395395

396396
def __invert__(self) -> AlwaysTrue:
397397
"""Transform the Expression into its negated version."""

tests/expressions/test_expressions.py

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -770,7 +770,7 @@ def test_not_json_serialization_and_deserialization() -> None:
770770

771771
def test_always_true() -> None:
772772
always_true = AlwaysTrue()
773-
assert always_true.model_dump_json() == '"true"'
773+
assert always_true.model_dump_json() == "true"
774774
assert str(always_true) == "AlwaysTrue()"
775775
assert repr(always_true) == "AlwaysTrue()"
776776
assert always_true == eval(repr(always_true))
@@ -779,7 +779,7 @@ def test_always_true() -> None:
779779

780780
def test_always_false() -> None:
781781
always_false = AlwaysFalse()
782-
assert always_false.model_dump_json() == '"false"'
782+
assert always_false.model_dump_json() == "false"
783783
assert str(always_false) == "AlwaysFalse()"
784784
assert repr(always_false) == "AlwaysFalse()"
785785
assert always_false == eval(repr(always_false))

0 commit comments

Comments
 (0)