Coercion warning feedback for unary distinct outside aggregate function

A warning is emitted when using the standalone :func:`_.sql.distinct`
function in a :func:`_sql.select` columns list outside of an aggregate
function; this function is not intended as a replacement for the use of
:meth:`.Select.distinct`. Pull request courtesy bekapono.

Fixes: #11526
Closes: #13162
Pull-request: https://github.com/sqlalchemy/sqlalchemy/pull/13162
Pull-request-sha: c6e8be7a49fecb1402dc249c84e7f982c1c34821

Change-Id: Ibbdd64a922c62a7a9ead566590ad854db4066565

diff --git a/doc/build/changelog/unreleased_21/11526.rst b/doc/build/changelog/unreleased_21/11526.rst
new file mode 100644 (file)
index 0000000..61930d0
--- /dev/null
+++ b/doc/build/changelog/unreleased_21/11526.rst
@@ -0,0 +1,8 @@
+.. change::
+    :tags: bug, sql
+    :tickets: 11526
+
+    A warning is emitted when using the standalone :func:`_.sql.distinct`
+    function in a :func:`_sql.select` columns list outside of an aggregate
+    function; this function is not intended as a replacement for the use of
+    :meth:`.Select.distinct`. Pull request courtesy bekapono.

diff --git a/lib/sqlalchemy/sql/compiler.py b/lib/sqlalchemy/sql/compiler.py
index 66de4813eef0212e397b0486e6df2608c780d4bd..5273aa6e7f790f890f3aec973e95ba6c50f0e6fb 100644 (file)
--- a/lib/sqlalchemy/sql/compiler.py
+++ b/lib/sqlalchemy/sql/compiler.py
@@ -3119,6 +3119,8 @@ class SQLCompiler(Compiled):
 
         text: str
 
+        kwargs["within_aggregate_function"] = True
+
         if disp:
             text = disp(func, **kwargs)
         else:
@@ -3257,6 +3259,16 @@ class SQLCompiler(Compiled):
             kw["add_to_result_map"] = add_to_result_map
             kw["result_map_targets"] = result_map_targets
 
+        if unary.operator is operators.distinct_op and not kw.get(
+            "within_aggregate_function", False
+        ):
+            util.warn(
+                "Column-expression-level unary distinct() "
+                "should not be used outside of an aggregate "
+                "function. For general 'SELECT DISTINCT' support"
+                "use select().distinct()."
+            )
+
         if unary.operator:
             if unary.modifier:
                 raise exc.CompileError(

diff --git a/test/orm/test_query.py b/test/orm/test_query.py
index 6edfd15580a438d6e2879d63da62a4ab45cf7da7..0dff01612af471835a3f7a850883fa3c47e047fe 100644 (file)
--- a/test/orm/test_query.py
+++ b/test/orm/test_query.py
@@ -4625,10 +4625,11 @@ class DistinctTest(QueryTest, AssertsCompiledSQL):
             .all(),
         )
 
+    @testing.emits_warning("Column-expression-level unary distinct")
     def test_basic_standalone(self):
         User = self.classes.User
 
-        # issue 6008.  the UnaryExpression now places itself into the
+        # issue #6008.  the UnaryExpression now places itself into the
         # result map so that it can be matched positionally without the need
         # for any label.
         q = fixture_session().query(distinct(User.id)).order_by(User.id)
@@ -4637,10 +4638,12 @@ class DistinctTest(QueryTest, AssertsCompiledSQL):
         )
         eq_([(7,), (8,), (9,), (10,)], q.all())
 
+    @testing.emits_warning("Column-expression-level unary distinct")
     def test_standalone_w_subquery(self):
+        # additional test for #6008
         User = self.classes.User
-        q = fixture_session().query(distinct(User.id))
 
+        q = fixture_session().query(distinct(User.id))
         subq = q.subquery()
         q = fixture_session().query(subq).order_by(subq.c[0])
         eq_([(7,), (8,), (9,), (10,)], q.all())

diff --git a/test/sql/test_compiler.py b/test/sql/test_compiler.py
index cf2afe4448bc1c3d56190d40dfce693fa2e2423d..1dd703751564cc992bd285818334691e9b84116e 100644 (file)
--- a/test/sql/test_compiler.py
+++ b/test/sql/test_compiler.py
@@ -1940,24 +1940,21 @@ class SelectTest(fixtures.TestBase, AssertsCompiledSQL):
             dialect=default.DefaultDialect(supports_native_boolean=True),
         )
 
-    def test_distinct(self):
+    def test_distinct_select_modifier(self):
         self.assert_compile(
-            select(table1.c.myid.distinct()),
-            "SELECT DISTINCT mytable.myid FROM mytable",
-        )
-
-        self.assert_compile(
-            select(distinct(table1.c.myid)),
+            select(table1.c.myid).distinct(),
             "SELECT DISTINCT mytable.myid FROM mytable",
         )
 
         self.assert_compile(
-            select(distinct(table1.c.myid)).set_label_style(
-                LABEL_STYLE_TABLENAME_PLUS_COL
-            ),
-            "SELECT DISTINCT mytable.myid FROM mytable",
+            select(table1.c.myid)
+            .distinct()
+            .set_label_style(LABEL_STYLE_TABLENAME_PLUS_COL),
+            "SELECT DISTINCT mytable.myid AS mytable_myid FROM mytable",
         )
 
+    @testing.emits_warning("Column-expression-level unary distinct")
+    def test_distinct_function_6008(self):
         # the bug fixed here as part of #6008 is the same bug that's
         # in 1.3 as well, producing
         # "SELECT anon_2.anon_1 FROM (SELECT distinct mytable.myid
@@ -1969,8 +1966,15 @@ class SelectTest(fixtures.TestBase, AssertsCompiledSQL):
         )
 
         self.assert_compile(
-            select(table1.c.myid).distinct(),
-            "SELECT DISTINCT mytable.myid FROM mytable",
+            select(select(table1.c.myid.distinct()).subquery()),
+            "SELECT anon_2.anon_1 FROM (SELECT "
+            "DISTINCT mytable.myid AS anon_1 FROM mytable) AS anon_2",
+        )
+
+    def test_distinct_function(self):
+        self.assert_compile(
+            select(func.sum(distinct(table1.c.myid))),
+            "SELECT sum(DISTINCT mytable.myid) AS sum_1 FROM mytable",
         )
 
         self.assert_compile(
@@ -1983,6 +1987,23 @@ class SelectTest(fixtures.TestBase, AssertsCompiledSQL):
             "SELECT count(DISTINCT mytable.myid) AS count_1 FROM mytable",
         )
 
+    def test_distinct_function_warn_outside_aggregate(self):
+        with testing.expect_warnings(
+            "Column-expression-level unary distinct.*SELECT DISTINCT"
+        ):
+            self.assert_compile(
+                select(distinct(table1.c.myid)),
+                "SELECT DISTINCT mytable.myid FROM mytable",
+            )
+
+        with testing.expect_warnings(
+            "Column-expression-level unary distinct.*SELECT DISTINCT"
+        ):
+            self.assert_compile(
+                select(table1.c.myid.distinct()),
+                "SELECT DISTINCT mytable.myid FROM mytable",
+            )
+
     def test_distinct_on(self):
         with testing.expect_deprecated(
             "Passing expression to",

diff --git a/test/sql/test_operators.py b/test/sql/test_operators.py
index 91d3a3de4e45bcef711275d89b5ad33dd0525f55..d1f2233d94495322e752be81615ed2f18c1de9e8 100644 (file)
--- a/test/sql/test_operators.py
+++ b/test/sql/test_operators.py
@@ -5314,6 +5314,7 @@ class BitOpTest(fixtures.TestBase, testing.AssertsCompiledSQL):
         argnames="py_op, sql_op",
     )
     @testing.variation("named", ["column", "unnamed", "label"])
+    @testing.emits_warning("Column-expression-level unary distinct")
     def test_wraps_named_column_heuristic(self, py_op, sql_op, named):
         """test for #12681"""
 
@@ -5325,7 +5326,6 @@ class BitOpTest(fixtures.TestBase, testing.AssertsCompiledSQL):
                 select(expr),
                 f"SELECT {sql_op}q",
             )
-
         elif named.unnamed:
             expr = py_op(literal("x", Integer))
             assert isinstance(expr, UnaryExpression)

diff --git a/test/sql/test_resultset.py b/test/sql/test_resultset.py
index f5dbf2b46ce552eaa8c0b7c08ad9f6b12ff85332..24a041bf40221097983356ee6c025fff49674e3e 100644 (file)
--- a/test/sql/test_resultset.py
+++ b/test/sql/test_resultset.py
@@ -972,6 +972,7 @@ class CursorResultTest(fixtures.TablesTest):
         not_in("user_name", r._mapping)
         eq_(list(r._fields), ["users.user_id", "users.user_name"])
 
+    @testing.emits_warning("Column-expression-level unary distinct")
     def test_column_accessor_unary(self, connection):
         users = self.tables.users
 

diff --git a/test/sql/test_types.py b/test/sql/test_types.py
index d68880c3f185be5433bdadb7150a233e2e2afb26..990b8ff6f2cb72fc812abadfd076cc41a76ab51b 100644 (file)
--- a/test/sql/test_types.py
+++ b/test/sql/test_types.py
@@ -3943,11 +3943,13 @@ class ExpressionTest(
 
     def test_distinct(self, connection):
         test_table = self.tables.test
+        s = select(test_table.c.avalue).distinct()
+        eq_(connection.execute(s).scalar(), 25)
 
-        s = select(distinct(test_table.c.avalue))
+        s = select(func.sum(test_table.c.avalue.distinct()))
         eq_(connection.execute(s).scalar(), 25)
 
-        s = select(test_table.c.avalue.distinct())
+        s = select(func.sum(distinct(test_table.c.avalue)))
         eq_(connection.execute(s).scalar(), 25)
 
         assert distinct(test_table.c.data).type == test_table.c.data.type