Skip to content

Commit b9b26e5

Browse files
Added flag to allow override SnowflakeDialect div_is_floor_div flag (#545)
* Changed default behavior of SnowflakeDialect to disable the use of / division operator as floor div. Changed flag div_is_floor_div to False. * Update Description.md * Added flag to allow customer to test new behavior of div_is_floordiv 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 * Added warning for use of div_is_floor_div with `True` value. Added tests to validate results of true and floor divisions using `force_div_is_floordiv` flag.
1 parent d84484e commit b9b26e5

File tree

7 files changed

+226
-1
lines changed

7 files changed

+226
-1
lines changed

DESCRIPTION.md

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,10 @@ Source code is also available at:
1010
# Release Notes
1111
- (Unreleased)
1212
- Fix return value of snowflake get_table_names
13+
- Added `force_div_is_floordiv` flag to override `div_is_floordiv` new default value `False` in `SnowflakeDialect`.
14+
- With the flag in `False`, the `/` division operator will be treated as a float division and `//` as a floor division.
15+
- This flag is added to maintain backward compatibility with the previous behavior of Snowflake Dialect division.
16+
- This flag will be removed in the future and Snowflake Dialect will use `div_is_floor_div` as `False`.
1317

1418
- v1.7.2(December 18, 2024)
1519
- Fix quoting of `_` as column name

pyproject.toml

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -87,6 +87,11 @@ extra-dependencies = ["SQLAlchemy>=1.4.19,<2.0.0"]
8787
features = ["development", "pandas"]
8888
python = "3.8"
8989

90+
[tool.hatch.envs.sa14.scripts]
91+
test-dialect = "pytest --ignore_v20_test -ra -vvv --tb=short --cov snowflake.sqlalchemy --cov-append --junitxml ./junit.xml --ignore=tests/sqlalchemy_test_suite tests/"
92+
test-dialect-compatibility = "pytest --ignore_v20_test -ra -vvv --tb=short --cov snowflake.sqlalchemy --cov-append --junitxml ./junit.xml tests/sqlalchemy_test_suite"
93+
test-dialect-aws = "pytest --ignore_v20_test -m \"aws\" -ra -vvv --tb=short --cov snowflake.sqlalchemy --cov-append --junitxml ./junit.xml --ignore=tests/sqlalchemy_test_suite tests/"
94+
9095
[tool.hatch.envs.default.env-vars]
9196
COVERAGE_FILE = "coverage.xml"
9297
SQLACHEMY_WARN_20 = "1"
@@ -134,4 +139,5 @@ markers = [
134139
"requires_external_volume: tests that needs a external volume to be executed",
135140
"external: tests that could but should only run on our external CI",
136141
"feature_max_lob_size: tests that could but should only run on our external CI",
142+
"feature_v20: tests that could but should only run on SqlAlchemy v20",
137143
]

src/snowflake/sqlalchemy/base.py

Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@
66
import operator
77
import re
88
import string
9+
import warnings
910
from typing import List
1011

1112
from sqlalchemy import exc as sa_exc
@@ -802,6 +803,26 @@ def visit_join(self, join, asfrom=False, from_linter=None, **kwargs):
802803
+ join.onclause._compiler_dispatch(self, from_linter=from_linter, **kwargs)
803804
)
804805

806+
def visit_truediv_binary(self, binary, operator, **kw):
807+
if self.dialect.div_is_floordiv:
808+
warnings.warn(
809+
"div_is_floordiv value will be changed to False in a future release. This will generate a behavior change on true and floor division. Please review https://docs.sqlalchemy.org/en/20/changelog/whatsnew_20.html#python-division-operator-performs-true-division-for-all-backends-added-floor-division",
810+
PendingDeprecationWarning,
811+
stacklevel=2,
812+
)
813+
return (
814+
self.process(binary.left, **kw) + " / " + self.process(binary.right, **kw)
815+
)
816+
817+
def visit_floordiv_binary(self, binary, operator, **kw):
818+
if self.dialect.div_is_floordiv and IS_VERSION_20:
819+
warnings.warn(
820+
"div_is_floordiv value will be changed to False in a future release. This will generate a behavior change on true and floor division. Please review https://docs.sqlalchemy.org/en/20/changelog/whatsnew_20.html#python-division-operator-performs-true-division-for-all-backends-added-floor-division",
821+
PendingDeprecationWarning,
822+
stacklevel=2,
823+
)
824+
return super().visit_floordiv_binary(binary, operator, **kw)
825+
805826
def render_literal_value(self, value, type_):
806827
# escape backslash
807828
return super().render_literal_value(value, type_).replace("\\", "\\\\")

src/snowflake/sqlalchemy/snowdialect.py

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -79,6 +79,9 @@ class SnowflakeDialect(default.DefaultDialect):
7979
colspecs = colspecs
8080
ischema_names = ischema_names
8181

82+
# target database treats the / division operator as “floor division”
83+
div_is_floordiv = False
84+
8285
# all str types must be converted in Unicode
8386
convert_unicode = True
8487

@@ -147,10 +150,17 @@ class SnowflakeDialect(default.DefaultDialect):
147150

148151
def __init__(
149152
self,
153+
force_div_is_floordiv: bool = True,
150154
isolation_level: Optional[str] = SnowflakeIsolationLevel.READ_COMMITTED.value,
151155
**kwargs: Any,
152156
):
153157
super().__init__(isolation_level=isolation_level, **kwargs)
158+
self.force_div_is_floordiv = force_div_is_floordiv
159+
self.div_is_floordiv = force_div_is_floordiv
160+
161+
def initialize(self, connection):
162+
super().initialize(connection)
163+
self.div_is_floordiv = self.force_div_is_floordiv
154164

155165
@classmethod
156166
def dbapi(cls):

tests/conftest.py

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -47,6 +47,26 @@
4747
TEST_SCHEMA = f"sqlalchemy_tests_{str(uuid.uuid4()).replace('-', '_')}"
4848

4949

50+
def pytest_addoption(parser):
51+
parser.addoption(
52+
"--ignore_v20_test",
53+
action="store_true",
54+
default=False,
55+
help="skip sqlalchemy 2.0 exclusive tests",
56+
)
57+
58+
59+
def pytest_collection_modifyitems(config, items):
60+
if config.getoption("--ignore_v20_test"):
61+
# --ignore_v20_test given in cli: skip sqlalchemy 2.0 tests
62+
skip_feature_v2 = pytest.mark.skip(
63+
reason="need remove --ignore_v20_test option to run"
64+
)
65+
for item in items:
66+
if "feature_v20" in item.keywords:
67+
item.add_marker(skip_feature_v2)
68+
69+
5070
@pytest.fixture(scope="session")
5171
def on_travis():
5272
return os.getenv("TRAVIS", "").lower() == "true"

tests/test_compiler.py

Lines changed: 79 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2,12 +2,14 @@
22
# Copyright (c) 2012-2023 Snowflake Computing Inc. All rights reserved.
33
#
44

5+
import pytest
56
from sqlalchemy import Integer, String, and_, func, insert, select
67
from sqlalchemy.schema import DropColumnComment, DropTableComment
78
from sqlalchemy.sql import column, quoted_name, table
89
from sqlalchemy.testing.assertions import AssertsCompiledSQL
910

1011
from snowflake.sqlalchemy import snowdialect
12+
from src.snowflake.sqlalchemy.snowdialect import SnowflakeDialect
1113

1214
table1 = table(
1315
"table1", column("id", Integer), column("name", String), column("value", Integer)
@@ -135,3 +137,80 @@ def test_outer_lateral_join():
135137
str(stmt.compile(dialect=snowdialect.dialect()))
136138
== "SELECT colname AS label \nFROM abc JOIN LATERAL flatten(PARSE_JSON(colname2)) AS anon_1 GROUP BY colname"
137139
)
140+
141+
142+
@pytest.mark.feature_v20
143+
def test_division_operator_with_force_div_is_floordiv_false():
144+
col1 = column("col1", Integer)
145+
col2 = column("col2", Integer)
146+
stmt = col1 / col2
147+
assert (
148+
str(stmt.compile(dialect=SnowflakeDialect(force_div_is_floordiv=False)))
149+
== "col1 / col2"
150+
)
151+
152+
153+
@pytest.mark.feature_v20
154+
def test_division_operator_with_denominator_expr_force_div_is_floordiv_false():
155+
col1 = column("col1", Integer)
156+
col2 = column("col2", Integer)
157+
stmt = col1 / func.sqrt(col2)
158+
assert (
159+
str(stmt.compile(dialect=SnowflakeDialect(force_div_is_floordiv=False)))
160+
== "col1 / sqrt(col2)"
161+
)
162+
163+
164+
@pytest.mark.feature_v20
165+
def test_division_operator_with_force_div_is_floordiv_default_true():
166+
col1 = column("col1", Integer)
167+
col2 = column("col2", Integer)
168+
stmt = col1 / col2
169+
assert str(stmt.compile(dialect=SnowflakeDialect())) == "col1 / col2"
170+
171+
172+
@pytest.mark.feature_v20
173+
def test_division_operator_with_denominator_expr_force_div_is_floordiv_default_true():
174+
col1 = column("col1", Integer)
175+
col2 = column("col2", Integer)
176+
stmt = col1 / func.sqrt(col2)
177+
assert str(stmt.compile(dialect=SnowflakeDialect())) == "col1 / sqrt(col2)"
178+
179+
180+
@pytest.mark.feature_v20
181+
def test_floor_division_operator_force_div_is_floordiv_false():
182+
col1 = column("col1", Integer)
183+
col2 = column("col2", Integer)
184+
stmt = col1 // col2
185+
assert (
186+
str(stmt.compile(dialect=SnowflakeDialect(force_div_is_floordiv=False)))
187+
== "FLOOR(col1 / col2)"
188+
)
189+
190+
191+
@pytest.mark.feature_v20
192+
def test_floor_division_operator_with_denominator_expr_force_div_is_floordiv_false():
193+
col1 = column("col1", Integer)
194+
col2 = column("col2", Integer)
195+
stmt = col1 // func.sqrt(col2)
196+
assert (
197+
str(stmt.compile(dialect=SnowflakeDialect(force_div_is_floordiv=False)))
198+
== "FLOOR(col1 / sqrt(col2))"
199+
)
200+
201+
202+
@pytest.mark.feature_v20
203+
def test_floor_division_operator_force_div_is_floordiv_default_true():
204+
col1 = column("col1", Integer)
205+
col2 = column("col2", Integer)
206+
stmt = col1 // col2
207+
assert str(stmt.compile(dialect=SnowflakeDialect())) == "col1 / col2"
208+
209+
210+
@pytest.mark.feature_v20
211+
def test_floor_division_operator_with_denominator_expr_force_div_is_floordiv_default_true():
212+
col1 = column("col1", Integer)
213+
col2 = column("col2", Integer)
214+
stmt = col1 // func.sqrt(col2)
215+
res = stmt.compile(dialect=SnowflakeDialect())
216+
assert str(res) == "FLOOR(col1 / sqrt(col2))"

tests/test_core.py

Lines changed: 86 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -31,13 +31,15 @@
3131
create_engine,
3232
dialects,
3333
exc,
34+
func,
3435
insert,
3536
inspect,
3637
text,
3738
)
3839
from sqlalchemy.exc import DBAPIError, NoSuchTableError, OperationalError
39-
from sqlalchemy.sql import and_, not_, or_, select
40+
from sqlalchemy.sql import and_, literal, not_, or_, select
4041
from sqlalchemy.sql.ddl import CreateTable
42+
from sqlalchemy.testing.assertions import eq_
4143

4244
import snowflake.connector.errors
4345
import snowflake.sqlalchemy.snowdialect
@@ -1864,3 +1866,86 @@ def test_snowflake_sqlalchemy_as_valid_client_type():
18641866
snowflake.connector.connection.DEFAULT_CONFIGURATION[
18651867
"internal_application_version"
18661868
] = origin_internal_app_version
1869+
1870+
1871+
@pytest.mark.parametrize(
1872+
"operation",
1873+
[
1874+
[
1875+
literal(5),
1876+
literal(10),
1877+
0.5,
1878+
],
1879+
[literal(5), func.sqrt(literal(10)), 1.5811388300841895],
1880+
[literal(4), literal(5), decimal.Decimal("0.800000")],
1881+
[literal(2), literal(2), 1.0],
1882+
[literal(3), literal(2), 1.5],
1883+
[literal(4), literal(1.5), 2.666667],
1884+
[literal(5.5), literal(10.7), 0.5140187],
1885+
[literal(5.5), literal(8), 0.6875],
1886+
],
1887+
)
1888+
def test_true_division_operation(engine_testaccount, operation):
1889+
# expected_warning = "div_is_floordiv value will be changed to False in a future release. This will generate a behavior change on true and floor division. Please review https://docs.sqlalchemy.org/en/20/changelog/whatsnew_20.html#python-division-operator-performs-true-division-for-all-backends-added-floor-division"
1890+
# with pytest.warns(PendingDeprecationWarning, match=expected_warning):
1891+
with engine_testaccount.connect() as conn:
1892+
eq_(
1893+
conn.execute(select(operation[0] / operation[1])).fetchall(),
1894+
[((operation[2]),)],
1895+
)
1896+
1897+
1898+
@pytest.mark.parametrize(
1899+
"operation",
1900+
[
1901+
[literal(5), literal(10), 0.5, 0.5],
1902+
[literal(5), func.sqrt(literal(10)), 1.5811388300841895, 1.0],
1903+
[
1904+
literal(4),
1905+
literal(5),
1906+
decimal.Decimal("0.800000"),
1907+
decimal.Decimal("0.800000"),
1908+
],
1909+
[literal(2), literal(2), 1.0, 1.0],
1910+
[literal(3), literal(2), 1.5, 1.5],
1911+
[literal(4), literal(1.5), 2.666667, 2.0],
1912+
[literal(5.5), literal(10.7), 0.5140187, 0],
1913+
[literal(5.5), literal(8), 0.6875, 0.6875],
1914+
],
1915+
)
1916+
@pytest.mark.feature_v20
1917+
def test_division_force_div_is_floordiv_default(engine_testaccount, operation):
1918+
expected_warning = "div_is_floordiv value will be changed to False in a future release. This will generate a behavior change on true and floor division. Please review https://docs.sqlalchemy.org/en/20/changelog/whatsnew_20.html#python-division-operator-performs-true-division-for-all-backends-added-floor-division"
1919+
with pytest.warns(PendingDeprecationWarning, match=expected_warning):
1920+
with engine_testaccount.connect() as conn:
1921+
eq_(
1922+
conn.execute(
1923+
select(operation[0] / operation[1], operation[0] // operation[1])
1924+
).fetchall(),
1925+
[(operation[2], operation[3])],
1926+
)
1927+
1928+
1929+
@pytest.mark.parametrize(
1930+
"operation",
1931+
[
1932+
[literal(5), literal(10), 0.5, 0],
1933+
[literal(5), func.sqrt(literal(10)), 1.5811388300841895, 1.0],
1934+
[literal(4), literal(5), decimal.Decimal("0.800000"), 0],
1935+
[literal(2), literal(2), 1.0, 1.0],
1936+
[literal(3), literal(2), 1.5, 1],
1937+
[literal(4), literal(1.5), 2.666667, 2.0],
1938+
[literal(5.5), literal(10.7), 0.5140187, 0],
1939+
[literal(5.5), literal(8), 0.6875, 0],
1940+
],
1941+
)
1942+
@pytest.mark.feature_v20
1943+
def test_division_force_div_is_floordiv_false(db_parameters, operation):
1944+
engine = create_engine(URL(**db_parameters), **{"force_div_is_floordiv": False})
1945+
with engine.connect() as conn:
1946+
eq_(
1947+
conn.execute(
1948+
select(operation[0] / operation[1], operation[0] // operation[1])
1949+
).fetchall(),
1950+
[(operation[2], operation[3])],
1951+
)

0 commit comments

Comments
 (0)