From 319f09ffced3f655e7d500b3a9965e19468fd9d9 Mon Sep 17 00:00:00 2001 From: Mike Bayer Date: Mon, 9 May 2022 10:27:51 -0400 Subject: [PATCH] dont use the label convention for memoized entities Fixed issue where ORM results would apply incorrect key names to the returned :class:`.Row` objects in the case where the set of columns to be selected were changed, such as when using :meth:`.Select.with_only_columns`. Fixes: #8001 Change-Id: If3a2a5d00d15ebc2e9d41494845cfb3b06f80dcc --- doc/build/changelog/unreleased_14/8001.rst | 8 ++++ lib/sqlalchemy/orm/context.py | 48 +++++++++++++++++----- test/orm/test_query.py | 43 ++++++++----------- 3 files changed, 63 insertions(+), 36 deletions(-) create mode 100644 doc/build/changelog/unreleased_14/8001.rst diff --git a/doc/build/changelog/unreleased_14/8001.rst b/doc/build/changelog/unreleased_14/8001.rst new file mode 100644 index 0000000000..aa8251445a --- /dev/null +++ b/doc/build/changelog/unreleased_14/8001.rst @@ -0,0 +1,8 @@ +.. change:: + :tags: bug, orm + :tickets: 8001 + + Fixed issue where ORM results would apply incorrect key names to the + returned :class:`.Row` objects in the case where the set of columns to be + selected were changed, such as when using + :meth:`.Select.with_only_columns`. diff --git a/lib/sqlalchemy/orm/context.py b/lib/sqlalchemy/orm/context.py index 7a263daf88..28fea2f9b3 100644 --- a/lib/sqlalchemy/orm/context.py +++ b/lib/sqlalchemy/orm/context.py @@ -767,10 +767,6 @@ class ORMSelectCompileState(ORMCompileState, SelectState): else: self.label_style = self.select_statement._label_style - self._label_convention = self._column_naming_convention( - statement._label_style, self.use_legacy_query_style - ) - if select_statement._memoized_select_entities: self._memoized_entities = { memoized_entities: _QueryEntity.to_compile_state( @@ -784,6 +780,14 @@ class ORMSelectCompileState(ORMCompileState, SelectState): ) } + # label_convention is stateful and will yield deduping keys if it + # sees the same key twice. therefore it's important that it is not + # invoked for the above "memoized" entities that aren't actually + # in the columns clause + self._label_convention = self._column_naming_convention( + statement._label_style, self.use_legacy_query_style + ) + _QueryEntity.to_compile_state( self, select_statement._raw_columns, @@ -2202,11 +2206,15 @@ class _QueryEntity: entity._select_iterable, entities_collection, idx, + is_current_entities, ) else: if entity._annotations.get("bundle", False): _BundleEntity( - compile_state, entity, entities_collection + compile_state, + entity, + entities_collection, + is_current_entities, ) elif entity._is_clause_list: # this is legacy only - test_composites.py @@ -2216,10 +2224,15 @@ class _QueryEntity: entity._select_iterable, entities_collection, idx, + is_current_entities, ) else: _ColumnEntity._for_columns( - compile_state, [entity], entities_collection, idx + compile_state, + [entity], + entities_collection, + idx, + is_current_entities, ) elif entity.is_bundle: _BundleEntity(compile_state, entity, entities_collection) @@ -2417,6 +2430,7 @@ class _BundleEntity(_QueryEntity): compile_state, expr, entities_collection, + is_current_entities, setup_entities=True, parent_bundle=None, ): @@ -2447,6 +2461,7 @@ class _BundleEntity(_QueryEntity): compile_state, expr, entities_collection, + is_current_entities, parent_bundle=self, ) elif isinstance(expr, Bundle): @@ -2454,6 +2469,7 @@ class _BundleEntity(_QueryEntity): compile_state, expr, entities_collection, + is_current_entities, parent_bundle=self, ) else: @@ -2462,6 +2478,7 @@ class _BundleEntity(_QueryEntity): [expr], entities_collection, None, + is_current_entities, parent_bundle=self, ) @@ -2527,6 +2544,7 @@ class _ColumnEntity(_QueryEntity): columns, entities_collection, raw_column_index, + is_current_entities, parent_bundle=None, ): for column in columns: @@ -2546,6 +2564,7 @@ class _ColumnEntity(_QueryEntity): entities_collection, _entity, raw_column_index, + is_current_entities, parent_bundle=parent_bundle, ) else: @@ -2555,6 +2574,7 @@ class _ColumnEntity(_QueryEntity): entities_collection, _entity, raw_column_index, + is_current_entities, parent_bundle=parent_bundle, ) else: @@ -2563,6 +2583,7 @@ class _ColumnEntity(_QueryEntity): column, entities_collection, raw_column_index, + is_current_entities, parent_bundle=parent_bundle, ) @@ -2653,12 +2674,14 @@ class _RawColumnEntity(_ColumnEntity): column, entities_collection, raw_column_index, + is_current_entities, parent_bundle=None, ): self.expr = column self.raw_column_index = raw_column_index self.translate_raw_column = raw_column_index is not None - if column._is_text_clause: + + if not is_current_entities or column._is_text_clause: self._label_name = None else: self._label_name = compile_state._label_convention(column) @@ -2717,6 +2740,7 @@ class _ORMColumnEntity(_ColumnEntity): entities_collection, parententity, raw_column_index, + is_current_entities, parent_bundle=None, ): annotations = column._annotations @@ -2743,9 +2767,13 @@ class _ORMColumnEntity(_ColumnEntity): self.translate_raw_column = raw_column_index is not None self.raw_column_index = raw_column_index - self._label_name = compile_state._label_convention( - column, col_name=orm_key - ) + + if is_current_entities: + self._label_name = compile_state._label_convention( + column, col_name=orm_key + ) + else: + self._label_name = None _entity._post_inspect self.entity_zero = self.entity_zero_or_selectable = ezero = _entity diff --git a/test/orm/test_query.py b/test/orm/test_query.py index 8374d05d4d..b1e1bca7c3 100644 --- a/test/orm/test_query.py +++ b/test/orm/test_query.py @@ -45,7 +45,6 @@ from sqlalchemy.orm import aliased from sqlalchemy.orm import attributes from sqlalchemy.orm import backref from sqlalchemy.orm import Bundle -from sqlalchemy.orm import clear_mappers from sqlalchemy.orm import column_property from sqlalchemy.orm import contains_eager from sqlalchemy.orm import defer @@ -877,6 +876,18 @@ class RowLabelingTest(QueryTest): assert_row_keys(stmt, expected, coreorm_exec) + def test_with_only_columns(self, assert_row_keys): + """test #8001""" + + User, Address = self.classes("User", "Address") + + stmt = select(User.id, Address.email_address).join_from(User, Address) + stmt = stmt.with_only_columns( + stmt.selected_columns.id, stmt.selected_columns.email_address + ) + + assert_row_keys(stmt, ["id", "email_address"], "orm") + def test_explicit_cols_legacy(self): User = self.classes.User @@ -1021,34 +1032,14 @@ class RowLabelingTest(QueryTest): eq_(row._mapping.keys(), ["id", "name", "id", "name"]) @testing.fixture - def uname_fixture(self): + def uname_fixture(self, registry): class Foo: pass - if False: - # this conditional creates the table each time which would - # eliminate cross-test memoization issues. if the tests - # are failing without this then there's a memoization issue. - # check AnnotatedColumn memoized keys - m = MetaData() - users = Table( - "users", - m, - Column("id", Integer, primary_key=True), - Column( - "name", - String, - ), - ) - self.mapper_registry.map_imperatively( - Foo, users, properties={"uname": users.c.name} - ) - else: - users = self.tables.users - clear_mappers() - self.mapper_registry.map_imperatively( - Foo, users, properties={"uname": users.c.name} - ) + users = self.tables.users + registry.map_imperatively( + Foo, users, properties={"uname": users.c.name} + ) return Foo -- 2.47.2