From: bekapono Date: Tue, 24 Mar 2026 01:30:46 +0000 (-0400) Subject: Coercion warning feedback for unary distinct outside aggregate function X-Git-Tag: rel_2_1_0b2~13 X-Git-Url: http://git.ipfire.org/gitweb/index.cgi?a=commitdiff_plain;h=9ad90693bc8f2303f2c964271166a80520bee9bd;p=thirdparty%2Fsqlalchemy%2Fsqlalchemy.git 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 index 0000000000..61930d0e34 --- /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 66de4813ee..5273aa6e7f 100644 --- 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 6edfd15580..0dff01612a 100644 --- 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 cf2afe4448..1dd7037515 100644 --- 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 91d3a3de4e..d1f2233d94 100644 --- 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 f5dbf2b46c..24a041bf40 100644 --- 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 d68880c3f1..990b8ff6f2 100644 --- 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