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
(cherry picked from commit a4c7dbc49e820e4eaf111b618b20d80e6b5ddc43)

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 7fa163d6e687bad28f897f343e819bfb1971788b..bcedf45f20ccff4db35890e3c99326af003add74 100644 (file)
--- a/doc/build/core/operators.rst
+++ b/doc/build/core/operators.rst
@@ -556,7 +556,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 02b40029de403fb5ba65a5dac4342ff97c00eda6..7f7243bbe4fa980bdcde5903b5636f01ccdeb77c 100644 (file)
--- a/lib/sqlalchemy/sql/compiler.py
+++ b/lib/sqlalchemy/sql/compiler.py
@@ -3170,6 +3170,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 d588776b1ab8b0826dd432af0fdbbb6f83720d44..e546be94474dd411e42dd7d6b1bb1a0fed89b02b 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 b7a3bd2f6b7cba97a1043453c5e55163972699bc..2811c65dffd691a29db342bfd4a008b557660a9d 100644 (file)
--- a/test/sql/test_operators.py
+++ b/test/sql/test_operators.py
@@ -10,6 +10,7 @@ from sqlalchemy import between
 from sqlalchemy import bindparam
 from sqlalchemy import Enum
 from sqlalchemy import exc
+from sqlalchemy import Float
 from sqlalchemy import Integer
 from sqlalchemy import intersect
 from sqlalchemy import join
@@ -2658,6 +2659,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 d721819a6734fc0ab7ad8a6a3918973ea90ebc4e..853d9e1c74ffc85f2b35d5ecdab2620d10b04bc2 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 = (