fix(bigframes): Fix IsInOp literal bug with sqlglot#17356
fix(bigframes): Fix IsInOp literal bug with sqlglot#17356TrevorBergeron wants to merge 2 commits into
Conversation
There was a problem hiding this comment.
Code Review
This pull request replaces sge.convert with sql.literal when compiling comparison operations, and adds unit tests and snapshots to verify the handling of various float literals (such as infinity, NaN, and scientific notation). The review feedback correctly points out that using sql.literal will likely raise a NameError because the sql module is not imported, and suggests using sge.literal instead for consistency and safety.
| if must_upcast_bools and dtype == dtypes.BOOL_DTYPE: | ||
| value = int(value) | ||
| values.append(sge.convert(value)) | ||
| values.append(sql.literal(value)) |
There was a problem hiding this comment.
Using sql.literal may raise a NameError if the sql module or alias is not imported in this file. Since sqlglot.expressions is already imported and used as sge (e.g., sge.Expression and sge.convert), it is safer and more consistent to use sge.literal instead.
| values.append(sql.literal(value)) | |
| values.append(sge.literal(value)) |
Thank you for opening a Pull Request! Before submitting your PR, there are a few things you can do to make sure it goes smoothly:
Fixes #<issue_number_goes_here> 🦕