]> 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:27:51 +0000 (10:27 -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

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 7a263daf8865990427cadb4e1f6d80ca985982c5..28fea2f9b3bb83907400cd6193601e14e7d01f12 100644 (file)
@@ -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
index 8374d05d4db0ae6ea1752a936f90f2dcad2c7346..b1e1bca7c34cafd71ac4e1ee561156879271988d 100644 (file)
@@ -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