Check for column object in eval_none, not propkey

Fixed bug involving JSON NULL evaluation logic added in 1.1 as part
of :ticket:`3514` where the logic would not accommodate ORM
mapped attributes named differently from the :class:`.Column`
that was mapped.

Change-Id: I1848afcfb63ad7f074f315d8d3097666069b42be
Fixes: #4031

diff --git a/doc/build/changelog/changelog_11.rst b/doc/build/changelog/changelog_11.rst
index 602be13d24eaffd5efa87645ad6d9f48b314fac5..4bf1dc969aab7b5b18eac4c4b98f7fc04170494c 100644 (file)
--- a/doc/build/changelog/changelog_11.rst
+++ b/doc/build/changelog/changelog_11.rst
@@ -31,6 +31,17 @@
         ORDER parameter understood by Oracle.  Pull request
         courtesy David Moore.
 
+    .. change:: 4031
+        :tags: bug, orm
+        :versions: 1.2.0b2
+        :tickets: 4031
+
+        Fixed bug involving JSON NULL evaluation logic added in 1.1 as part
+        of :ticket:`3514` where the logic would not accommodate ORM
+        mapped attributes named differently from the :class:`.Column`
+        that was mapped.
+
+
 .. changelog::
     :version: 1.1.11
     :released: Monday, June 19, 2017

diff --git a/lib/sqlalchemy/orm/mapper.py b/lib/sqlalchemy/orm/mapper.py
index 1042442c0412768ad45333054ee4576242a5f2fb..6998a4c57264fc0d520a2732ec5bea9af4fa3ee4 100644 (file)
--- a/lib/sqlalchemy/orm/mapper.py
+++ b/lib/sqlalchemy/orm/mapper.py
@@ -2020,7 +2020,7 @@ class Mapper(InspectionAttr):
             (
                 table,
                 frozenset(
-                    col.key for col in columns
+                    col for col in columns
                     if col.type.should_evaluate_none
                 )
             )

diff --git a/lib/sqlalchemy/orm/persistence.py b/lib/sqlalchemy/orm/persistence.py
index 924b9e1c94fd0ae1954e8abec921472481765b4c..faacd018e1c5852a812b806d33f0178865da205a 100644 (file)
--- a/lib/sqlalchemy/orm/persistence.py
+++ b/lib/sqlalchemy/orm/persistence.py
@@ -398,7 +398,7 @@ def _collect_insert_commands(
         for propkey in set(propkey_to_col).intersection(state_dict):
             value = state_dict[propkey]
             col = propkey_to_col[propkey]
-            if value is None and propkey not in eval_none and not render_nulls:
+            if value is None and col not in eval_none and not render_nulls:
                 continue
             elif not bulk and hasattr(value, '__clause_element__') or \
                     isinstance(value, sql.ClauseElement):

diff --git a/test/orm/test_unitofworkv2.py b/test/orm/test_unitofworkv2.py
index ecd3650003db23b0e2b82cb833df6bf396a54236..270c3708e5aad95edfe58af39844646a913f4ed1 100644 (file)
--- a/test/orm/test_unitofworkv2.py
+++ b/test/orm/test_unitofworkv2.py
@@ -2501,63 +2501,102 @@ class NullEvaluatingTest(fixtures.MappedTest, testing.AssertsExecutionResults):
                 String(50).evaluates_none(), default='default_val'),
         )
 
+        Table(
+            'test_w_renames', metadata,
+            Column('id', Integer, primary_key=True,
+                   test_needs_autoincrement=True),
+            Column('evals_null_no_default', EvalsNull()),
+            Column('evals_null_default', EvalsNull(), default='default_val'),
+            Column('no_eval_null_no_default', String(50)),
+            Column('no_eval_null_default', String(50), default='default_val'),
+            Column(
+                'builtin_evals_null_no_default', String(50).evaluates_none()),
+            Column(
+                'builtin_evals_null_default',
+                String(50).evaluates_none(), default='default_val'),
+        )
+
     @classmethod
     def setup_classes(cls):
         class Thing(cls.Basic):
             pass
 
+        class AltNameThing(cls.Basic):
+            pass
+
     @classmethod
     def setup_mappers(cls):
         Thing = cls.classes.Thing
+        AltNameThing = cls.classes.AltNameThing
 
         mapper(Thing, cls.tables.test)
 
+        mapper(AltNameThing, cls.tables.test_w_renames, column_prefix="_foo_")
+
     def _assert_col(self, name, value):
-        Thing = self.classes.Thing
+        Thing, AltNameThing = self.classes.Thing, self.classes.AltNameThing
         s = Session()
 
         col = getattr(Thing, name)
         obj = s.query(col).filter(col == value).one()
         eq_(obj[0], value)
 
+        col = getattr(AltNameThing, "_foo_" + name)
+        obj = s.query(col).filter(col == value).one()
+        eq_(obj[0], value)
+
     def _test_insert(self, attr, expected):
-        Thing = self.classes.Thing
+        Thing, AltNameThing = self.classes.Thing, self.classes.AltNameThing
 
         s = Session()
         t1 = Thing(**{attr: None})
         s.add(t1)
+
+        t2 = AltNameThing(**{"_foo_" + attr: None})
+        s.add(t2)
+
         s.commit()
 
         self._assert_col(attr, expected)
 
     def _test_bulk_insert(self, attr, expected):
-        Thing = self.classes.Thing
+        Thing, AltNameThing = self.classes.Thing, self.classes.AltNameThing
 
         s = Session()
         s.bulk_insert_mappings(
             Thing, [{attr: None}]
         )
+        s.bulk_insert_mappings(
+            AltNameThing, [{"_foo_" + attr: None}]
+        )
         s.commit()
 
         self._assert_col(attr, expected)
 
     def _test_insert_novalue(self, attr, expected):
-        Thing = self.classes.Thing
+        Thing, AltNameThing = self.classes.Thing, self.classes.AltNameThing
 
         s = Session()
         t1 = Thing()
         s.add(t1)
+
+        t2 = AltNameThing()
+        s.add(t2)
+
         s.commit()
 
         self._assert_col(attr, expected)
 
     def _test_bulk_insert_novalue(self, attr, expected):
-        Thing = self.classes.Thing
+        Thing, AltNameThing = self.classes.Thing, self.classes.AltNameThing
 
         s = Session()
         s.bulk_insert_mappings(
             Thing, [{}]
         )
+        s.bulk_insert_mappings(
+            AltNameThing, [{}]
+        )
         s.commit()
 
         self._assert_col(attr, expected)