From a65d5c250e9fd7090311ef12f28d7d959c6c738e Mon Sep 17 00:00:00 2001 From: Mike Bayer Date: Sun, 29 Mar 2020 14:24:39 -0400 Subject: [PATCH] Add a third labeling mode for SELECT statements Enhanced the disambiguating labels feature of the :func:`~.sql.expression.select` construct such that when a select statement is used in a subquery, repeated column names from different tables are now automatically labeled with a unique label name, without the need to use the full "apply_labels()" feature that conbines tablename plus column name. The disambigated labels are available as plain string keys in the .c collection of the subquery, and most importantly the feature allows an ORM :func:`.orm.aliased` construct against the combination of an entity and an arbitrary subquery to work correctly, targeting the correct columns despite same-named columns in the source tables, without the need for an "apply labels" warning. The existing labeling style is now called LABEL_STYLE_TABLENAME_PLUS_COL. This labeling style will remain used throughout the ORM as has been the case for over a decade, however, the new disambiguation scheme could theoretically replace this scheme entirely. The new scheme would dramatically alter how SQL looks when rendered from the ORM to be more succinct but arguably harder to read. The tablename_columnname scheme used by Join.c is unaffected here, as that's still hardcoded to that scheme. Fixes: #5221 Change-Id: Ib47d9e0f35046b3afc77bef6e65709b93d0c3026 --- doc/build/changelog/migration_20.rst | 99 +++++++++++ doc/build/changelog/unreleased_14/5221.rst | 22 +++ lib/sqlalchemy/ext/baked.py | 3 +- lib/sqlalchemy/orm/query.py | 11 +- lib/sqlalchemy/sql/base.py | 21 +++ lib/sqlalchemy/sql/compiler.py | 22 +-- lib/sqlalchemy/sql/elements.py | 5 + lib/sqlalchemy/sql/selectable.py | 190 ++++++++++++++++----- lib/sqlalchemy/testing/assertions.py | 3 +- lib/sqlalchemy/util/deprecations.py | 1 + test/orm/test_query.py | 9 +- test/sql/test_compare.py | 1 + test/sql/test_selectable.py | 79 +++++++-- 13 files changed, 384 insertions(+), 82 deletions(-) create mode 100644 doc/build/changelog/unreleased_14/5221.rst diff --git a/doc/build/changelog/migration_20.rst b/doc/build/changelog/migration_20.rst index e61c63bf99..854c94e4a3 100644 --- a/doc/build/changelog/migration_20.rst +++ b/doc/build/changelog/migration_20.rst @@ -910,6 +910,105 @@ and the limiting of the entities/columns to ``User`` is done on the result:: result = session.execute(stmt).scalars(User).all() +.. _migration_20_query_from_self: + +Selecting from the query itself as a subquery, e.g. "from_self()" +------------------------------------------------------------------- + +The :meth:`.Query.from_self` method is a very complicated method that is rarely +used. The purpose of this method is to convert a :class:`.Query` into a +subquery, then return a new :class:`.Query` which SELECTs from that subquery. +The elaborate aspect of this method is that the returned query applies +automatic translation of ORM entities and columns to be stated in the SELECT in +terms of the subquery, as well as that it allows the entities and columns to be +SELECTed from to be modified. + +Because :meth:`.Query.from_self` packs an intense amount of implicit +translation into the SQL it produces, while it does allow a certain kind of +pattern to be executed very succinctly, real world use of this method is +infrequent as it is not simple to understand. + +In SQLAlchemy 2.0, as the :func:`.future.select` construct will be expected +to handle every pattern the ORM :class:`.Query` does now, the pattern of +:meth:`.Query.from_self` can be invoked now by making use of the +:func:`.orm.aliased` function in conjunction with a subquery, that is +the :meth:`.Query.subquery` or :meth:`.Select.subquery` method. Version 1.4 +of SQLAlchemy has enhanced the ability of the :func:`.orm.aliased` construct +to correctly extract columns from a given subquery. + +Starting with a :meth:`.Query.from_self` query that selects from two different +entities, then converts itself to select just one of the entities from +a subquery:: + + q = session.query(User, Address.email_address).\ + join(User.addresses).\ + from_self(User).order_by(Address.email_address) + +The above query SELECTS from "user" and "address", then applies a subquery +to SELECT only the "users" row but still with ORDER BY the email address +column:: + + SELECT anon_1.user_id AS anon_1_user_id + FROM ( + SELECT "user".id AS user_id, address.email_address AS address_email_address + FROM "user" JOIN address ON "user".id = address.user_id + ) AS anon_1 ORDER BY anon_1.address_email_address + +The SQL query above illustrates the automatic translation of the "user" and +"address" tables in terms of the anonymously named subquery. + +In 2.0, we perform these steps explicitly using :func:`.orm.aliased`:: + + from sqlalchemy.orm import aliased + + subq = session.query(User, Address.email_address).\ + join(User.addresses).subquery() + + # state the User and Address entities both in terms of the subquery + ua = aliased(User, subq) + aa = aliased(Address, subq) + + # then select using those entities + q = session.query(ua).order_by(aa.email_address) + +The above query renders the identical SQL structure, but uses a more +succinct labeling scheme that doesn't pull in table names (that labeling +scheme is still available if the :meth:`.Select.apply_labels` method is used):: + + SELECT anon_1.id AS anon_1_id + FROM ( + SELECT "user".id AS id, address.email_address AS email_address + FROM "user" JOIN address ON "user".id = address.user_id + ) AS anon_1 ORDER BY anon_1.email_address + +SQLAlchemy 1.4 features improved disambiguation of columns in subqueries, +so even if our ``User`` and ``Address`` entities have overlapping column names, +we can select from both entities at once without having to specify any +particular labeling:: + + subq = session.query(User, Address).\ + join(User.addresses).subquery() + + ua = aliased(User, subq) + aa = aliased(Address, subq) + + q = session.query(ua, aa).order_by(aa.email_address) + +The above query will disambiguate the ``.id`` column of ``User`` and +``Address``, where ``Address.id`` is rendered and tracked as ``id_1``:: + + SELECT anon_1.id AS anon_1_id, anon_1.id_1 AS anon_1_id_1, + anon_1.user_id AS anon_1_user_id, + anon_1.email_address AS anon_1_email_address + FROM ( + SELECT "user".id AS id, address.id AS id_1, + address.user_id AS user_id, address.email_address AS email_address + FROM "user" JOIN address ON "user".id = address.user_id + ) AS anon_1 ORDER BY anon_1.email_address + +:ticket:`5221` + + Transparent Statement Compilation Caching replaces "Baked" queries, works in Core ================================================================================== diff --git a/doc/build/changelog/unreleased_14/5221.rst b/doc/build/changelog/unreleased_14/5221.rst new file mode 100644 index 0000000000..afb35f3fdb --- /dev/null +++ b/doc/build/changelog/unreleased_14/5221.rst @@ -0,0 +1,22 @@ +.. change:: + :tags: feature, sql + :tickets: 5221 + + Enhanced the disambiguating labels feature of the + :func:`~.sql.expression.select` construct such that when a select statement + is used in a subquery, repeated column names from different tables are now + automatically labeled with a unique label name, without the need to use the + full "apply_labels()" feature that conbines tablename plus column name. + The disambigated labels are available as plain string keys in the .c + collection of the subquery, and most importantly the feature allows an ORM + :func:`.orm.aliased` construct against the combination of an entity and an + arbitrary subquery to work correctly, targeting the correct columns despite + same-named columns in the source tables, without the need for an "apply + labels" warning. + + + .. seealso:: + + :ref:`migration_20_query_from_self` - Illustrates the new + disambiguation feature as part of a strategy to migrate away from the + :meth:`.Query.from_self` method. \ No newline at end of file diff --git a/lib/sqlalchemy/ext/baked.py b/lib/sqlalchemy/ext/baked.py index cf67387e43..ff741db32c 100644 --- a/lib/sqlalchemy/ext/baked.py +++ b/lib/sqlalchemy/ext/baked.py @@ -25,6 +25,7 @@ from ..orm.session import Session from ..sql import func from ..sql import literal_column from ..sql import util as sql_util +from ..sql.selectable import LABEL_STYLE_TABLENAME_PLUS_COL from ..util import collections_abc @@ -436,7 +437,7 @@ class Result(object): self.session, context, self._params, self._post_criteria ) - context.statement.use_labels = True + context.statement._label_style = LABEL_STYLE_TABLENAME_PLUS_COL if context.autoflush and not context.populate_existing: self.session._autoflush() q = context.query.params(self._params).with_session(self.session) diff --git a/lib/sqlalchemy/orm/query.py b/lib/sqlalchemy/orm/query.py index de11ba7dc8..4ba82d1e88 100644 --- a/lib/sqlalchemy/orm/query.py +++ b/lib/sqlalchemy/orm/query.py @@ -56,6 +56,7 @@ from ..sql.base import _generative from ..sql.base import ColumnCollection from ..sql.base import Generative from ..sql.selectable import ForUpdateArg +from ..sql.selectable import LABEL_STYLE_TABLENAME_PLUS_COL from ..util import collections_abc __all__ = ["Query", "QueryContext", "aliased"] @@ -1209,6 +1210,14 @@ class Query(Generative): self.session = session + @util.deprecated_20( + ":meth:`.Query.from_self`", + alternative="The new approach is to use the :func:`.orm.aliased` " + "construct in conjunction with a subquery. See the section " + ":ref:`Selecting from the query itself as a subquery " + "` in the 2.0 migration notes for an " + "example.", + ) def from_self(self, *entities): r"""return a Query that selects from this Query's SELECT statement. @@ -3313,7 +3322,7 @@ class Query(Generative): def __iter__(self): context = self._compile_context() - context.statement.use_labels = True + context.statement.label_style = LABEL_STYLE_TABLENAME_PLUS_COL if self._autoflush and not self._populate_existing: self.session._autoflush() return self._execute_and_instances(context) diff --git a/lib/sqlalchemy/sql/base.py b/lib/sqlalchemy/sql/base.py index f093cad909..f39f932e90 100644 --- a/lib/sqlalchemy/sql/base.py +++ b/lib/sqlalchemy/sql/base.py @@ -638,6 +638,27 @@ class Executable(Generative): return None +class prefix_anon_map(dict): + """A map that creates new keys for missing key access. + + Considers keys of the form " " to produce + new symbols "_", where "index" is an incrementing integer + corresponding to . + + Inlines the approach taken by :class:`sqlalchemy.util.PopulateDict` which + is otherwise usually used for this type of operation. + + """ + + def __missing__(self, key): + (ident, derived) = key.split(" ", 1) + anonymous_counter = self.get(derived, 1) + self[derived] = anonymous_counter + 1 + value = derived + "_" + str(anonymous_counter) + self[key] = value + return value + + class SchemaEventTarget(object): """Base class for elements that are the targets of :class:`.DDLEvents` events. diff --git a/lib/sqlalchemy/sql/compiler.py b/lib/sqlalchemy/sql/compiler.py index 000fc05fae..87ae5232e7 100644 --- a/lib/sqlalchemy/sql/compiler.py +++ b/lib/sqlalchemy/sql/compiler.py @@ -40,6 +40,7 @@ from . import schema from . import selectable from . import sqltypes from .base import NO_ARG +from .base import prefix_anon_map from .elements import quoted_name from .. import exc from .. import util @@ -541,27 +542,6 @@ class _CompileLabel(elements.ColumnElement): return self -class prefix_anon_map(dict): - """A map that creates new keys for missing key access. - - Considers keys of the form " " to produce - new symbols "_", where "index" is an incrementing integer - corresponding to . - - Inlines the approach taken by :class:`sqlalchemy.util.PopulateDict` which - is otherwise usually used for this type of operation. - - """ - - def __missing__(self, key): - (ident, derived) = key.split(" ", 1) - anonymous_counter = self.get(derived, 1) - self[derived] = anonymous_counter + 1 - value = derived + "_" + str(anonymous_counter) - self[key] = value - return value - - class SQLCompiler(Compiled): """Default implementation of :class:`.Compiled`. diff --git a/lib/sqlalchemy/sql/elements.py b/lib/sqlalchemy/sql/elements.py index f644b16d91..2b994c513d 100644 --- a/lib/sqlalchemy/sql/elements.py +++ b/lib/sqlalchemy/sql/elements.py @@ -939,6 +939,11 @@ class ColumnElement( """ return self._anon_label(getattr(self, "name", None)) + @util.memoized_property + def _dedupe_anon_label(self): + label = getattr(self, "name", None) or "anon" + return self._anon_label(label + "_") + @util.memoized_property def _label_anon_label(self): return self._anon_label(getattr(self, "_label", None)) diff --git a/lib/sqlalchemy/sql/selectable.py b/lib/sqlalchemy/sql/selectable.py index 5ffdf23d8f..4eab60801a 100644 --- a/lib/sqlalchemy/sql/selectable.py +++ b/lib/sqlalchemy/sql/selectable.py @@ -36,6 +36,7 @@ from .base import Generative from .base import HasCompileState from .base import HasMemoized from .base import Immutable +from .base import prefix_anon_map from .coercions import _document_text_coercion from .elements import _anonymous_label from .elements import and_ @@ -1770,7 +1771,9 @@ class Subquery(AliasedReturnsRows): "use the :meth:`.Query.scalar_subquery` method.", ) def as_scalar(self): - return self.element.scalar_subquery() + return self.element._set_label_style( + LABEL_STYLE_NONE + ).scalar_subquery() class FromGrouping(GroupedElement, FromClause): @@ -2281,6 +2284,9 @@ class SelectBase( :meth:`.SelectBase.scalar_subquery`. """ + if self._label_style is not LABEL_STYLE_NONE: + self = self._set_label_style(LABEL_STYLE_NONE) + return ScalarSelect(self) def label(self, name): @@ -2356,7 +2362,16 @@ class SelectBase( .. versionadded:: 1.4 """ - return Subquery._construct(self, name) + + return Subquery._construct(self._ensure_disambiguated_names(), name) + + def _ensure_disambiguated_names(self): + """Ensure that the names generated by this selectbase will be + disambiguated in some way, if possible. + + """ + + raise NotImplementedError() def alias(self, name=None, flat=False): """Return a named subquery against this :class:`.SelectBase`. @@ -2391,6 +2406,17 @@ class SelectStatementGrouping(GroupedElement, SelectBase): # type: (SelectBase) -> None self.element = coercions.expect(roles.SelectStatementRole, element) + def _ensure_disambiguated_names(self): + new_element = self.element._ensure_disambiguated_names() + if new_element is not self.element: + return SelectStatementGrouping(new_element) + else: + return self + + @property + def _label_style(self): + return self.element._label_style + @property def select_statement(self): return self.element @@ -2470,6 +2496,11 @@ class DeprecatedSelectBaseGenerations(object): self.group_by.non_generative(self, *clauses) +LABEL_STYLE_NONE = util.symbol("LABEL_STYLE_NONE") +LABEL_STYLE_TABLENAME_PLUS_COL = util.symbol("LABEL_STYLE_TABLENAME_PLUS_COL") +LABEL_STYLE_DISAMBIGUATE_ONLY = util.symbol("LABEL_STYLE_DISAMBIGUATE_ONLY") + + class GenerativeSelect(DeprecatedSelectBaseGenerations, SelectBase): """Base class for SELECT statements where additional elements can be added. @@ -2491,6 +2522,7 @@ class GenerativeSelect(DeprecatedSelectBaseGenerations, SelectBase): def __init__( self, + _label_style=LABEL_STYLE_NONE, use_labels=False, limit=None, offset=None, @@ -2498,7 +2530,10 @@ class GenerativeSelect(DeprecatedSelectBaseGenerations, SelectBase): group_by=None, bind=None, ): - self.use_labels = use_labels + if use_labels: + _label_style = LABEL_STYLE_TABLENAME_PLUS_COL + + self._label_style = _label_style if limit is not None: self.limit.non_generative(self, limit) @@ -2572,7 +2607,10 @@ class GenerativeSelect(DeprecatedSelectBaseGenerations, SelectBase): key_share=key_share, ) - @_generative + @property + def use_labels(self): + return self._label_style is LABEL_STYLE_TABLENAME_PLUS_COL + def apply_labels(self): """return a new selectable with the 'use_labels' flag set to True. @@ -2584,7 +2622,13 @@ class GenerativeSelect(DeprecatedSelectBaseGenerations, SelectBase): """ - self.use_labels = True + return self._set_label_style(LABEL_STYLE_TABLENAME_PLUS_COL) + + def _set_label_style(self, style): + if self._label_style is not style: + self = self._generate() + self._label_style = style + return self @property def _group_by_clause(self): @@ -2951,6 +2995,22 @@ class CompoundSelect(HasCompileState, GenerativeSelect): return True return False + def _set_label_style(self, style): + if self._label_style is not style: + self = self._generate() + select_0 = self.selects[0]._set_label_style(style) + self.selects = [select_0] + self.selects[1:] + + return self + + def _ensure_disambiguated_names(self): + new_select = self.selects[0]._ensure_disambiguated_names() + if new_select is not self.selects[0]: + self = self._generate() + self.selects = [new_select] + self.selects[1:] + + return self + def _generate_fromclause_column_proxies(self, subquery): # this is a slightly hacky thing - the union exports a @@ -2963,8 +3023,8 @@ class CompoundSelect(HasCompileState, GenerativeSelect): # ForeignKeys in. this would allow the union() to have all # those fks too. select_0 = self.selects[0] - if self.use_labels: - select_0 = select_0.apply_labels() + if self._label_style is not LABEL_STYLE_NONE: + select_0 = select_0._set_label_style(self._label_style) select_0._generate_fromclause_column_proxies(subquery) # hand-construct the "_proxies" collection to include all @@ -3299,6 +3359,7 @@ class Select( ("_hints", InternalTraversal.dp_table_hint_list), ("_distinct", InternalTraversal.dp_boolean), ("_distinct_on", InternalTraversal.dp_clauseelement_list), + ("_label_style", InternalTraversal.dp_plain_obj), ] + HasPrefixes._has_prefixes_traverse_internals + HasSuffixes._has_suffixes_traverse_internals @@ -4081,29 +4142,35 @@ class Select( """ names = set() + pa = None + collection = [] - cols = _select_iterables(self._raw_columns) - - def name_for_col(c): + for c in _select_iterables(self._raw_columns): # we use key_label since this name is intended for targeting # within the ColumnCollection only, it's not related to SQL # rendering which always uses column name for SQL label names - if self.use_labels: + if self._label_style is LABEL_STYLE_TABLENAME_PLUS_COL: name = c._key_label else: name = c._proxy_key if name in names: - if self.use_labels: - name = c._label_anon_label + if pa is None: + pa = prefix_anon_map() + + if self._label_style is LABEL_STYLE_TABLENAME_PLUS_COL: + name = c._label_anon_label % pa else: - name = c.anon_label + name = c.anon_label % pa else: names.add(name) - return name + collection.append((name, c)) - return ColumnCollection( - (name_for_col(c), c) for c in cols - ).as_immutable() + return ColumnCollection(collection).as_immutable() + + def _ensure_disambiguated_names(self): + if self._label_style is LABEL_STYLE_NONE: + self = self._set_label_style(LABEL_STYLE_DISAMBIGUATE_ONLY) + return self def _generate_columns_plus_names(self, anon_for_dupe_key): cols = _select_iterables(self._raw_columns) @@ -4117,30 +4184,54 @@ class Select( # anon label, apply _dedupe_label_anon_label to the subsequent # occurrences of it. - if self.use_labels: + if self._label_style is LABEL_STYLE_NONE: + # don't generate any labels + same_cols = set() + + return [ + (None, c, c in same_cols or same_cols.add(c)) for c in cols + ] + else: names = {} + use_tablename_labels = ( + self._label_style is LABEL_STYLE_TABLENAME_PLUS_COL + ) + def name_for_col(c): if not c._render_label_in_columns_clause: return (None, c, False) - elif c._label is None: + elif use_tablename_labels: + if c._label is None: + repeated = c.anon_label in names + names[c.anon_label] = c + return (None, c, repeated) + elif getattr(c, "name", None) is None: + # this is a scalar_select(). need to improve this case repeated = c.anon_label in names names[c.anon_label] = c return (None, c, repeated) - name = c._label + if use_tablename_labels: + name = effective_name = c._label + else: + name = None + effective_name = c.name repeated = False - if name in names: + if effective_name in names: # when looking to see if names[name] is the same column as # c, use hash(), so that an annotated version of the column # is seen as the same as the non-annotated - if hash(names[name]) != hash(c): + if hash(names[effective_name]) != hash(c): # different column under the same name. apply # disambiguating label - name = c._label_anon_label + if use_tablename_labels: + name = c._label_anon_label + else: + name = c.anon_label if anon_for_dupe_key and name in names: # here, c._label_anon_label is definitely unique to @@ -4154,27 +4245,26 @@ class Select( # already present. apply the "dedupe" label to # subsequent occurrences of the column so that the # original stays non-ambiguous - name = c._dedupe_label_anon_label + if use_tablename_labels: + name = c._dedupe_label_anon_label + else: + name = c._dedupe_anon_label repeated = True else: names[name] = c elif anon_for_dupe_key: # same column under the same name. apply the "dedupe" # label so that the original stays non-ambiguous - name = c._dedupe_label_anon_label + if use_tablename_labels: + name = c._dedupe_label_anon_label + else: + name = c._dedupe_anon_label repeated = True else: - names[name] = c + names[effective_name] = c return name, c, repeated return [name_for_col(c) for c in cols] - else: - # repeated name logic only for use labels at the moment - same_cols = set() - - return [ - (None, c, c in same_cols or same_cols.add(c)) for c in cols - ] def _generate_fromclause_column_proxies(self, subquery): """generate column proxies to place in the exported .c collection @@ -4183,17 +4273,33 @@ class Select( keys_seen = set() prox = [] + pa = None + + tablename_plus_col = ( + self._label_style is LABEL_STYLE_TABLENAME_PLUS_COL + ) + disambiguate_only = self._label_style is LABEL_STYLE_DISAMBIGUATE_ONLY + for name, c, repeated in self._generate_columns_plus_names(False): if not hasattr(c, "_make_proxy"): continue - if name is None: - key = None - elif self.use_labels: + elif tablename_plus_col: key = c._key_label if key is not None and key in keys_seen: - key = c._label_anon_label + if pa is None: + pa = prefix_anon_map() + key = c._label_anon_label % pa + keys_seen.add(key) + elif disambiguate_only: + key = c.key + if key is not None and key in keys_seen: + if pa is None: + pa = prefix_anon_map() + key = c.anon_label % pa keys_seen.add(key) else: + # one of the above label styles is set for subqueries + # as of #5221 so this codepath is likely not called now. key = None prox.append( c._make_proxy( @@ -4435,6 +4541,8 @@ class TextualSelect(SelectBase): __visit_name__ = "textual_select" + _label_style = LABEL_STYLE_NONE + _traverse_internals = [ ("element", InternalTraversal.dp_clauseelement), ("column_args", InternalTraversal.dp_clauseelement_list), @@ -4472,6 +4580,12 @@ class TextualSelect(SelectBase): (c.key, c) for c in self.column_args ).as_immutable() + def _set_label_style(self, style): + return self + + def _ensure_disambiguated_names(self): + return self + @property def _bind(self): return self.element._bind diff --git a/lib/sqlalchemy/testing/assertions.py b/lib/sqlalchemy/testing/assertions.py index 7dada1394b..5af29a7238 100644 --- a/lib/sqlalchemy/testing/assertions.py +++ b/lib/sqlalchemy/testing/assertions.py @@ -24,6 +24,7 @@ from .. import types as sqltypes from .. import util from ..engine import default from ..engine import url +from ..sql.selectable import LABEL_STYLE_TABLENAME_PLUS_COL from ..util import compat from ..util import decorator @@ -398,7 +399,7 @@ class AssertsCompiledSQL(object): if isinstance(clause, orm.Query): context = clause._compile_context() - context.statement.use_labels = True + context.statement._label_style = LABEL_STYLE_TABLENAME_PLUS_COL clause = context.statement elif isinstance(clause, orm.persistence.BulkUD): with mock.patch.object(clause, "_execute_stmt") as stmt_mock: diff --git a/lib/sqlalchemy/util/deprecations.py b/lib/sqlalchemy/util/deprecations.py index b78a71b1b4..e3d4b88c41 100644 --- a/lib/sqlalchemy/util/deprecations.py +++ b/lib/sqlalchemy/util/deprecations.py @@ -201,6 +201,7 @@ def _sanitize_restructured_text(text): name += "()" return name + text = re.sub(r":ref:`(.+) <.*>`", lambda m: '"%s"' % m.group(1), text) return re.sub(r"\:(\w+)\:`~?\.?(.+?)`", repl, text) diff --git a/test/orm/test_query.py b/test/orm/test_query.py index b0d1897e4c..a581481085 100644 --- a/test/orm/test_query.py +++ b/test/orm/test_query.py @@ -54,6 +54,7 @@ from sqlalchemy.orm.util import join from sqlalchemy.orm.util import with_parent from sqlalchemy.sql import expression from sqlalchemy.sql import operators +from sqlalchemy.sql.selectable import LABEL_STYLE_TABLENAME_PLUS_COL from sqlalchemy.testing import AssertsCompiledSQL from sqlalchemy.testing import fixtures from sqlalchemy.testing import is_ @@ -1265,7 +1266,7 @@ class OperatorTest(QueryTest, AssertsCompiledSQL): sess = Session() lead = sess.query(entity) context = lead._compile_context() - context.statement.use_labels = True + context.statement._label_style = LABEL_STYLE_TABLENAME_PLUS_COL lead = context.statement.compile(dialect=dialect) expected = (str(lead) + " WHERE " + expected).replace("\n", "") clause = sess.query(entity).filter(clause) @@ -1279,7 +1280,7 @@ class OperatorTest(QueryTest, AssertsCompiledSQL): lead = sess.query(from_).join(onclause, aliased=True) full = lead.filter(clause) context = lead._compile_context() - context.statement.use_labels = True + context.statement._label_style = LABEL_STYLE_TABLENAME_PLUS_COL lead = context.statement.compile(dialect=dialect) expected = (str(lead) + " WHERE " + expected).replace("\n", "") @@ -4024,9 +4025,7 @@ class DistinctTest(QueryTest, AssertsCompiledSQL): .distinct() ) - # TODO: this should warn for ambiguous labels when the flag - # is not present; is this flag also in core? See issue #5221 - subq = q.subquery(with_labels=True) + subq = q.subquery() # note this is a bit cutting edge; two differnet entities against # the same subquery. diff --git a/test/sql/test_compare.py b/test/sql/test_compare.py index b5fad54dc5..ab612053eb 100644 --- a/test/sql/test_compare.py +++ b/test/sql/test_compare.py @@ -300,6 +300,7 @@ class CoreFixtures(object): select([table_a.c.a]), select([table_a.c.a, table_a.c.b]), select([table_a.c.b, table_a.c.a]), + select([table_a.c.b, table_a.c.a]).apply_labels(), select([table_a.c.a]).where(table_a.c.b == 5), select([table_a.c.a]) .where(table_a.c.b == 5) diff --git a/test/sql/test_selectable.py b/test/sql/test_selectable.py index 30dfc0630d..58e7ee6a17 100644 --- a/test/sql/test_selectable.py +++ b/test/sql/test_selectable.py @@ -750,6 +750,54 @@ class SelectableTest( assert u1.corresponding_column(table2.c.col2) is u1.c._all_columns[1] assert u1.corresponding_column(table2.c.col3) is u1.c._all_columns[2] + def test_union_alias_dupe_keys_disambiguates_in_subq_compile_one(self): + s1 = select([table1.c.col1, table1.c.col2, table2.c.col1]).limit(1) + s2 = select([table2.c.col1, table2.c.col2, table2.c.col3]).limit(1) + u1 = union(s1, s2).subquery() + + eq_(u1.c.keys(), ["col1", "col2", "col1_1"]) + + stmt = select([u1]) + + eq_(stmt.selected_columns.keys(), ["col1", "col2", "col1_1"]) + + # the union() sets a new labeling form in the first SELECT + self.assert_compile( + stmt, + "SELECT anon_1.col1, anon_1.col2, anon_1.col1_1 FROM " + "((SELECT table1.col1, table1.col2, table2.col1 AS col1_1 " + "FROM table1, table2 LIMIT :param_1) UNION " + "(SELECT table2.col1, table2.col2, table2.col3 FROM table2 " + "LIMIT :param_2)) AS anon_1", + ) + + def test_union_alias_dupe_keys_disambiguates_in_subq_compile_two(self): + a = table("a", column("id")) + b = table("b", column("id"), column("aid")) + d = table("d", column("id"), column("aid")) + + u1 = union( + a.join(b, a.c.id == b.c.aid).select().apply_labels(), + a.join(d, a.c.id == d.c.aid).select().apply_labels(), + ).alias() + + eq_(u1.c.keys(), ["a_id", "b_id", "b_aid"]) + + stmt = select([u1]) + + eq_(stmt.selected_columns.keys(), ["a_id", "b_id", "b_aid"]) + + # the union() detects that the first SELECT already has a labeling + # style and uses that + self.assert_compile( + stmt, + "SELECT anon_1.a_id, anon_1.b_id, anon_1.b_aid FROM " + "(SELECT a.id AS a_id, b.id AS b_id, b.aid AS b_aid " + "FROM a JOIN b ON a.id = b.aid " + "UNION SELECT a.id AS a_id, d.id AS d_id, d.aid AS d_aid " + "FROM a JOIN d ON a.id = d.aid) AS anon_1", + ) + def test_union_alias_dupe_keys_grouped(self): s1 = select([table1.c.col1, table1.c.col2, table2.c.col1]).limit(1) s2 = select([table2.c.col1, table2.c.col2, table2.c.col3]).limit(1) @@ -974,10 +1022,15 @@ class SelectableTest( def test_self_referential_select_raises(self): t = table("t", column("x")) - s = select([t]) + # this issue is much less likely as subquery() applies a labeling + # style to the select, eliminating the self-referential call unless + # the select already had labeling applied + + s = select([t]).apply_labels() with testing.expect_deprecated("The SelectBase.c"): - s.where.non_generative(s, s.c.x > 5) + s.where.non_generative(s, s.c.t_x > 5) + assert_raises_message( exc.InvalidRequestError, r"select\(\) construct refers to itself as a FROM", @@ -2718,6 +2771,8 @@ class WithLabelsTest(fixtures.TestBase): sel = self._names_overlap() self._assert_result_keys(sel, ["x"]) + self._assert_subq_result_keys(sel, ["x", "x_1"]) + def test_names_overlap_label(self): sel = self._names_overlap().apply_labels() eq_(list(sel.selected_columns.keys()), ["t1_x", "t2_x"]) @@ -2732,6 +2787,7 @@ class WithLabelsTest(fixtures.TestBase): def test_names_overlap_keys_dont_nolabel(self): sel = self._names_overlap_keys_dont() + eq_(list(sel.selected_columns.keys()), ["a", "b"]) eq_(list(sel.subquery().c.keys()), ["a", "b"]) self._assert_result_keys(sel, ["x"]) @@ -2756,14 +2812,12 @@ class WithLabelsTest(fixtures.TestBase): def test_labels_overlap_label(self): sel = self._labels_overlap().apply_labels() - t2 = sel.froms[1] eq_( - list(sel.selected_columns.keys()), - ["t_x_id", t2.c.id._label_anon_label], + list(sel.selected_columns.keys()), ["t_x_id", "t_x_id_1"], ) eq_( list(sel.subquery().c.keys()), - ["t_x_id", t2.c.id._label_anon_label], + ["t_x_id", "t_x_id_1"], # ["t_x_id", "t_x_id"] # if we turn off deduping entirely, ) self._assert_result_keys(sel, ["t_x_id", "t_x_id_1"]) @@ -2801,14 +2855,11 @@ class WithLabelsTest(fixtures.TestBase): def test_keylabels_overlap_labels_dont_label(self): sel = self._keylabels_overlap_labels_dont().apply_labels() - t2 = sel.froms[1] eq_( - list(sel.selected_columns.keys()), - ["t_x_id", t2.c.id._label_anon_label], + list(sel.selected_columns.keys()), ["t_x_id", "t_x_b_1"], ) eq_( - list(sel.subquery().c.keys()), - ["t_x_id", t2.c.id._label_anon_label], + list(sel.subquery().c.keys()), ["t_x_id", "t_x_b_1"], ) self._assert_result_keys(sel, ["t_a", "t_x_b"]) self._assert_subq_result_keys(sel, ["t_a", "t_x_b"]) @@ -2828,14 +2879,12 @@ class WithLabelsTest(fixtures.TestBase): def test_keylabels_overlap_labels_overlap_label(self): sel = self._keylabels_overlap_labels_overlap().apply_labels() - t2 = sel.froms[1] eq_( - list(sel.selected_columns.keys()), - ["t_x_a", t2.c.a._label_anon_label], + list(sel.selected_columns.keys()), ["t_x_a", "t_x_id_1"], ) # deduping for different cols but same label - eq_(list(sel.subquery().c.keys()), ["t_x_a", t2.c.a._label_anon_label]) + eq_(list(sel.subquery().c.keys()), ["t_x_a", "t_x_id_1"]) # if we turn off deduping entirely # eq_(list(sel.subquery().c.keys()), ["t_x_a", "t_x_a"]) -- 2.47.3