Skip to content

Commit 9ad9069

Browse files
bekaponozzzeek
bekapono
authored and
zzzeek
committed
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: #13162 Pull-request-sha: c6e8be7 Change-Id: Ibbdd64a922c62a7a9ead566590ad854db4066565
1 parent f244d5c commit 9ad9069

7 files changed

Lines changed: 65 additions & 18 deletions

File tree

‎doc/build/changelog/unreleased_21/11526.rst‎

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,8 @@
1+
.. change::
2+
:tags: bug, sql
3+
:tickets: 11526
4+
5+
A warning is emitted when using the standalone :func:`_.sql.distinct`
6+
function in a :func:`_sql.select` columns list outside of an aggregate
7+
function; this function is not intended as a replacement for the use of
8+
:meth:`.Select.distinct`. Pull request courtesy bekapono.

‎lib/sqlalchemy/sql/compiler.py‎

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3119,6 +3119,8 @@ def visit_function(
31193119

31203120
text: str
31213121

3122+
kwargs["within_aggregate_function"] = True
3123+
31223124
if disp:
31233125
text = disp(func, **kwargs)
31243126
else:
@@ -3257,6 +3259,16 @@ def visit_unary(
32573259
kw["add_to_result_map"] = add_to_result_map
32583260
kw["result_map_targets"] = result_map_targets
32593261

3262+
if unary.operator is operators.distinct_op and not kw.get(
3263+
"within_aggregate_function", False
3264+
):
3265+
util.warn(
3266+
"Column-expression-level unary distinct() "
3267+
"should not be used outside of an aggregate "
3268+
"function. For general 'SELECT DISTINCT' support"
3269+
"use select().distinct()."
3270+
)
3271+
32603272
if unary.operator:
32613273
if unary.modifier:
32623274
raise exc.CompileError(

‎test/orm/test_query.py‎

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -4625,10 +4625,11 @@ def test_basic(self):
46254625
.all(),
46264626
)
46274627

4628+
@testing.emits_warning("Column-expression-level unary distinct")
46284629
def test_basic_standalone(self):
46294630
User = self.classes.User
46304631

4631-
# issue 6008. the UnaryExpression now places itself into the
4632+
# issue #6008. the UnaryExpression now places itself into the
46324633
# result map so that it can be matched positionally without the need
46334634
# for any label.
46344635
q = fixture_session().query(distinct(User.id)).order_by(User.id)
@@ -4637,10 +4638,12 @@ def test_basic_standalone(self):
46374638
)
46384639
eq_([(7,), (8,), (9,), (10,)], q.all())
46394640

4641+
@testing.emits_warning("Column-expression-level unary distinct")
46404642
def test_standalone_w_subquery(self):
4643+
# additional test for #6008
46414644
User = self.classes.User
4642-
q = fixture_session().query(distinct(User.id))
46434645

4646+
q = fixture_session().query(distinct(User.id))
46444647
subq = q.subquery()
46454648
q = fixture_session().query(subq).order_by(subq.c[0])
46464649
eq_([(7,), (8,), (9,), (10,)], q.all())

‎test/sql/test_compiler.py‎

Lines changed: 34 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -1940,24 +1940,21 @@ def test_true_short_circuit(self):
19401940
dialect=default.DefaultDialect(supports_native_boolean=True),
19411941
)
19421942

1943-
def test_distinct(self):
1943+
def test_distinct_select_modifier(self):
19441944
self.assert_compile(
1945-
select(table1.c.myid.distinct()),
1946-
"SELECT DISTINCT mytable.myid FROM mytable",
1947-
)
1948-
1949-
self.assert_compile(
1950-
select(distinct(table1.c.myid)),
1945+
select(table1.c.myid).distinct(),
19511946
"SELECT DISTINCT mytable.myid FROM mytable",
19521947
)
19531948

19541949
self.assert_compile(
1955-
select(distinct(table1.c.myid)).set_label_style(
1956-
LABEL_STYLE_TABLENAME_PLUS_COL
1957-
),
1958-
"SELECT DISTINCT mytable.myid FROM mytable",
1950+
select(table1.c.myid)
1951+
.distinct()
1952+
.set_label_style(LABEL_STYLE_TABLENAME_PLUS_COL),
1953+
"SELECT DISTINCT mytable.myid AS mytable_myid FROM mytable",
19591954
)
19601955

1956+
@testing.emits_warning("Column-expression-level unary distinct")
1957+
def test_distinct_function_6008(self):
19611958
# the bug fixed here as part of #6008 is the same bug that's
19621959
# in 1.3 as well, producing
19631960
# "SELECT anon_2.anon_1 FROM (SELECT distinct mytable.myid
@@ -1969,8 +1966,15 @@ def test_distinct(self):
19691966
)
19701967

19711968
self.assert_compile(
1972-
select(table1.c.myid).distinct(),
1973-
"SELECT DISTINCT mytable.myid FROM mytable",
1969+
select(select(table1.c.myid.distinct()).subquery()),
1970+
"SELECT anon_2.anon_1 FROM (SELECT "
1971+
"DISTINCT mytable.myid AS anon_1 FROM mytable) AS anon_2",
1972+
)
1973+
1974+
def test_distinct_function(self):
1975+
self.assert_compile(
1976+
select(func.sum(distinct(table1.c.myid))),
1977+
"SELECT sum(DISTINCT mytable.myid) AS sum_1 FROM mytable",
19741978
)
19751979

19761980
self.assert_compile(
@@ -1983,6 +1987,23 @@ def test_distinct(self):
19831987
"SELECT count(DISTINCT mytable.myid) AS count_1 FROM mytable",
19841988
)
19851989

1990+
def test_distinct_function_warn_outside_aggregate(self):
1991+
with testing.expect_warnings(
1992+
"Column-expression-level unary distinct.*SELECT DISTINCT"
1993+
):
1994+
self.assert_compile(
1995+
select(distinct(table1.c.myid)),
1996+
"SELECT DISTINCT mytable.myid FROM mytable",
1997+
)
1998+
1999+
with testing.expect_warnings(
2000+
"Column-expression-level unary distinct.*SELECT DISTINCT"
2001+
):
2002+
self.assert_compile(
2003+
select(table1.c.myid.distinct()),
2004+
"SELECT DISTINCT mytable.myid FROM mytable",
2005+
)
2006+
19862007
def test_distinct_on(self):
19872008
with testing.expect_deprecated(
19882009
"Passing expression to",

‎test/sql/test_operators.py‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -5314,6 +5314,7 @@ class BitOpTest(fixtures.TestBase, testing.AssertsCompiledSQL):
53145314
argnames="py_op, sql_op",
53155315
)
53165316
@testing.variation("named", ["column", "unnamed", "label"])
5317+
@testing.emits_warning("Column-expression-level unary distinct")
53175318
def test_wraps_named_column_heuristic(self, py_op, sql_op, named):
53185319
"""test for #12681"""
53195320

@@ -5325,7 +5326,6 @@ def test_wraps_named_column_heuristic(self, py_op, sql_op, named):
53255326
select(expr),
53265327
f"SELECT {sql_op}q",
53275328
)
5328-
53295329
elif named.unnamed:
53305330
expr = py_op(literal("x", Integer))
53315331
assert isinstance(expr, UnaryExpression)

‎test/sql/test_resultset.py‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -972,6 +972,7 @@ def test_column_accessor_labels_w_dots(self, connection):
972972
not_in("user_name", r._mapping)
973973
eq_(list(r._fields), ["users.user_id", "users.user_name"])
974974

975+
@testing.emits_warning("Column-expression-level unary distinct")
975976
def test_column_accessor_unary(self, connection):
976977
users = self.tables.users
977978

‎test/sql/test_types.py‎

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -3943,11 +3943,13 @@ def test_expression_typing(self):
39433943

39443944
def test_distinct(self, connection):
39453945
test_table = self.tables.test
3946+
s = select(test_table.c.avalue).distinct()
3947+
eq_(connection.execute(s).scalar(), 25)
39463948

3947-
s = select(distinct(test_table.c.avalue))
3949+
s = select(func.sum(test_table.c.avalue.distinct()))
39483950
eq_(connection.execute(s).scalar(), 25)
39493951

3950-
s = select(test_table.c.avalue.distinct())
3952+
s = select(func.sum(distinct(test_table.c.avalue)))
39513953
eq_(connection.execute(s).scalar(), 25)
39523954

39533955
assert distinct(test_table.c.data).type == test_table.c.data.type

0 commit comments

Comments
 (0)