]> git.ipfire.org Git - thirdparty/sqlalchemy/sqlalchemy.git/commitdiff
dont use the label convention for memoized entities
authorMike Bayer <mike_mp@zzzcomputing.com>
Mon, 9 May 2022 14:27:51 +0000 (10:27 -0400)
committerMike Bayer <mike_mp@zzzcomputing.com>
Mon, 9 May 2022 14:35:03 +0000 (10:35 -0400)
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
(cherry picked from commit 319f09ffced3f655e7d500b3a9965e19468fd9d9)

doc/build/changelog/unreleased_14/8001.rst [new file with mode: 0644]
lib/sqlalchemy/orm/context.py
test/orm/test_query.py

diff --git a/doc/build/changelog/unreleased_14/8001.rst b/doc/build/changelog/unreleased_14/8001.rst
new file mode 100644 (file)
index 0000000..aa82514
--- /dev/null
@@ -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`.
index 49d354cb3d26a028a6ffad994bc8e907dd237fe7..2e3066db937f32054a1a1b12dbf8f59dd90d0590 100644 (file)
@@ -676,10 +676,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(
@@ -693,6 +689,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,
@@ -2444,11 +2448,15 @@ class _QueryEntity(object):
                             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
@@ -2458,10 +2466,15 @@ class _QueryEntity(object):
                             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)
@@ -2666,6 +2679,7 @@ class _BundleEntity(_QueryEntity):
         compile_state,
         expr,
         entities_collection,
+        is_current_entities,
         setup_entities=True,
         parent_bundle=None,
     ):
@@ -2696,6 +2710,7 @@ class _BundleEntity(_QueryEntity):
                         compile_state,
                         expr,
                         entities_collection,
+                        is_current_entities,
                         parent_bundle=self,
                     )
                 elif isinstance(expr, Bundle):
@@ -2703,6 +2718,7 @@ class _BundleEntity(_QueryEntity):
                         compile_state,
                         expr,
                         entities_collection,
+                        is_current_entities,
                         parent_bundle=self,
                     )
                 else:
@@ -2711,6 +2727,7 @@ class _BundleEntity(_QueryEntity):
                         [expr],
                         entities_collection,
                         None,
+                        is_current_entities,
                         parent_bundle=self,
                     )
 
@@ -2784,6 +2801,7 @@ class _ColumnEntity(_QueryEntity):
         columns,
         entities_collection,
         raw_column_index,
+        is_current_entities,
         parent_bundle=None,
     ):
         for column in columns:
@@ -2803,6 +2821,7 @@ class _ColumnEntity(_QueryEntity):
                         entities_collection,
                         _entity,
                         raw_column_index,
+                        is_current_entities,
                         parent_bundle=parent_bundle,
                     )
                 else:
@@ -2812,6 +2831,7 @@ class _ColumnEntity(_QueryEntity):
                         entities_collection,
                         _entity,
                         raw_column_index,
+                        is_current_entities,
                         parent_bundle=parent_bundle,
                     )
             else:
@@ -2820,6 +2840,7 @@ class _ColumnEntity(_QueryEntity):
                     column,
                     entities_collection,
                     raw_column_index,
+                    is_current_entities,
                     parent_bundle=parent_bundle,
                 )
 
@@ -2910,12 +2931,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)
@@ -2974,6 +2997,7 @@ class _ORMColumnEntity(_ColumnEntity):
         entities_collection,
         parententity,
         raw_column_index,
+        is_current_entities,
         parent_bundle=None,
     ):
         annotations = column._annotations
@@ -3000,9 +3024,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
index 6d9aee584af41d0214b1587de4203e65379ab1ae..0539e6fe658b4377a50ecc94746e8f8a8f3c51b5 100644 (file)
@@ -44,7 +44,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
@@ -861,6 +860,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
 
@@ -1005,34 +1016,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(object):
             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