Ensure attribute keys used for bulk update pk set

Fixed bug in :meth:`.Session.bulk_update_mappings` where an alternate-named
primary key attribute would not track properly into the UPDATE statement.

Change-Id: I33e9140f45827772768fa548adcfeb4dbfc2208d
Fixes: #3849
(cherry picked from commit 6a688b736429e27a892bc02111414491fe4103b0)

diff --git a/doc/build/changelog/changelog_10.rst b/doc/build/changelog/changelog_10.rst
index cfe8ed27b696f6ec0c228a86dd059ed0e2432f57..bf21ce2f7528d19ef6bd83d12ba8bb25b722db93 100644 (file)
--- a/doc/build/changelog/changelog_10.rst
+++ b/doc/build/changelog/changelog_10.rst
@@ -18,6 +18,14 @@
 .. changelog::
     :version: 1.0.16
 
+    .. change::
+        :tags: bug, orm
+        :tickets: 3849
+        :versions: 1.1.4
+
+        Fixed bug in :meth:`.Session.bulk_update_mappings` where an alternate-named
+        primary key attribute would not track properly into the UPDATE statement.
+
     .. change::
         :tags: bug, mssql
         :tickets: 3810

diff --git a/lib/sqlalchemy/orm/mapper.py b/lib/sqlalchemy/orm/mapper.py
index e5bd42014cc212ca533e98dda03ca5ff27f4856c..c1f8d28fdb4d2a1e42d7ecf50c991f989c39497a 100644 (file)
--- a/lib/sqlalchemy/orm/mapper.py
+++ b/lib/sqlalchemy/orm/mapper.py
@@ -1945,6 +1945,16 @@ class Mapper(InspectionAttr):
             for table, pks in self._pks_by_table.items()
         )
 
+    @_memoized_configured_property
+    def _pk_attr_keys_by_table(self):
+        return dict(
+            (
+                table,
+                frozenset([self._columntoproperty[col].key for col in pks])
+            )
+            for table, pks in self._pks_by_table.items()
+        )
+
     @_memoized_configured_property
     def _server_default_cols(self):
         return dict(

diff --git a/lib/sqlalchemy/orm/persistence.py b/lib/sqlalchemy/orm/persistence.py
index 5d69f51f3ff7336e02c2f736c5525d8ff69f9569..d2922dccb6cb65a54d6d849ba907d7ca29c4a98d 100644 (file)
--- a/lib/sqlalchemy/orm/persistence.py
+++ b/lib/sqlalchemy/orm/persistence.py
@@ -503,7 +503,7 @@ def _collect_update_commands(
                 (propkey_to_col[propkey]._label, state_dict.get(propkey))
                 for propkey in
                 set(propkey_to_col).
-                intersection(mapper._pk_keys_by_table[table])
+                intersection(mapper._pk_attr_keys_by_table[table])
             )
         else:
             pk_params = {}

diff --git a/test/orm/test_bulk.py b/test/orm/test_bulk.py
index 7e1b0523fad9871d8d0607830d07fe2891093f9c..4cc6905be1c1890f0550ba57398503f06d0f216f 100644 (file)
--- a/test/orm/test_bulk.py
+++ b/test/orm/test_bulk.py
@@ -208,6 +208,109 @@ class BulkUDPostfetchTest(BulkTest, fixtures.MappedTest):
         eq_(a1.y, 2)
 
 
+class BulkUDTestAltColKeys(BulkTest, fixtures.MappedTest):
+    @classmethod
+    def define_tables(cls, metadata):
+        Table(
+            'people_keys', metadata,
+            Column(
+                'person_id', Integer,
+                primary_key=True, key='id'),
+            Column('name', String(50), key='personname'))
+
+        Table(
+            'people_attrs', metadata,
+            Column(
+                'person_id', Integer,
+                primary_key=True),
+            Column('name', String(50)))
+
+        Table(
+            'people_both', metadata,
+            Column(
+                'person_id', Integer,
+                primary_key=True, key="id_key"),
+            Column('name', String(50), key='name_key'))
+
+    @classmethod
+    def setup_classes(cls):
+        class PersonKeys(cls.Comparable):
+            pass
+
+        class PersonAttrs(cls.Comparable):
+            pass
+
+        class PersonBoth(cls.Comparable):
+            pass
+
+    @classmethod
+    def setup_mappers(cls):
+        PersonKeys, PersonAttrs, PersonBoth = cls.classes(
+            "PersonKeys", "PersonAttrs", "PersonBoth")
+        people_keys, people_attrs, people_both = cls.tables(
+            "people_keys", "people_attrs", "people_both")
+
+        mapper(PersonKeys, people_keys)
+        mapper(PersonAttrs, people_attrs, properties={
+            'id': people_attrs.c.person_id,
+            'personname': people_attrs.c.name
+        })
+
+        mapper(PersonBoth, people_both, properties={
+            'id': people_both.c.id_key,
+            'personname': people_both.c.name_key
+        })
+
+    def test_insert_keys(self):
+        self._test_insert(self.classes.PersonKeys)
+
+    def test_insert_attrs(self):
+        self._test_insert(self.classes.PersonAttrs)
+
+    def test_insert_both(self):
+        self._test_insert(self.classes.PersonBoth)
+
+    def test_update_keys(self):
+        self._test_update(self.classes.PersonKeys)
+
+    def test_update_attrs(self):
+        self._test_update(self.classes.PersonAttrs)
+
+    def test_update_both(self):
+        # want to make sure that before [ticket:3849], this did not have
+        # a successful behavior or workaround
+        self._test_update(self.classes.PersonBoth)
+
+    def _test_insert(self, person_cls):
+        Person = person_cls
+
+        s = Session()
+        s.bulk_insert_mappings(
+            Person, [{"id": 5, "personname": "thename"}]
+        )
+
+        eq_(
+            s.query(Person).first(),
+            Person(id=5, personname="thename")
+        )
+
+    def _test_update(self, person_cls):
+        Person = person_cls
+
+        s = Session()
+        s.add(Person(id=5, personname="thename"))
+        s.commit()
+
+        s.bulk_update_mappings(
+            Person, [{"id": 5, "personname": "newname"}]
+        )
+
+        eq_(
+            s.query(Person).first(),
+            Person(id=5, personname="newname")
+        )
+
+
 class BulkInheritanceTest(BulkTest, fixtures.MappedTest):
     @classmethod
     def define_tables(cls, metadata):