Added flag to allow override SnowflakeDialect div_is_floor_div flag#545
Conversation
|
Looks good. You might want to enable these sqlalchemy-provided tests again, which I believe should pass now: |
719b39b to
cfc4f11
Compare
cfc4f11 to
a86f140
Compare
| - v1.7.0(November 22, 2024) | ||
| - (Unreleased) | ||
| - Added `force_div_is_floordiv` flag to override `div_is_floordiv` new default value `False` in `SnowflakeDialect`. | ||
| - With the flag in `False`, the `/` division operator will be treated as a float division and `//` as a floor division. |
There was a problem hiding this comment.
This is not how it works.
This flag is a signal from the dialect towards sqlalchemy that describes the behavior of this sql implementation (Snowflake). So if select 4/5 returns 0, div_is_floordiv must be set to True. If it returns 0.8, this flag must be set to False.
So the previous value of div_is_floordiv = True for snowflake was just a bug, which should be fixed. Just as for other bugs, there is no need to make this configurable.
There was a problem hiding this comment.
Hi @jochenott
We agree that the correct value for the flag div_is_floordiv in Snowflake should be False. However, current users expect the / and // operators to work as true divisions, although / should behave as a true division and // as a floor division. That's why the behavioral change will be introduced as a flag, which will be removed in a future major release.
Thank you for pointing out this code issue and your contribution!!!
d187421 to
85f5fd0
Compare
|
Anything missing to get this merged and released? |
…division operator as floor div. Changed flag div_is_floor_div to False.
…that will be introduce, using new flag force_div_floordiv allow to test the new division behavior. Update sa14:scripts to ignore feature_v20 from execution
85f5fd0 to
7c00df6
Compare
Added tests to validate results of true and floor divisions using `force_div_is_floordiv` flag.
7c00df6 to
24b4bb0
Compare
Please answer these questions before submitting your pull requests. Thanks!
What GitHub issue is this PR addressing? Make sure that there is an accompanying issue to your PR.
Fixes SNOW-1812881: Division broken #543
Fill out the following pre-review checklist:
Please describe how your code solves the related issue.
The default behavior of the flag div_is_floor_div was changed to False. This ensures that Python and Snowflake operators match the right division. In this first version a flag would be added
force_div_is_floor_divto override the current behavior.