From: Mike Bayer Date: Tue, 25 Feb 2025 15:11:29 +0000 (-0500) Subject: add postgresql distinct_on (patch 4) X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=5f8ac7099641a6e78a1bafc00bb82e755c2003ff;p=thirdparty%2Fsqlalchemy%2Fsqlalchemy.git add postgresql distinct_on (patch 4) Added syntax extension :func:`_postgresql.distinct_on` to build ``DISTINCT ON`` clauses. The old api, that passed columns to :meth:`_sql.Select.distinct`, is now deprecated. Fixes: #12342 Change-Id: Ia6a7e647a11e57b6ac2f50848778c20dc55eaf54 --- diff --git a/doc/build/changelog/unreleased_21/12195.rst b/doc/build/changelog/unreleased_21/12195.rst index a36d1bc8a8..7ecee32222 100644 --- a/doc/build/changelog/unreleased_21/12195.rst +++ b/doc/build/changelog/unreleased_21/12195.rst @@ -16,5 +16,5 @@ .. seealso:: - :ref:`examples.syntax_extensions` + :ref:`examples_syntax_extensions` diff --git a/doc/build/changelog/unreleased_21/12342.rst b/doc/build/changelog/unreleased_21/12342.rst new file mode 100644 index 0000000000..b146e7129f --- /dev/null +++ b/doc/build/changelog/unreleased_21/12342.rst @@ -0,0 +1,7 @@ +.. change:: + :tags: feature, postgresql + :tickets: 12342 + + Added syntax extension :func:`_postgresql.distinct_on` to build ``DISTINCT + ON`` clauses. The old api, that passed columns to + :meth:`_sql.Select.distinct`, is now deprecated. diff --git a/doc/build/dialects/mysql.rst b/doc/build/dialects/mysql.rst index 657cd2a418..d00d30e9de 100644 --- a/doc/build/dialects/mysql.rst +++ b/doc/build/dialects/mysql.rst @@ -223,6 +223,8 @@ MySQL DML Constructs .. autoclass:: sqlalchemy.dialects.mysql.Insert :members: +.. autofunction:: sqlalchemy.dialects.mysql.limit + mysqlclient (fork of MySQL-Python) diff --git a/doc/build/dialects/postgresql.rst b/doc/build/dialects/postgresql.rst index cbd357db7a..009463e6ee 100644 --- a/doc/build/dialects/postgresql.rst +++ b/doc/build/dialects/postgresql.rst @@ -590,6 +590,8 @@ PostgreSQL SQL Elements and Functions .. autoclass:: ts_headline +.. autofunction:: distinct_on + PostgreSQL Constraint Types --------------------------- diff --git a/lib/sqlalchemy/dialects/mysql/__init__.py b/lib/sqlalchemy/dialects/mysql/__init__.py index d722c1d30c..743fa47ab9 100644 --- a/lib/sqlalchemy/dialects/mysql/__init__.py +++ b/lib/sqlalchemy/dialects/mysql/__init__.py @@ -102,4 +102,5 @@ __all__ = ( "insert", "Insert", "match", + "limit", ) diff --git a/lib/sqlalchemy/dialects/postgresql/__init__.py b/lib/sqlalchemy/dialects/postgresql/__init__.py index 88935e2024..e426df71be 100644 --- a/lib/sqlalchemy/dialects/postgresql/__init__.py +++ b/lib/sqlalchemy/dialects/postgresql/__init__.py @@ -37,6 +37,7 @@ from .dml import Insert from .dml import insert from .ext import aggregate_order_by from .ext import array_agg +from .ext import distinct_on from .ext import ExcludeConstraint from .ext import phraseto_tsquery from .ext import plainto_tsquery @@ -164,4 +165,5 @@ __all__ = ( "array_agg", "insert", "Insert", + "distinct_on", ) diff --git a/lib/sqlalchemy/dialects/postgresql/base.py b/lib/sqlalchemy/dialects/postgresql/base.py index ef7e67841a..684478bd7f 100644 --- a/lib/sqlalchemy/dialects/postgresql/base.py +++ b/lib/sqlalchemy/dialects/postgresql/base.py @@ -1980,6 +1980,21 @@ class PGCompiler(compiler.SQLCompiler): else: return "" + def visit_postgresql_distinct_on(self, element, **kw): + if self.stack[-1]["selectable"]._distinct_on: + raise exc.CompileError( + "Cannot mix ``select.ext(distinct_on(...))`` and " + "``select.distinct(...)``" + ) + + if element._distinct_on: + cols = ", ".join( + self.process(col, **kw) for col in element._distinct_on + ) + return f"ON ({cols})" + else: + return None + def for_update_clause(self, select, **kw): if select._for_update_arg.read: if select._for_update_arg.key_share: diff --git a/lib/sqlalchemy/dialects/postgresql/ext.py b/lib/sqlalchemy/dialects/postgresql/ext.py index 37dab86dd8..0f110b8e06 100644 --- a/lib/sqlalchemy/dialects/postgresql/ext.py +++ b/lib/sqlalchemy/dialects/postgresql/ext.py @@ -8,26 +8,30 @@ from __future__ import annotations from typing import Any +from typing import Sequence from typing import TYPE_CHECKING from typing import TypeVar from . import types from .array import ARRAY +from ... import exc from ...sql import coercions from ...sql import elements from ...sql import expression from ...sql import functions from ...sql import roles from ...sql import schema +from ...sql.base import SyntaxExtension from ...sql.schema import ColumnCollectionConstraint from ...sql.sqltypes import TEXT from ...sql.visitors import InternalTraversal -_T = TypeVar("_T", bound=Any) - if TYPE_CHECKING: + from ...sql._typing import _ColumnExpressionArgument from ...sql.visitors import _TraverseInternalsType +_T = TypeVar("_T", bound=Any) + class aggregate_order_by(expression.ColumnElement): """Represent a PostgreSQL aggregate order by expression. @@ -495,3 +499,63 @@ class ts_headline(_regconfig_fn): for c in args ] super().__init__(*(initial_arg + addtl_args), **kwargs) + + +def distinct_on(*expr: _ColumnExpressionArgument[Any]) -> DistinctOnClause: + """apply a DISTINCT_ON to a SELECT statement + + e.g.:: + + stmt = select(tbl).ext(distinct_on(t.c.some_col)) + + this supersedes the previous approach of using + ``select(tbl).distinct(t.c.some_col))`` to apply a similar construct. + + .. versionadded:: 2.1 + + """ + return DistinctOnClause(expr) + + +class DistinctOnClause(SyntaxExtension, expression.ClauseElement): + stringify_dialect = "postgresql" + __visit_name__ = "postgresql_distinct_on" + + _traverse_internals: _TraverseInternalsType = [ + ("_distinct_on", InternalTraversal.dp_clauseelement_tuple), + ] + + def __init__(self, distinct_on: Sequence[_ColumnExpressionArgument[Any]]): + self._distinct_on = tuple( + coercions.expect(roles.ByOfRole, e, apply_propagate_attrs=self) + for e in distinct_on + ) + + def apply_to_select(self, select_stmt: expression.Select[Any]) -> None: + if select_stmt._distinct_on: + raise exc.InvalidRequestError( + "Cannot mix ``select.ext(distinct_on(...))`` and " + "``select.distinct(...)``" + ) + # mark this select as a distinct + select_stmt.distinct.non_generative(select_stmt) + + select_stmt.apply_syntax_extension_point( + self._merge_other_distinct, "pre_columns" + ) + + def _merge_other_distinct( + self, existing: Sequence[elements.ClauseElement] + ) -> Sequence[elements.ClauseElement]: + res = [] + to_merge = () + for e in existing: + if isinstance(e, DistinctOnClause): + to_merge += e._distinct_on + else: + res.append(e) + if to_merge: + res.append(DistinctOnClause(to_merge + self._distinct_on)) + else: + res.append(self) + return res diff --git a/lib/sqlalchemy/orm/context.py b/lib/sqlalchemy/orm/context.py index bc25eff636..9d01886388 100644 --- a/lib/sqlalchemy/orm/context.py +++ b/lib/sqlalchemy/orm/context.py @@ -1750,9 +1750,10 @@ class _ORMSelectCompileState(_ORMCompileState, SelectState): statement._order_by_clauses += tuple(order_by) if distinct_on: - statement.distinct.non_generative(statement, *distinct_on) + statement._distinct = True + statement._distinct_on = distinct_on elif distinct: - statement.distinct.non_generative(statement) + statement._distinct = True if group_by: statement._group_by_clauses += tuple(group_by) diff --git a/lib/sqlalchemy/orm/query.py b/lib/sqlalchemy/orm/query.py index 39b25378d2..5619ab1ecd 100644 --- a/lib/sqlalchemy/orm/query.py +++ b/lib/sqlalchemy/orm/query.py @@ -91,6 +91,7 @@ from ..sql.selectable import HasSuffixes from ..sql.selectable import LABEL_STYLE_TABLENAME_PLUS_COL from ..sql.selectable import SelectLabelStyle from ..util import deprecated +from ..util import warn_deprecated from ..util.typing import Literal from ..util.typing import Self from ..util.typing import TupleAny @@ -2687,11 +2688,18 @@ class Query( the PostgreSQL dialect will render a ``DISTINCT ON ()`` construct. - .. deprecated:: 1.4 Using \*expr in other dialects is deprecated - and will raise :class:`_exc.CompileError` in a future version. + .. deprecated:: 2.1 Passing expressions to + :meth:`_orm.Query.distinct` is deprecated, use + :func:`_postgresql.distinct_on` instead. """ if expr: + warn_deprecated( + "Passing expression to ``distinct`` to generate a DISTINCT " + "ON clause is deprecated. Use instead the " + "``postgresql.distinct_on`` function as an extension.", + "2.1", + ) self._distinct = True self._distinct_on = self._distinct_on + tuple( coercions.expect(roles.ByOfRole, e) for e in expr @@ -2708,6 +2716,10 @@ class Query( :ref:`examples_syntax_extensions` + :func:`_mysql.limit` - DML LIMIT for MySQL + + :func:`_postgresql.distinct_on` - DISTINCT ON for PostgreSQL + .. versionadded:: 2.1 """ diff --git a/lib/sqlalchemy/sql/base.py b/lib/sqlalchemy/sql/base.py index 11496aea60..f867bfeb77 100644 --- a/lib/sqlalchemy/sql/base.py +++ b/lib/sqlalchemy/sql/base.py @@ -1030,6 +1030,10 @@ class HasSyntaxExtensions(Generic[_L]): :ref:`examples_syntax_extensions` + :func:`_mysql.limit` - DML LIMIT for MySQL + + :func:`_postgresql.distinct_on` - DISTINCT ON for PostgreSQL + .. versionadded:: 2.1 """ diff --git a/lib/sqlalchemy/sql/selectable.py b/lib/sqlalchemy/sql/selectable.py index 29cbd00072..c945c355c7 100644 --- a/lib/sqlalchemy/sql/selectable.py +++ b/lib/sqlalchemy/sql/selectable.py @@ -101,6 +101,7 @@ from .visitors import prefix_anon_map from .. import exc from .. import util from ..util import HasMemoized_ro_memoized_attribute +from ..util import warn_deprecated from ..util.typing import Literal from ..util.typing import Self from ..util.typing import TupleAny @@ -6273,28 +6274,49 @@ class Select( SELECT DISTINCT user.id, user.name FROM user - The method also accepts an ``*expr`` parameter which produces the - PostgreSQL dialect-specific ``DISTINCT ON`` expression. Using this - parameter on other backends which don't support this syntax will - raise an error. + The method also historically accepted an ``*expr`` parameter which + produced the PostgreSQL dialect-specific ``DISTINCT ON`` expression. + This is now replaced using the :func:`_postgresql.distinct_on` + extension:: + + from sqlalchemy import select + from sqlalchemy.dialects.postgresql import distinct_on + + stmt = select(users_table).ext(distinct_on(users_table.c.name)) + + Using this parameter on other backends which don't support this + syntax will raise an error. :param \*expr: optional column expressions. When present, the PostgreSQL dialect will render a ``DISTINCT ON ()`` construct. A deprecation warning and/or :class:`_exc.CompileError` will be raised on other backends. + .. deprecated:: 2.1 Passing expressions to + :meth:`_sql.Select.distinct` is deprecated, use + :func:`_postgresql.distinct_on` instead. + .. deprecated:: 1.4 Using \*expr in other dialects is deprecated and will raise :class:`_exc.CompileError` in a future version. + .. seealso:: + + :func:`_postgresql.distinct_on` + + :meth:`_sql.HasSyntaxExtensions.ext` """ + self._distinct = True if expr: - self._distinct = True + warn_deprecated( + "Passing expression to ``distinct`` to generate a " + "DISTINCT ON clause is deprecated. Use instead the " + "``postgresql.distinct_on`` function as an extension.", + "2.1", + ) self._distinct_on = self._distinct_on + tuple( coercions.expect(roles.ByOfRole, e, apply_propagate_attrs=self) for e in expr ) - else: - self._distinct = True return self @_generative diff --git a/lib/sqlalchemy/testing/fixtures/__init__.py b/lib/sqlalchemy/testing/fixtures/__init__.py index ae88818300..f5f58e9e3f 100644 --- a/lib/sqlalchemy/testing/fixtures/__init__.py +++ b/lib/sqlalchemy/testing/fixtures/__init__.py @@ -23,6 +23,7 @@ from .sql import CacheKeySuite as CacheKeySuite from .sql import ( ComputedReflectionFixtureTest as ComputedReflectionFixtureTest, ) +from .sql import DistinctOnFixture as DistinctOnFixture from .sql import insertmanyvalues_fixture as insertmanyvalues_fixture from .sql import NoCache as NoCache from .sql import RemovesEvents as RemovesEvents diff --git a/lib/sqlalchemy/testing/fixtures/sql.py b/lib/sqlalchemy/testing/fixtures/sql.py index d1f06683f1..dc7add481e 100644 --- a/lib/sqlalchemy/testing/fixtures/sql.py +++ b/lib/sqlalchemy/testing/fixtures/sql.py @@ -17,6 +17,7 @@ from .base import TestBase from .. import config from .. import mock from ..assertions import eq_ +from ..assertions import expect_deprecated from ..assertions import ne_ from ..util import adict from ..util import drop_all_tables_from_metadata @@ -533,3 +534,26 @@ def insertmanyvalues_fixture( return orig_conn(dialect, context) connection._exec_insertmany_context = _exec_insertmany_context + + +class DistinctOnFixture: + @config.fixture(params=["legacy", "new"]) + def distinct_on_fixture(self, request): + from sqlalchemy.dialects.postgresql import distinct_on + + def go(query, *expr): + if request.param == "legacy": + if expr: + with expect_deprecated( + "Passing expression to ``distinct`` to generate a " + "DISTINCT " + "ON clause is deprecated. Use instead the " + "``postgresql.distinct_on`` function as an extension." + ): + return query.distinct(*expr) + else: + return query.distinct() + elif request.param == "new": + return query.ext(distinct_on(*expr)) + + return go diff --git a/lib/sqlalchemy/testing/suite/test_select.py b/lib/sqlalchemy/testing/suite/test_select.py index e6c4aa24f6..79a371d88b 100644 --- a/lib/sqlalchemy/testing/suite/test_select.py +++ b/lib/sqlalchemy/testing/suite/test_select.py @@ -1837,7 +1837,10 @@ class DistinctOnTest(AssertsCompiledSQL, fixtures.TablesTest): @testing.fails_if(testing.requires.supports_distinct_on) def test_distinct_on(self): - stm = select("*").distinct(column("q")).select_from(table("foo")) + with testing.expect_deprecated( + "Passing expression to ``distinct`` to generate " + ): + stm = select("*").distinct(column("q")).select_from(table("foo")) with testing.expect_deprecated( "DISTINCT ON is currently supported only by the PostgreSQL " ): diff --git a/test/dialect/postgresql/test_compiler.py b/test/dialect/postgresql/test_compiler.py index 8e241b82e5..4d739cf171 100644 --- a/test/dialect/postgresql/test_compiler.py +++ b/test/dialect/postgresql/test_compiler.py @@ -1,4 +1,5 @@ import random +import re from sqlalchemy import and_ from sqlalchemy import BigInteger @@ -42,6 +43,7 @@ from sqlalchemy.dialects.postgresql import aggregate_order_by from sqlalchemy.dialects.postgresql import ARRAY as PG_ARRAY from sqlalchemy.dialects.postgresql import array from sqlalchemy.dialects.postgresql import array_agg as pg_array_agg +from sqlalchemy.dialects.postgresql import distinct_on from sqlalchemy.dialects.postgresql import DOMAIN from sqlalchemy.dialects.postgresql import ExcludeConstraint from sqlalchemy.dialects.postgresql import insert @@ -72,6 +74,7 @@ from sqlalchemy.testing.assertions import assert_raises_message from sqlalchemy.testing.assertions import AssertsCompiledSQL from sqlalchemy.testing.assertions import eq_ from sqlalchemy.testing.assertions import eq_ignore_whitespace +from sqlalchemy.testing.assertions import expect_deprecated from sqlalchemy.testing.assertions import expect_warnings from sqlalchemy.testing.assertions import is_ from sqlalchemy.types import TypeEngine @@ -3501,7 +3504,12 @@ class InsertOnConflictTest( ) -class DistinctOnTest(fixtures.MappedTest, AssertsCompiledSQL): +class DistinctOnTest( + fixtures.MappedTest, + AssertsCompiledSQL, + fixtures.CacheKeySuite, + fixtures.DistinctOnFixture, +): """Test 'DISTINCT' with SQL expression language and orm.Query with an emphasis on PG's 'DISTINCT ON' syntax. @@ -3518,80 +3526,81 @@ class DistinctOnTest(fixtures.MappedTest, AssertsCompiledSQL): Column("b", String), ) - def test_plain_generative(self): + def test_distinct_on_no_cols(self, distinct_on_fixture): self.assert_compile( - select(self.table).distinct(), + distinct_on_fixture(select(self.table)), "SELECT DISTINCT t.id, t.a, t.b FROM t", ) - def test_on_columns_generative(self): + def test_distinct_on_cols(self, distinct_on_fixture): self.assert_compile( - select(self.table).distinct(self.table.c.a), + distinct_on_fixture(select(self.table), self.table.c.a), "SELECT DISTINCT ON (t.a) t.id, t.a, t.b FROM t", ) - def test_on_columns_generative_multi_call(self): self.assert_compile( - select(self.table) - .distinct(self.table.c.a) - .distinct(self.table.c.b), + distinct_on_fixture( + self.table.select(), self.table.c.a, self.table.c.b + ), "SELECT DISTINCT ON (t.a, t.b) t.id, t.a, t.b FROM t", + checkparams={}, ) - def test_plain_inline(self): - self.assert_compile( - select(self.table).distinct(), - "SELECT DISTINCT t.id, t.a, t.b FROM t", - ) + def test_distinct_on_columns_generative_multi_call( + self, distinct_on_fixture + ): + stmt = select(self.table) + stmt = distinct_on_fixture(stmt, self.table.c.a) + stmt = distinct_on_fixture(stmt, self.table.c.b) - def test_on_columns_inline_list(self): self.assert_compile( - select(self.table) - .distinct(self.table.c.a, self.table.c.b) - .order_by(self.table.c.a, self.table.c.b), - "SELECT DISTINCT ON (t.a, t.b) t.id, " - "t.a, t.b FROM t ORDER BY t.a, t.b", + stmt, + "SELECT DISTINCT ON (t.a, t.b) t.id, t.a, t.b FROM t", ) - def test_on_columns_inline_scalar(self): - self.assert_compile( - select(self.table).distinct(self.table.c.a), - "SELECT DISTINCT ON (t.a) t.id, t.a, t.b FROM t", - ) + def test_distinct_on_dupe_columns_generative_multi_call( + self, distinct_on_fixture + ): + stmt = select(self.table) + stmt = distinct_on_fixture(stmt, self.table.c.a) + stmt = distinct_on_fixture(stmt, self.table.c.a) - def test_literal_binds(self): self.assert_compile( - select(self.table).distinct(self.table.c.a == 10), - "SELECT DISTINCT ON (t.a = 10) t.id, t.a, t.b FROM t", - literal_binds=True, + stmt, + "SELECT DISTINCT ON (t.a, t.a) t.id, t.a, t.b FROM t", ) - def test_query_plain(self): + def test_legacy_query_plain(self, distinct_on_fixture): sess = Session() self.assert_compile( - sess.query(self.table).distinct(), + distinct_on_fixture(sess.query(self.table)), "SELECT DISTINCT t.id AS t_id, t.a AS t_a, t.b AS t_b FROM t", ) - def test_query_on_columns(self): + def test_legacy_query_on_columns(self, distinct_on_fixture): sess = Session() self.assert_compile( - sess.query(self.table).distinct(self.table.c.a), + distinct_on_fixture(sess.query(self.table), self.table.c.a), "SELECT DISTINCT ON (t.a) t.id AS t_id, t.a AS t_a, " "t.b AS t_b FROM t", ) - def test_query_on_columns_multi_call(self): + def test_legacy_query_distinct_on_columns_multi_call( + self, distinct_on_fixture + ): sess = Session() self.assert_compile( - sess.query(self.table) - .distinct(self.table.c.a) - .distinct(self.table.c.b), + distinct_on_fixture( + distinct_on_fixture(sess.query(self.table), self.table.c.a), + self.table.c.b, + ), "SELECT DISTINCT ON (t.a, t.b) t.id AS t_id, t.a AS t_a, " "t.b AS t_b FROM t", ) - def test_query_on_columns_subquery(self): + def test_legacy_query_distinct_on_columns_subquery( + self, distinct_on_fixture + ): sess = Session() class Foo: @@ -3604,33 +3613,34 @@ class DistinctOnTest(fixtures.MappedTest, AssertsCompiledSQL): f1 = aliased(Foo, subq) self.assert_compile( - sess.query(f1).distinct(f1.a, f1.b), + distinct_on_fixture(sess.query(f1), f1.a, f1.b), "SELECT DISTINCT ON (anon_1.a, anon_1.b) anon_1.id " "AS anon_1_id, anon_1.a AS anon_1_a, anon_1.b " "AS anon_1_b FROM (SELECT t.id AS id, t.a AS a, " "t.b AS b FROM t) AS anon_1", ) - def test_query_distinct_on_aliased(self): + def test_legacy_query_distinct_on_aliased(self, distinct_on_fixture): class Foo: pass + clear_mappers() self.mapper_registry.map_imperatively(Foo, self.table) a1 = aliased(Foo) sess = Session() + + q = distinct_on_fixture(sess.query(a1), a1.a) self.assert_compile( - sess.query(a1).distinct(a1.a), + q, "SELECT DISTINCT ON (t_1.a) t_1.id AS t_1_id, " "t_1.a AS t_1_a, t_1.b AS t_1_b FROM t AS t_1", ) - def test_distinct_on_subquery_anon(self): + def test_distinct_on_subquery_anon(self, distinct_on_fixture): sq = select(self.table).alias() - q = ( - select(self.table.c.id, sq.c.id) - .distinct(sq.c.id) - .where(self.table.c.id == sq.c.id) - ) + q = distinct_on_fixture( + select(self.table.c.id, sq.c.id), sq.c.id + ).where(self.table.c.id == sq.c.id) self.assert_compile( q, @@ -3639,13 +3649,11 @@ class DistinctOnTest(fixtures.MappedTest, AssertsCompiledSQL): "AS b FROM t) AS anon_1 WHERE t.id = anon_1.id", ) - def test_distinct_on_subquery_named(self): + def test_distinct_on_subquery_named(self, distinct_on_fixture): sq = select(self.table).alias("sq") - q = ( - select(self.table.c.id, sq.c.id) - .distinct(sq.c.id) - .where(self.table.c.id == sq.c.id) - ) + q = distinct_on_fixture( + select(self.table.c.id, sq.c.id), sq.c.id + ).where(self.table.c.id == sq.c.id) self.assert_compile( q, "SELECT DISTINCT ON (sq.id) t.id, sq.id AS id_1 " @@ -3653,6 +3661,111 @@ class DistinctOnTest(fixtures.MappedTest, AssertsCompiledSQL): "t.b AS b FROM t) AS sq WHERE t.id = sq.id", ) + @fixtures.CacheKeySuite.run_suite_tests + def test_distinct_on_ext_cache_key(self): + def leg(): + with expect_deprecated("Passing expression"): + return self.table.select().distinct(self.table.c.a) + + return lambda: [ + self.table.select().ext(distinct_on(self.table.c.a)), + self.table.select().ext(distinct_on(self.table.c.b)), + self.table.select().ext( + distinct_on(self.table.c.a, self.table.c.b) + ), + self.table.select().ext( + distinct_on(self.table.c.b, self.table.c.a) + ), + self.table.select(), + self.table.select().distinct(), + leg(), + ] + + def test_distinct_on_cache_key_equal(self, distinct_on_fixture): + self._run_cache_key_equal_fixture( + lambda: [ + distinct_on_fixture(self.table.select(), self.table.c.a), + distinct_on_fixture(select(self.table), self.table.c.a), + ], + compare_values=True, + ) + self._run_cache_key_equal_fixture( + lambda: [ + distinct_on_fixture( + distinct_on_fixture(self.table.select(), self.table.c.a), + self.table.c.b, + ), + distinct_on_fixture( + select(self.table), self.table.c.a, self.table.c.b + ), + ], + compare_values=True, + ) + + def test_distinct_on_literal_binds(self, distinct_on_fixture): + self.assert_compile( + distinct_on_fixture(select(self.table), self.table.c.a == 10), + "SELECT DISTINCT ON (t.a = 10) t.id, t.a, t.b FROM t", + literal_binds=True, + ) + + def test_distinct_on_col_str(self, distinct_on_fixture): + stmt = distinct_on_fixture(select(self.table), "a") + self.assert_compile( + stmt, + "SELECT DISTINCT ON (t.a) t.id, t.a, t.b FROM t", + dialect="postgresql", + ) + + def test_distinct_on_label(self, distinct_on_fixture): + stmt = distinct_on_fixture(select(self.table.c.a.label("foo")), "foo") + self.assert_compile(stmt, "SELECT DISTINCT ON (foo) t.a AS foo FROM t") + + def test_unresolvable_distinct_label(self, distinct_on_fixture): + stmt = distinct_on_fixture( + select(self.table.c.a.label("foo")), "not a label" + ) + with expect_raises_message( + exc.CompileError, + "Can't resolve label reference for.* expression 'not a" + " label' should be explicitly", + ): + self.assert_compile(stmt, "ingored") + + def test_distinct_on_ext_with_legacy_distinct(self): + with ( + expect_raises_message( + exc.InvalidRequestError, + re.escape( + "Cannot mix ``select.ext(distinct_on(...))`` and " + "``select.distinct(...)``" + ), + ), + expect_deprecated("Passing expression"), + ): + s = ( + self.table.select() + .distinct(self.table.c.b) + .ext(distinct_on(self.table.c.a)) + ) + + # opposite order is not detected... + with expect_deprecated("Passing expression"): + s = ( + self.table.select() + .ext(distinct_on(self.table.c.a)) + .distinct(self.table.c.b) + ) + # but it raises while compiling + with expect_raises_message( + exc.CompileError, + re.escape( + "Cannot mix ``select.ext(distinct_on(...))`` and " + "``select.distinct(...)``" + ), + ): + self.assert_compile(s, "ignored") + class FullTextSearchTest(fixtures.TestBase, AssertsCompiledSQL): """Tests for full text searching""" diff --git a/test/orm/test_core_compilation.py b/test/orm/test_core_compilation.py index a961962d91..10b831f837 100644 --- a/test/orm/test_core_compilation.py +++ b/test/orm/test_core_compilation.py @@ -20,6 +20,7 @@ from sqlalchemy import true from sqlalchemy import union from sqlalchemy import update from sqlalchemy import util +from sqlalchemy.dialects.postgresql import distinct_on from sqlalchemy.orm import aliased from sqlalchemy.orm import column_property from sqlalchemy.orm import contains_eager @@ -45,6 +46,7 @@ from sqlalchemy.testing import fixtures from sqlalchemy.testing import is_ from sqlalchemy.testing import mock from sqlalchemy.testing import Variation +from sqlalchemy.testing.assertions import expect_deprecated from sqlalchemy.testing.fixtures import fixture_session from sqlalchemy.testing.util import resolve_lambda from sqlalchemy.util.langhelpers import hybridproperty @@ -365,7 +367,13 @@ class SelectableTest(QueryTest, AssertsCompiledSQL): class PropagateAttrsTest(QueryTest): + __backend__ = True + def propagate_cases(): + def distinct_deprecated(User, user_table): + with expect_deprecated("Passing expression to"): + return select(1).distinct(User.id).select_from(user_table) + return testing.combinations( (lambda: select(1), False), (lambda User: select(User.id), True), @@ -431,8 +439,13 @@ class PropagateAttrsTest(QueryTest): ), ( # changed as part of #9805 - lambda User, user_table: select(1) - .distinct(User.id) + distinct_deprecated, + True, + testing.requires.supports_distinct_on, + ), + ( + lambda user_table, User: select(1) + .ext(distinct_on(User.id)) .select_from(user_table), True, testing.requires.supports_distinct_on, diff --git a/test/orm/test_query.py b/test/orm/test_query.py index 88e76e7c38..3fd8f89131 100644 --- a/test/orm/test_query.py +++ b/test/orm/test_query.py @@ -4981,36 +4981,6 @@ class DistinctTest(QueryTest, AssertsCompiledSQL): "addresses_email_address FROM users, addresses) AS anon_1", ) - def test_columns_augmented_sql_union_two(self): - User, Address = self.classes.User, self.classes.Address - - sess = fixture_session() - - q = ( - sess.query( - User.id, - User.name.label("foo"), - Address.id, - ) - .distinct(Address.email_address) - .order_by(User.id, User.name) - ) - q2 = sess.query(User.id, User.name.label("foo"), Address.id) - - self.assert_compile( - q.union(q2), - "SELECT anon_1.users_id AS anon_1_users_id, " - "anon_1.foo AS anon_1_foo, anon_1.addresses_id AS " - "anon_1_addresses_id FROM " - "((SELECT DISTINCT ON (addresses.email_address) users.id " - "AS users_id, users.name AS foo, " - "addresses.id AS addresses_id FROM users, addresses " - "ORDER BY users.id, users.name) " - "UNION SELECT users.id AS users_id, users.name AS foo, " - "addresses.id AS addresses_id FROM users, addresses) AS anon_1", - dialect="postgresql", - ) - def test_columns_augmented_sql_two(self): User, Address = self.classes.User, self.classes.Address @@ -5046,14 +5016,112 @@ class DistinctTest(QueryTest, AssertsCompiledSQL): "addresses_1.id", ) - def test_columns_augmented_sql_three(self): + +class DistinctOnTest( + QueryTest, AssertsCompiledSQL, fixtures.DistinctOnFixture +): + """a test suite that is obstensibly specific to the PostgreSQL-only + DISTINCT ON clause, however is actually testing a few things: + + 1. the legacy query.distinct() feature's handling of this directly + 2. PostgreSQL's distinct_on() extension + 3. the ability for Query to use statement extensions in general + 4. ORM compilation of statement extensions, with or without adaptations + + items 3 and 4 are universal to all statement extensions, with the PG + distinct_on() extension serving as the test case. + + """ + + __dialect__ = "default" + + @testing.fixture + def distinct_on_transform(self, distinct_on_fixture): + + def go(expr): + def transform(query): + return distinct_on_fixture(query, expr) + + return transform + + return go + + def test_distinct_on_definitely_adapted(self, distinct_on_transform): + """there are few cases where a query-wide adapter is used on + per-column expressions in SQLAlchemy 2 and greater. however the + legacy query.union() case still relies on such an adapter, so make + use of this codepath to exercise column adaptation for edge features + such as "distinct_on" + + """ + User, Address = self.classes.User, self.classes.Address + + sess = fixture_session() + + q = sess.query( + User.id, + User.name.label("foo"), + Address.email_address, + ).order_by(User.id, User.name) + q2 = sess.query(User.id, User.name.label("foo"), Address.email_address) + + q3 = q.union(q2).with_transformation( + distinct_on_transform(Address.email_address) + ) + + self.assert_compile( + q3, + "SELECT DISTINCT ON (anon_1.addresses_email_address) " + "anon_1.users_id AS anon_1_users_id, anon_1.foo AS anon_1_foo, " + "anon_1.addresses_email_address AS anon_1_addresses_email_address " + "FROM ((SELECT users.id AS users_id, users.name AS foo, " + "addresses.email_address AS addresses_email_address FROM users, " + "addresses ORDER BY users.id, users.name) " + "UNION SELECT users.id AS users_id, users.name AS foo, " + "addresses.email_address AS addresses_email_address " + "FROM users, addresses) AS anon_1", + dialect="postgresql", + ) + + def test_columns_augmented_sql_union_two(self, distinct_on_transform): + User, Address = self.classes.User, self.classes.Address + + sess = fixture_session() + + q = ( + sess.query( + User.id, + User.name.label("foo"), + Address.id, + ) + .with_transformation(distinct_on_transform(Address.email_address)) + .order_by(User.id, User.name) + ) + + q2 = sess.query(User.id, User.name.label("foo"), Address.id) + + self.assert_compile( + q.union(q2), + "SELECT anon_1.users_id AS anon_1_users_id, " + "anon_1.foo AS anon_1_foo, anon_1.addresses_id AS " + "anon_1_addresses_id FROM " + "((SELECT DISTINCT ON (addresses.email_address) users.id " + "AS users_id, users.name AS foo, " + "addresses.id AS addresses_id FROM users, addresses " + "ORDER BY users.id, users.name) " + "UNION SELECT users.id AS users_id, users.name AS foo, " + "addresses.id AS addresses_id FROM users, addresses) AS anon_1", + dialect="postgresql", + ) + + def test_columns_augmented_three(self, distinct_on_transform): User, Address = self.classes.User, self.classes.Address sess = fixture_session() q = ( sess.query(User.id, User.name.label("foo"), Address.id) - .distinct(User.name) + .with_transformation(distinct_on_transform(User.name)) .order_by(User.id, User.name, Address.email_address) ) @@ -5066,7 +5134,7 @@ class DistinctTest(QueryTest, AssertsCompiledSQL): dialect="postgresql", ) - def test_columns_augmented_distinct_on(self): + def test_columns_augmented_four(self, distinct_on_transform): User, Address = self.classes.User, self.classes.Address sess = fixture_session() @@ -5078,7 +5146,7 @@ class DistinctTest(QueryTest, AssertsCompiledSQL): Address.id, Address.email_address, ) - .distinct(Address.email_address) + .with_transformation(distinct_on_transform(Address.email_address)) .order_by(User.id, User.name, Address.email_address) .set_label_style(LABEL_STYLE_TABLENAME_PLUS_COL) .subquery() @@ -5105,16 +5173,17 @@ class DistinctTest(QueryTest, AssertsCompiledSQL): dialect="postgresql", ) - def test_columns_augmented_sql_three_using_label_reference(self): + def test_legacy_columns_augmented_sql_three_using_label_reference(self): User, Address = self.classes.User, self.classes.Address sess = fixture_session() - q = ( - sess.query(User.id, User.name.label("foo"), Address.id) - .distinct("name") - .order_by(User.id, User.name, Address.email_address) - ) + with expect_deprecated("Passing expression to"): + q = ( + sess.query(User.id, User.name.label("foo"), Address.id) + .distinct("name") + .order_by(User.id, User.name, Address.email_address) + ) # no columns are added when DISTINCT ON is used self.assert_compile( @@ -5125,14 +5194,15 @@ class DistinctTest(QueryTest, AssertsCompiledSQL): dialect="postgresql", ) - def test_columns_augmented_sql_illegal_label_reference(self): + def test_legacy_columns_augmented_sql_illegal_label_reference(self): User, Address = self.classes.User, self.classes.Address sess = fixture_session() - q = sess.query(User.id, User.name.label("foo"), Address.id).distinct( - "not a label" - ) + with expect_deprecated("Passing expression to"): + q = sess.query( + User.id, User.name.label("foo"), Address.id + ).distinct("not a label") from sqlalchemy.dialects import postgresql @@ -5146,7 +5216,7 @@ class DistinctTest(QueryTest, AssertsCompiledSQL): dialect=postgresql.dialect(), ) - def test_columns_augmented_sql_four(self): + def test_columns_augmented_sql_four(self, distinct_on_transform): User, Address = self.classes.User, self.classes.Address sess = fixture_session() @@ -5154,7 +5224,7 @@ class DistinctTest(QueryTest, AssertsCompiledSQL): q = ( sess.query(User) .join(User.addresses) - .distinct(Address.email_address) + .with_transformation(distinct_on_transform(Address.email_address)) .options(joinedload(User.addresses)) .order_by(desc(Address.email_address)) .limit(2) diff --git a/test/sql/test_compiler.py b/test/sql/test_compiler.py index 9e5d11bbfd..e0160396ff 100644 --- a/test/sql/test_compiler.py +++ b/test/sql/test_compiler.py @@ -1981,8 +1981,9 @@ class SelectTest(fixtures.TestBase, AssertsCompiledSQL): def test_distinct_on(self): with testing.expect_deprecated( + "Passing expression to", "DISTINCT ON is currently supported only by the PostgreSQL " - "dialect" + "dialect", ): select("*").distinct(table1.c.myid).compile() diff --git a/test/sql/test_text.py b/test/sql/test_text.py index 941a02d9e7..3cd13ab00f 100644 --- a/test/sql/test_text.py +++ b/test/sql/test_text.py @@ -840,7 +840,9 @@ class TextErrorsTest(fixtures.TestBase, AssertsCompiledSQL): self._test(select(table1.c.myid).select_from, "mytable", "mytable") -class OrderByLabelResolutionTest(fixtures.TestBase, AssertsCompiledSQL): +class OrderByLabelResolutionTest( + fixtures.TestBase, AssertsCompiledSQL, fixtures.DistinctOnFixture +): __dialect__ = "default" def _test_exception(self, stmt, offending_clause, dialect=None): @@ -851,7 +853,9 @@ class OrderByLabelResolutionTest(fixtures.TestBase, AssertsCompiledSQL): "Textual SQL " "expression %r should be explicitly " r"declared as text\(%r\)" % (offending_clause, offending_clause), - stmt.compile, + self.assert_compile, + stmt, + "not expected", dialect=dialect, ) @@ -934,27 +938,19 @@ class OrderByLabelResolutionTest(fixtures.TestBase, AssertsCompiledSQL): stmt = select(table1.c.myid).order_by("foobar") self._test_exception(stmt, "foobar") - def test_distinct_label(self): - stmt = select(table1.c.myid.label("foo")).distinct("foo") + def test_distinct_label(self, distinct_on_fixture): + stmt = distinct_on_fixture(select(table1.c.myid.label("foo")), "foo") self.assert_compile( stmt, "SELECT DISTINCT ON (foo) mytable.myid AS foo FROM mytable", dialect="postgresql", ) - def test_distinct_label_keyword(self): - stmt = select(table1.c.myid.label("foo")).distinct("foo") - self.assert_compile( - stmt, - "SELECT DISTINCT ON (foo) mytable.myid AS foo FROM mytable", - dialect="postgresql", + def test_unresolvable_distinct_label(self, distinct_on_fixture): + stmt = distinct_on_fixture( + select(table1.c.myid.label("foo")), "not a label" ) - - def test_unresolvable_distinct_label(self): - from sqlalchemy.dialects import postgresql - - stmt = select(table1.c.myid.label("foo")).distinct("not a label") - self._test_exception(stmt, "not a label", dialect=postgresql.dialect()) + self._test_exception(stmt, "not a label", dialect="postgresql") def test_group_by_label(self): stmt = select(table1.c.myid.label("foo")).group_by("foo") @@ -1043,8 +1039,8 @@ class OrderByLabelResolutionTest(fixtures.TestBase, AssertsCompiledSQL): "mytable.description FROM mytable ORDER BY fb DESC", ) - def test_pg_distinct(self): - stmt = select(table1).distinct("name") + def test_pg_distinct(self, distinct_on_fixture): + stmt = distinct_on_fixture(select(table1), "name") self.assert_compile( stmt, "SELECT DISTINCT ON (mytable.name) mytable.myid, "