-
Notifications
You must be signed in to change notification settings - Fork 133
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
fix: implement modulus operator #1048
Conversation
@Linchin This seems like a pretty safe change. Any chance you can review it? |
Can you do me a favor? The sqlalchemy commands that would generate a statement that includes something akin to I want to ensure that I am picturing this the way you are. Thanks. |
@chalmerlowe Sure, this is how it behaves before the change In [1]: import sqlalchemy as sa; from sqlalchemy_bigquery import BigQueryDialect
In [2]: meta = sa.MetaData(); table = sa.Table("table_name", meta, sa.Column("col_name", sa.Integer()))
In [3]: print(str(sa.select(table.c.col_name).where(table.c.col_name % 2 == 0).compile(dialect=BigQueryDialect())))
SELECT `table_name`.`col_name`
FROM `table_name`
WHERE `table_name`.`col_name` % :col_name_1 = :param_1 Which the following screenshots confirm is not the correct syntax in BQ: and this is how it behaves after the change In [1]: import sqlalchemy as sa; from sqlalchemy_bigquery import BigQueryDialect
In [2]: meta = sa.MetaData(); table = sa.Table("table_name", meta, sa.Column("col_name", sa.Integer()))
In [3]: print(str(sa.select(table.c.col_name).where(table.c.col_name % 2 == 0).compile(dialect=BigQueryDialect())))
SELECT `table_name`.`col_name`
FROM `table_name`
WHERE MOD(`table_name`.`col_name`, :col_name_1) = :param_1
In [4]: sa.select(table.c.col_name).where(table.c.col_name % 2 == 0).compile(dialect=BigQueryDialect()).params
Out[4]: {'col_name_1': 2, 'param_1': 0} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM.
Thanks for pushing this PR. We appreciate the effort and your support in better understanding the nature of the issue this solves. Have a great day! 💜 |
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> 🦕
Adds implementation of mod operator
%
Currently if I try to use
%
in python code, it will fall back to the SA default compiler implementation which renders it as a percent sign, which is not syntactically valid in BQ. I could workaround this by usingfunc.mod
, but this isn't portable across dialects (eg. SQL Server has support for%
, but notMOD
)