Re: [I] feat(expressions/visitor): add support for bool [iceberg-python]

["geruh (via GitHub)" <gi...@apache.org>]

geruh commented on issue #2681:
URL: https://github.com/apache/iceberg-python/issues/2681#issuecomment-3621305627

   Hey @HynekBlaha,
   
   Thanks for reporting this! I took a look since we have some expression work in progress on the JSON side. 
   
   It looks like when you call `pl.scan_iceberg(table).filter(pl.lit(True|False))`, Polars converts the filter to a PyIceberg expression via AST parsing which can be seen [here](https://github.com/pola-rs/polars/blob/main/py-polars/src/polars/io/iceberg/_utils.py#L138-L140). this passes a python True/False to PyIceberg's scan.filter(), which expects str | BooleanExpression. Python bool is neither. 
   
   ```
     @_convert_predicate.register(Constant)
     def _(a: Constant) -> Any:
         return a.value  
   
   ```
   
   As Kevin mentioned the visitor has handlers for `AlwaysTrue` and `AlwaysFalse`, but not for Python's bool. But, I think correct fix would be for polars to add a check for bools as they already have a dependency on the expressions in that class. 
   
   ```
   @_convert_predicate.register(Constant)
   def _(a: Constant) -> Any:
     if isinstance(a.value, bool):
         return pyiceberg.expressions.AlwaysTrue() if a.value else pyiceberg.expressions.AlwaysFalse()
     return a.value
   ```


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org