sungwy commented on code in PR #988:
URL: https://github.com/apache/iceberg-python/pull/988#discussion_r1699244665


##########
pyiceberg/expressions/literals.py:
##########
@@ -588,7 +589,7 @@ def _(self, type_var: DecimalType) -> Literal[Decimal]:
     def _(self, type_var: BooleanType) -> Literal[bool]:
         value_upper = self.value.upper()
         if value_upper in ["TRUE", "FALSE"]:
-            return BooleanLiteral(value_upper == "TRUE")
+            return BooleanLiteral(strtobool(value_upper))

Review Comment:
   I feel as though this is an over-optimization, since we check whether 
`value_upper` is in "TRUE" and "FALSE". WDYT?
   
   An alternative is to instead just use strtobool for the whole function, but 
that would mean that we support a string expression like:
   
   "col_A == 'off'" where col_A is a boolean column and 'off' gets parsed as 
False, and that feels unnecessarily over permissive. Should we leave this block 
of code as it is?



-- 
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-unsubscr...@iceberg.apache.org

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


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

Reply via email to