Skip to content

Commit

Permalink
Added warning for use of div_is_floor_div with True value.
Browse files Browse the repository at this point in the history
Added tests to validate results of true and floor divisions using `force_div_is_floordiv` flag.
  • Loading branch information
sfc-gh-jmartinezramirez committed Jan 8, 2025
1 parent 5e4e519 commit 7c00df6
Show file tree
Hide file tree
Showing 4 changed files with 111 additions and 2 deletions.
21 changes: 21 additions & 0 deletions src/snowflake/sqlalchemy/base.py
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@
import operator
import re
import string
import warnings
from typing import List

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

def visit_truediv_binary(self, binary, operator, **kw):
if self.dialect.div_is_floordiv:
warnings.warn(
"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",
PendingDeprecationWarning,
stacklevel=2,
)
return (
self.process(binary.left, **kw) + " / " + self.process(binary.right, **kw)
)

def visit_floordiv_binary(self, binary, operator, **kw):
if self.dialect.div_is_floordiv and IS_VERSION_20:
warnings.warn(
"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",
PendingDeprecationWarning,
stacklevel=2,
)
return super().visit_floordiv_binary(binary, operator, **kw)

def render_literal_value(self, value, type_):
# escape backslash
return super().render_literal_value(value, type_).replace("\\", "\\\\")
Expand Down
1 change: 0 additions & 1 deletion src/snowflake/sqlalchemy/snowdialect.py
Original file line number Diff line number Diff line change
Expand Up @@ -148,7 +148,6 @@ class SnowflakeDialect(default.DefaultDialect):

supports_identity_columns = True


def __init__(
self,
force_div_is_floordiv: bool = True,
Expand Down
4 changes: 4 additions & 0 deletions tests/test_compiler.py
Original file line number Diff line number Diff line change
Expand Up @@ -139,6 +139,7 @@ def test_outer_lateral_join():
)


@pytest.mark.feature_v20
def test_division_operator_with_force_div_is_floordiv_false():
col1 = column("col1", Integer)
col2 = column("col2", Integer)
Expand All @@ -149,6 +150,7 @@ def test_division_operator_with_force_div_is_floordiv_false():
)


@pytest.mark.feature_v20
def test_division_operator_with_denominator_expr_force_div_is_floordiv_false():
col1 = column("col1", Integer)
col2 = column("col2", Integer)
Expand All @@ -159,6 +161,7 @@ def test_division_operator_with_denominator_expr_force_div_is_floordiv_false():
)


@pytest.mark.feature_v20
def test_division_operator_with_force_div_is_floordiv_default_true():
col1 = column("col1", Integer)
col2 = column("col2", Integer)
Expand All @@ -168,6 +171,7 @@ def test_division_operator_with_force_div_is_floordiv_default_true():
)


@pytest.mark.feature_v20
def test_division_operator_with_denominator_expr_force_div_is_floordiv_default_true():
col1 = column("col1", Integer)
col2 = column("col2", Integer)
Expand Down
87 changes: 86 additions & 1 deletion tests/test_core.py
Original file line number Diff line number Diff line change
Expand Up @@ -31,13 +31,15 @@
create_engine,
dialects,
exc,
func,
insert,
inspect,
text,
)
from sqlalchemy.exc import DBAPIError, NoSuchTableError, OperationalError
from sqlalchemy.sql import and_, not_, or_, select
from sqlalchemy.sql import and_, literal, not_, or_, select
from sqlalchemy.sql.ddl import CreateTable
from sqlalchemy.testing.assertions import eq_

import snowflake.connector.errors
import snowflake.sqlalchemy.snowdialect
Expand Down Expand Up @@ -1864,3 +1866,86 @@ def test_snowflake_sqlalchemy_as_valid_client_type():
snowflake.connector.connection.DEFAULT_CONFIGURATION[
"internal_application_version"
] = origin_internal_app_version


@pytest.mark.parametrize(
"operation",
[
[
literal(5),
literal(10),
0.5,
],
[literal(5), func.sqrt(literal(10)), 1.5811388300841895],
[literal(4), literal(5), decimal.Decimal("0.800000")],
[literal(2), literal(2), 1.0],
[literal(3), literal(2), 1.5],
[literal(4), literal(1.5), 2.666667],
[literal(5.5), literal(10.7), 0.5140187],
[literal(5.5), literal(8), 0.6875],
],
)
def test_true_division_operation(engine_testaccount, operation):
# 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"
# with pytest.warns(PendingDeprecationWarning, match=expected_warning):
with engine_testaccount.connect() as conn:
eq_(
conn.execute(select(operation[0] / operation[1])).fetchall(),
[((operation[2]),)],
)


@pytest.mark.parametrize(
"operation",
[
[literal(5), literal(10), 0.5, 0.5],
[literal(5), func.sqrt(literal(10)), 1.5811388300841895, 1.0],
[
literal(4),
literal(5),
decimal.Decimal("0.800000"),
decimal.Decimal("0.800000"),
],
[literal(2), literal(2), 1.0, 1.0],
[literal(3), literal(2), 1.5, 1.5],
[literal(4), literal(1.5), 2.666667, 2.0],
[literal(5.5), literal(10.7), 0.5140187, 0],
[literal(5.5), literal(8), 0.6875, 0.6875],
],
)
@pytest.mark.feature_v20
def test_division_force_div_is_floordiv_default(engine_testaccount, operation):
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"
with pytest.warns(PendingDeprecationWarning, match=expected_warning):
with engine_testaccount.connect() as conn:
eq_(
conn.execute(
select(operation[0] / operation[1], operation[0] // operation[1])
).fetchall(),
[(operation[2], operation[3])],
)


@pytest.mark.parametrize(
"operation",
[
[literal(5), literal(10), 0.5, 0],
[literal(5), func.sqrt(literal(10)), 1.5811388300841895, 1.0],
[literal(4), literal(5), decimal.Decimal("0.800000"), 0],
[literal(2), literal(2), 1.0, 1.0],
[literal(3), literal(2), 1.5, 1],
[literal(4), literal(1.5), 2.666667, 2.0],
[literal(5.5), literal(10.7), 0.5140187, 0],
[literal(5.5), literal(8), 0.6875, 0],
],
)
@pytest.mark.feature_v20
def test_division_force_div_is_floordiv_false(db_parameters, operation):
engine = create_engine(URL(**db_parameters), **{"force_div_is_floordiv": False})
with engine.connect() as conn:
eq_(
conn.execute(
select(operation[0] / operation[1], operation[0] // operation[1])
).fetchall(),
[(operation[2], operation[3])],
)

0 comments on commit 7c00df6

Please sign in to comment.