--- /dev/null
+.. change::
+ :tags: bug, orm
+ :tickets: 6661
+
+ Refined the behavior of ORM subquery rendering with regards to deferred
+ columns and column properties to be more compatible with that of 1.3 while
+ also providing for 1.4's newer features. As a subquery in 1.4 does not make
+ use of loader options, including :func:`_orm.deferred`, a subquery that is
+ against an ORM entity with deferred attributes will now render those
+ deferred attributes that refer directly to mapped table columns, as these
+ are needed in the outer SELECT if that outer SELECT makes use of these
+ columns; however a deferred attribute that refers to a composed SQL
+ expression as we normally do with :func:`_orm.column_property` will not be
+ part of the subquery, as these can be selected explicitly if needed in the
+ subquery. If the entity is being SELECTed from this subquery, the column
+ expression can still render on "the outside" in terms of the derived
+ subquery columns. This produces essentially the same behavior as when
+ working with 1.3. However in this case the fix has to also make sure that
+ the ``.selected_columns`` collection of an ORM-enabled :func:`_sql.select`
+ also follows these rules, which in particular allows recursive CTEs to
+ render correctly in this scenario, which were previously failing to render
+ correctly due to this issue.
--- /dev/null
+.. change::
+ :tags: bug, sql
+ :tickets: 6663
+
+ Fixed issue in CTE constructs mostly relevant to ORM use cases where a
+ recursive CTE against "anonymous" labels such as those seen in ORM
+ ``column_property()`` mappings would render in the
+ ``WITH RECURSIVE xyz(...)`` section as their raw internal label and not a
+ cleanly anonymized name.
keymap_by_position = self._keymap_by_result_column_idx
- for idx, new in enumerate(
- invoked_statement._exported_columns_iterator()
- ):
+ for idx, new in enumerate(invoked_statement._all_selected_columns):
try:
rec = keymap_by_position[idx]
except KeyError:
("_only_load_props", InternalTraversal.dp_plain_obj),
("_set_base_alias", InternalTraversal.dp_boolean),
("_for_refresh_state", InternalTraversal.dp_boolean),
+ ("_render_for_subquery", InternalTraversal.dp_boolean),
]
# set to True by default from Query._statement_20(), to indicate
_only_load_props = None
_set_base_alias = False
_for_refresh_state = False
+ _render_for_subquery = False
current_path = _path_registry
self.select_statement = select_statement
# indicates this select() came from Query.statement
- self.for_statement = (
- for_statement
- ) = select_statement._compile_options._for_statement
+ self.for_statement = select_statement._compile_options._for_statement
# generally if we are from Query or directly from a select()
self.use_legacy_query_style = (
self.compile_options = select_statement._compile_options
- if not for_statement and not toplevel:
- # for subqueries, turn off eagerloads.
- # if "for_statement" mode is set, Query.subquery()
- # would have set this flag to False already if that's what's
- # desired
+ if not toplevel:
+ # for subqueries, turn off eagerloads and set
+ # "render_for_subquery".
self.compile_options += {
"_enable_eagerloads": False,
+ "_render_for_subquery": True,
}
# determine label style. we can make different decisions here.
else:
return target
- @classmethod
- def exported_columns_iterator(cls, statement):
- return (
- elem
- for elem in cls.all_selected_columns(statement)
- if not elem._is_text_clause
- )
-
@classmethod
def all_selected_columns(cls, statement):
for element in statement._raw_columns:
adapter.columns[prop.columns[0]] if adapter else prop.columns[0]
for prop in poly_properties
if isinstance(prop, properties.ColumnProperty)
+ and prop._renders_in_subqueries
]
def _columns_plus_keys(self, polymorphic_mappers=()):
"_mapped_by_synonym",
"_deferred_column_loader",
"_raise_column_loader",
+ "_renders_in_subqueries",
"raiseload",
)
if self.raiseload:
self.strategy_key += (("raiseload", True),)
+ def _memoized_attr__renders_in_subqueries(self):
+ return ("deferred", True) not in self.strategy_key or (
+ self not in self.parent._readonly_props
+ )
+
@util.preload_module("sqlalchemy.orm.state", "sqlalchemy.orm.strategies")
def _memoized_attr__deferred_column_loader(self):
state = util.preloaded.orm_state
if (
(
+ compile_state.compile_options._render_for_subquery
+ and self.parent_property._renders_in_subqueries
+ )
+ or (
loadopt
and "undefer_pks" in loadopt.local_opts
and set(self.columns).intersection(
elif isinstance(cte.element, selectable.CompoundSelect):
col_source = cte.element.selects[0]
else:
- assert False
+ assert False, "cte should only be against SelectBase"
recur_cols = [
c
for c in util.unique_list(
- col_source._exported_columns_iterator()
+ col_source._all_selected_columns
)
if c is not None
]
text += "(%s)" % (
", ".join(
- self.preparer.format_column(ident)
+ self.preparer.format_column(
+ ident, anon_map=self.anon_map
+ )
for ident in recur_cols
)
)
name=None,
table_name=None,
use_schema=False,
+ anon_map=None,
):
"""Prepare a quoted column name."""
if name is None:
name = column.name
+
+ if anon_map is not None and isinstance(
+ name, elements._truncated_label
+ ):
+ name = name.apply_map(anon_map)
+
if not getattr(column, "is_literal", False):
if use_table:
return (
from .elements import Null
from .selectable import HasCTE
from .selectable import HasPrefixes
+from .selectable import ReturnsRows
from .visitors import InternalTraversal
from .. import exc
from .. import util
HasCompileState,
DialectKWArgs,
HasPrefixes,
+ ReturnsRows,
Executable,
ClauseElement,
):
coercions.expect(roles.ColumnsClauseRole, c) for c in cols
)
- def _exported_columns_iterator(self):
- """Return the RETURNING columns as a sequence for this statement.
-
- .. versionadded:: 1.4
-
- """
-
+ @property
+ def _all_selected_columns(self):
return self._returning
@property
"""
# TODO: no coverage here
return ColumnCollection(
- (c.key, c) for c in self._exported_columns_iterator()
+ (c.key, c) for c in self._all_selected_columns
).as_immutable()
@_generative
@property
def selectable(self):
- raise NotImplementedError()
-
- def _exported_columns_iterator(self):
- """An iterator of column objects that represents the "exported"
- columns of this :class:`_expression.ReturnsRows`.
+ return self
- This is the same set of columns as are returned by
- :meth:`_expression.ReturnsRows.exported_columns`
- except they are returned
- as a simple iterator or sequence, rather than as a
- :class:`_expression.ColumnCollection` namespace.
+ @property
+ def _all_selected_columns(self):
+ """A sequence of column expression objects that represents the
+ "selected" columns of this :class:`_expression.ReturnsRows`.
- Subclasses should re-implement this method to bypass the interim
- creation of the :class:`_expression.ColumnCollection` if appropriate.
+ This is typically equivalent to .exported_columns except it is
+ delivered in the form of a straight sequence and not keyed
+ :class:`_expression.ColumnCollection`.
"""
- return iter(self.exported_columns)
+ raise NotImplementedError()
@property
def exported_columns(self):
is_selectable = True
- @property
- def selectable(self):
- return self
-
def _refresh_for_new_column(self, column):
raise NotImplementedError()
def _generate_proxy_for_new_column(self, column, subquery):
return self.element._generate_proxy_for_new_column(subquery)
- def _exported_columns_iterator(self):
- return self.element._exported_columns_iterator()
-
@property
def _all_selected_columns(self):
return self.element._all_selected_columns
for select in self.selects:
select._refresh_for_new_column(column)
- def _exported_columns_iterator(self):
- return self.selects[0]._exported_columns_iterator()
-
@property
def _all_selected_columns(self):
return self.selects[0]._all_selected_columns
def _memoized_attr__label_resolve_dict(self):
with_cols = dict(
(c._resolve_label or c._label or c.key, c)
- for c in self.statement._exported_columns_iterator()
+ for c in self.statement._all_selected_columns
if c._allow_label_resolve
)
only_froms = dict(
else:
return None
- @classmethod
- def exported_columns_iterator(cls, statement):
- return [
- c
- for c in _select_iterables(statement._raw_columns)
- if not c._is_text_clause
- ]
-
@classmethod
def all_selected_columns(cls, statement):
return [c for c in _select_iterables(statement._raw_columns)]
"""
- return self._exported_columns_iterator()
+ return iter(self._all_selected_columns)
def is_derived_from(self, fromclause):
if self in fromclause._cloned_set:
"""
return self.with_only_columns(
*util.preloaded.sql_util.reduce_columns(
- self._exported_columns_iterator(),
+ self._all_selected_columns,
only_synonyms=only_synonyms,
*(self._where_criteria + self._from_obj)
)
conv = SelectState._column_naming_convention(self._label_style)
return ColumnCollection(
- [(conv(c), c) for c in self._exported_columns_iterator()]
+ [
+ (conv(c), c)
+ for c in self._all_selected_columns
+ if not c._is_text_clause
+ ]
).as_immutable()
@HasMemoized.memoized_attribute
meth = SelectState.get_plugin_class(self).all_selected_columns
return list(meth(self))
- def _exported_columns_iterator(self):
- meth = SelectState.get_plugin_class(self).exported_columns_iterator
- return meth(self)
-
def _ensure_disambiguated_names(self):
if self._label_style is LABEL_STYLE_NONE:
self = self.set_label_style(LABEL_STYLE_DISAMBIGUATE_ONLY)
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"):
+ if c._is_text_clause:
continue
elif tablename_plus_col:
key = c._key_label
(c.key, c) for c in self.column_args
).as_immutable()
+ @property
+ def _all_selected_columns(self):
+ return self.column_args
+
def _set_label_style(self, style):
return self
from sqlalchemy.orm import aliased
from sqlalchemy.orm import column_property
from sqlalchemy.orm import contains_eager
+from sqlalchemy.orm import deferred
from sqlalchemy.orm import join as orm_join
from sqlalchemy.orm import joinedload
from sqlalchemy.orm import mapper
from sqlalchemy.orm import query_expression
from sqlalchemy.orm import relationship
+from sqlalchemy.orm import undefer
from sqlalchemy.orm import with_expression
from sqlalchemy.orm import with_polymorphic
from sqlalchemy.sql import and_
return User, Address
+ @testing.fixture
+ def deferred_fixture(self):
+ User = self.classes.User
+ users = self.tables.users
+
+ mapper(
+ User,
+ users,
+ properties={
+ "name": deferred(users.c.name),
+ "name_upper": column_property(
+ func.upper(users.c.name), deferred=True
+ ),
+ },
+ )
+
+ return User
+
+ @testing.fixture
+ def non_deferred_fixture(self):
+ User = self.classes.User
+ users = self.tables.users
+
+ mapper(
+ User,
+ users,
+ properties={
+ "name_upper": column_property(func.upper(users.c.name))
+ },
+ )
+
+ return User
+
def test_no_joinedload_in_subquery_select_rows(self, joinedload_fixture):
User, Address = joinedload_fixture
self.assert_compile(stmt2, expected)
- # TODO: need to test joinedload options, deferred mappings, deferred
- # options. these are all loader options that should *only* have an
- # effect on the outermost statement, never a subquery.
+ def test_deferred_subq_one(self, deferred_fixture):
+ """test for #6661"""
+ User = deferred_fixture
+
+ subq = select(User).subquery()
+
+ u1 = aliased(User, subq)
+ q = select(u1)
+
+ self.assert_compile(
+ q,
+ "SELECT anon_1.id "
+ "FROM (SELECT users.name AS name, "
+ "users.id AS id FROM users) AS anon_1",
+ )
+
+ # testing deferred opts separately for deterministic SQL generation
+
+ q = select(u1).options(undefer(u1.name))
+
+ self.assert_compile(
+ q,
+ "SELECT anon_1.name, anon_1.id "
+ "FROM (SELECT users.name AS name, "
+ "users.id AS id FROM users) AS anon_1",
+ )
+
+ q = select(u1).options(undefer(u1.name_upper))
+
+ self.assert_compile(
+ q,
+ "SELECT upper(anon_1.name) AS upper_1, anon_1.id "
+ "FROM (SELECT users.name AS name, "
+ "users.id AS id FROM users) AS anon_1",
+ )
+
+ def test_non_deferred_subq_one(self, non_deferred_fixture):
+ """test for #6661
+
+ cols that aren't deferred go into subqueries. 1.3 did this also.
+
+ """
+ User = non_deferred_fixture
+
+ subq = select(User).subquery()
+
+ u1 = aliased(User, subq)
+ q = select(u1)
+
+ self.assert_compile(
+ q,
+ "SELECT upper(anon_1.name) AS upper_1, anon_1.id, anon_1.name "
+ "FROM (SELECT upper(users.name) AS upper_2, users.id AS id, "
+ "users.name AS name FROM users) AS anon_1",
+ )
+
+ def test_deferred_subq_two(self, deferred_fixture):
+ """test for #6661
+
+ in this test, we are only confirming the current contract of ORM
+ subqueries which is that deferred + derived column_property's don't
+ export themselves into the .c. collection of a subquery.
+ We might want to revisit this in some way.
+
+ """
+ User = deferred_fixture
+
+ subq = select(User).subquery()
+
+ assert not hasattr(subq.c, "name_upper")
+
+ # "undefer" it by including it
+ subq = select(User, User.name_upper).subquery()
+
+ assert hasattr(subq.c, "name_upper")
+
+ def test_non_deferred_col_prop_targetable_in_subq(
+ self, non_deferred_fixture
+ ):
+ """test for #6661"""
+ User = non_deferred_fixture
+
+ subq = select(User).subquery()
+
+ assert hasattr(subq.c, "name_upper")
+
+ def test_recursive_cte_render_on_deferred(self, deferred_fixture):
+ """test for #6661.
+
+ this test is most directly the bug reported in #6661,
+ as the CTE uses stmt._exported_columns_iterator() ahead of compiling
+ the SELECT in order to get the list of columns that will be selected,
+ this has to match what the subquery is going to render.
+
+ This is also pretty fundamental to why deferred() as an option
+ can't be honored in a subquery; the subquery needs to export the
+ correct columns and it needs to do so without having to process
+ all the loader options. 1.3 OTOH when you got a subquery from
+ Query, it did a full compile_context. 1.4/2.0 we don't do that
+ anymore.
+
+ """
+
+ User = deferred_fixture
+
+ cte = select(User).cte(recursive=True)
+
+ # nonsensical, but we are just testing form
+ cte = cte.union_all(select(User).join(cte, cte.c.id == User.id))
+
+ stmt = select(User).join(cte, User.id == cte.c.id)
+
+ self.assert_compile(
+ stmt,
+ "WITH RECURSIVE anon_1(name, id) AS "
+ "(SELECT users.name AS name, users.id AS id FROM users "
+ "UNION ALL SELECT users.name AS name, users.id AS id "
+ "FROM users JOIN anon_1 ON anon_1.id = users.id) "
+ "SELECT users.id FROM users JOIN anon_1 ON users.id = anon_1.id",
+ )
+
+ # testing deferred opts separately for deterministic SQL generation
+ self.assert_compile(
+ stmt.options(undefer(User.name_upper)),
+ "WITH RECURSIVE anon_1(name, id) AS "
+ "(SELECT users.name AS name, users.id AS id FROM users "
+ "UNION ALL SELECT users.name AS name, users.id AS id "
+ "FROM users JOIN anon_1 ON anon_1.id = users.id) "
+ "SELECT upper(users.name) AS upper_1, users.id "
+ "FROM users JOIN anon_1 ON users.id = anon_1.id",
+ )
+
+ self.assert_compile(
+ stmt.options(undefer(User.name)),
+ "WITH RECURSIVE anon_1(name, id) AS "
+ "(SELECT users.name AS name, users.id AS id FROM users "
+ "UNION ALL SELECT users.name AS name, users.id AS id "
+ "FROM users JOIN anon_1 ON anon_1.id = users.id) "
+ "SELECT users.name, users.id "
+ "FROM users JOIN anon_1 ON users.id = anon_1.id",
+ )
class ExtraColsTest(QueryTest, AssertsCompiledSQL):
dialect=mssql.dialect(),
)
+ def test_recursive_w_anon_labels(self):
+ parts = table(
+ "parts", column("part"), column("sub_part"), column("quantity")
+ )
+
+ included_parts = (
+ select(
+ parts.c.sub_part.label(None),
+ parts.c.part.label(None),
+ parts.c.quantity,
+ )
+ .where(parts.c.part == "our part")
+ .cte(recursive=True)
+ )
+
+ incl_alias = included_parts.alias()
+ parts_alias = parts.alias()
+ included_parts = included_parts.union(
+ select(
+ parts_alias.c.sub_part,
+ parts_alias.c.part,
+ parts_alias.c.quantity,
+ ).where(parts_alias.c.part == incl_alias.c[0])
+ )
+
+ s = (
+ select(
+ included_parts.c[0],
+ func.sum(included_parts.c.quantity).label("total_quantity"),
+ )
+ .select_from(
+ included_parts.join(parts, included_parts.c[1] == parts.c.part)
+ )
+ .group_by(included_parts.c[0])
+ )
+ self.assert_compile(
+ s,
+ "WITH RECURSIVE anon_1(sub_part_1, part_1, quantity) "
+ "AS (SELECT parts.sub_part AS sub_part_1, parts.part "
+ "AS part_1, parts.quantity AS quantity FROM parts "
+ "WHERE parts.part = :part_2 UNION "
+ "SELECT parts_1.sub_part AS sub_part, "
+ "parts_1.part AS part, parts_1.quantity "
+ "AS quantity FROM parts AS parts_1, anon_1 AS anon_2 "
+ "WHERE parts_1.part = anon_2.sub_part_1) "
+ "SELECT anon_1.sub_part_1, "
+ "sum(anon_1.quantity) AS total_quantity FROM anon_1 "
+ "JOIN parts ON anon_1.part_1 = parts.part "
+ "GROUP BY anon_1.sub_part_1",
+ )
+
def test_recursive_inner_cte_unioned_to_alias(self):
parts = table(
"parts", column("part"), column("sub_part"), column("quantity")