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