Fix floordiv (//) for float/numeric by int with div_is_floordiv dialects

Fixed issue where floor division (``//``) between a :class:`.Float` or
:class:`.Numeric` numerator and an :class:`.Integer` denominator would omit
the ``FLOOR()`` SQL wrapper on dialects where
:attr:`.Dialect.div_is_floordiv` is ``True`` (the default, including
PostgreSQL and SQLite).  ``FLOOR()`` is now applied if either the
denominator or the numerator is a non-integer, so that expressions such as
``float_col // int_col`` render as ``FLOOR(float_col / int_col)`` instead
of the incorrect ``float_col / int_col``.  Pull request courtesy r266-tech.

Fixes: #10528
Closes: #13191
Pull-request: https://github.com/sqlalchemy/sqlalchemy/pull/13191
Pull-request-sha: c9cbc47c877e19c91f912556b4ead6cd26e3cfe6

Change-Id: I5f9f02d966aa6ccee214a2c5cc27a73a4292da03

diff --git a/doc/build/changelog/unreleased_20/10528.rst b/doc/build/changelog/unreleased_20/10528.rst
new file mode 100644 (file)
index 0000000..db7000e
--- /dev/null
+++ b/doc/build/changelog/unreleased_20/10528.rst
@@ -0,0 +1,12 @@
+.. change::
+    :tags: bug, sql
+    :tickets: 10528
+
+    Fixed issue where floor division (``//``) between a :class:`.Float` or
+    :class:`.Numeric` numerator and an :class:`.Integer` denominator would omit
+    the ``FLOOR()`` SQL wrapper on dialects where
+    :attr:`.Dialect.div_is_floordiv` is ``True`` (the default, including
+    PostgreSQL and SQLite).  ``FLOOR()`` is now applied if either the
+    denominator or the numerator is a non-integer, so that expressions such as
+    ``float_col // int_col`` render as ``FLOOR(float_col / int_col)`` instead
+    of the incorrect ``float_col / int_col``.  Pull request courtesy r266-tech.

diff --git a/doc/build/core/operators.rst b/doc/build/core/operators.rst
index d6493a4d6325a74b5dd5fe1326fb51da8622d3ee..a73352803984426ffa8819d4b6bda75bfb59f935 100644 (file)
--- a/doc/build/core/operators.rst
+++ b/doc/build/core/operators.rst
@@ -558,7 +558,7 @@ Arithmetic Operators
   For the default backend as well as backends such as PostgreSQL, the SQL ``/`` operator normally
   behaves this way for integer values::
 
-    >>> print(column("x") // 5)
+    >>> print(column("x", Integer) // 5)
     {printsql}x / :x_1{stop}
     >>> print(5 // column("x", Integer))
     {printsql}:x_1 / x{stop}

diff --git a/lib/sqlalchemy/sql/compiler.py b/lib/sqlalchemy/sql/compiler.py
index 4014fbf40baa288c0c92bb38377cadb4517a4edc..890a92f634cfce541ac3c88aa5bd103ce24d57d9 100644 (file)
--- a/lib/sqlalchemy/sql/compiler.py
+++ b/lib/sqlalchemy/sql/compiler.py
@@ -3331,6 +3331,7 @@ class SQLCompiler(Compiled):
         if (
             self.dialect.div_is_floordiv
             and binary.right.type._type_affinity is sqltypes.Integer
+            and binary.left.type._type_affinity is sqltypes.Integer
         ):
             return (
                 self.process(binary.left, **kw)

diff --git a/test/sql/test_cte.py b/test/sql/test_cte.py
index 62e5b0030b2e2b012ebd108ed0d2105087737c4e..02d0999270f2de7621025869cbb9c069caa3f2fa 100644 (file)
--- a/test/sql/test_cte.py
+++ b/test/sql/test_cte.py
@@ -86,7 +86,7 @@ class CTETest(fixtures.TestBase, AssertsCompiledSQL):
             "top_regions AS (SELECT "
             "regional_sales.region AS region FROM regional_sales "
             "WHERE regional_sales.total_sales > "
-            "(SELECT sum(regional_sales.total_sales) / :sum_1 AS "
+            "(SELECT FLOOR(sum(regional_sales.total_sales) / :sum_1) AS "
             "anon_1 FROM regional_sales)) "
             "SELECT orders.region, orders.product, "
             "sum(orders.quantity) AS product_units, "

diff --git a/test/sql/test_operators.py b/test/sql/test_operators.py
index a815cb63176e7193f735bdacec6d136d02f4a105..38aeeba9c0639d6550fc2cabe59b16cb7721eed4 100644 (file)
--- a/test/sql/test_operators.py
+++ b/test/sql/test_operators.py
@@ -2825,6 +2825,34 @@ class MathOperatorTest(fixtures.TestBase, testing.AssertsCompiledSQL):
     def test_floordiv_op_numeric(self):
         self.assert_compile(5.10 // literal(5.5), "FLOOR(:param_1 / :param_2)")
 
+    def test_floordiv_float_by_int(self):
+        """float // int must include FLOOR even with div_is_floordiv dialect"""
+        expr = column("a", Float()) // column("b", Integer())
+        self.assert_compile(
+            expr,
+            "FLOOR(a / b)",
+            dialect=postgresql.dialect(),
+        )
+
+    def test_floordiv_numeric_by_int(self):
+        """numeric // int must include FLOOR even with div_is_floordiv
+        dialect"""
+        expr = column("a", Numeric()) // column("b", Integer())
+        self.assert_compile(
+            expr,
+            "FLOOR(a / b)",
+            dialect=postgresql.dialect(),
+        )
+
+    def test_floordiv_int_by_int_pg(self):
+        """int // int can skip FLOOR when dialect has div_is_floordiv"""
+        expr = column("a", Integer()) // column("b", Integer())
+        self.assert_compile(
+            expr,
+            "a / b",
+            dialect=postgresql.dialect(),
+        )
+
     @testing.combinations(
         ("format", "mytable.myid %% %s"),
         ("qmark", "mytable.myid % ?"),

diff --git a/test/sql/test_selectable.py b/test/sql/test_selectable.py
index 80c87da0f9414a648fc3c7b1418cbd5e11488f12..9f076bf7fc8a4dee3488894205fbed8ac223d7df 100644 (file)
--- a/test/sql/test_selectable.py
+++ b/test/sql/test_selectable.py
@@ -884,8 +884,8 @@ class SelectableTest(
         no_group = column("a") // type_coerce(column("x"), Integer)
         group = column("b") // type_coerce(column("y") * column("w"), Integer)
 
-        self.assert_compile(no_group, "a / x")
-        self.assert_compile(group, "b / (y * w)")
+        self.assert_compile(no_group, "FLOOR(a / x)")
+        self.assert_compile(group, "FLOOR(b / (y * w))")
 
     def test_subquery_on_table(self):
         sel = (