Warn instead of raise for unmapped column that matches on key

Modified the change made to the ORM update/delete evaluator in
:ticket:`3366` such that if an unmapped column expression is present
in the update or delete, if the evaluator can match its name to the
mapped columns of the target class, a warning is emitted, rather than
raising UnevaluatableError.  This is essentially the pre-1.2 behavior,
and is to allow migration for applications that are currently relying
upon this pattern.  However, if the given attribute name cannot be
matched to the columns of the mapper, the UnevaluatableError is
still raised, which is what was fixed in :ticket:`3366`.

Change-Id: I658ed0dbf485b7f8009774f9c12d9912447abd2a
Fixes: #4073

diff --git a/doc/build/changelog/unreleased_12/4073.rst b/doc/build/changelog/unreleased_12/4073.rst
new file mode 100644 (file)
index 0000000..8a083df
--- /dev/null
+++ b/doc/build/changelog/unreleased_12/4073.rst
@@ -0,0 +1,13 @@
+.. change::
+    :tags: bug, orm
+    :tickets: 4073
+
+    Modified the change made to the ORM update/delete evaluator in
+    :ticket:`3366` such that if an unmapped column expression is present
+    in the update or delete, if the evaluator can match its name to the
+    mapped columns of the target class, a warning is emitted, rather than
+    raising UnevaluatableError.  This is essentially the pre-1.2 behavior,
+    and is to allow migration for applications that are currently relying
+    upon this pattern.  However, if the given attribute name cannot be
+    matched to the columns of the mapper, the UnevaluatableError is
+    still raised, which is what was fixed in :ticket:`3366`.

diff --git a/lib/sqlalchemy/orm/evaluator.py b/lib/sqlalchemy/orm/evaluator.py
index aff6718d311a41809913e1e6aea84706331f802c..3f2a83a0295294280138934ecb1aa0b3f9347243 100644 (file)
--- a/lib/sqlalchemy/orm/evaluator.py
+++ b/lib/sqlalchemy/orm/evaluator.py
@@ -7,6 +7,8 @@
 
 import operator
 from ..sql import operators
+from .. import inspect
+from .. import util
 
 
 class UnevaluatableError(Exception):
@@ -59,9 +61,17 @@ class EvaluatorCompiler(object):
                 )
             key = parentmapper._columntoproperty[clause].key
         else:
-            raise UnevaluatableError(
-                "Cannot evaluate column: %s" % clause
-            )
+            key = clause.key
+            if self.target_cls and key in inspect(self.target_cls).column_attrs:
+                util.warn(
+                    "Evaluating non-mapped column expression '%r' onto "
+                    "ORM instances; this is a deprecated use case.  Please "
+                    "make use of the actual mapped columns in ORM-evaluated "
+                    "UPDATE / DELETE expressions." % clause)
+            else:
+                raise UnevaluatableError(
+                    "Cannot evaluate column: %s" % clause
+                )
 
         get_corresponding_attr = operator.attrgetter(key)
         return lambda obj: get_corresponding_attr(obj)

diff --git a/lib/sqlalchemy/orm/persistence.py b/lib/sqlalchemy/orm/persistence.py
index b8fd0c79f2810c14d378f8ab7063b6a6f3fa705a..dfa05a85e915276ec233abeafa65268edc1ddefd 100644 (file)
--- a/lib/sqlalchemy/orm/persistence.py
+++ b/lib/sqlalchemy/orm/persistence.py
@@ -1394,11 +1394,11 @@ class BulkEvaluate(BulkUD):
 
             self._additional_evaluators(evaluator_compiler)
 
-        except evaluator.UnevaluatableError:
+        except evaluator.UnevaluatableError as err:
             raise sa_exc.InvalidRequestError(
-                "Could not evaluate current criteria in Python. "
-                "Specify 'fetch' or False for the "
-                "synchronize_session parameter.")
+                'Could not evaluate current criteria in Python: "%s". '
+                'Specify \'fetch\' or False for the '
+                'synchronize_session parameter.' % err)
 
         # TODO: detect when the where clause is a trivial primary key match
         self.matched_objects = [

diff --git a/test/orm/test_evaluator.py b/test/orm/test_evaluator.py
index ffb8d8701b2e71dad232a241997be93397f25cd6..dc96dc15da2e2d8e4016304a433d304cbb3587ea 100644 (file)
--- a/test/orm/test_evaluator.py
+++ b/test/orm/test_evaluator.py
@@ -15,7 +15,7 @@ from sqlalchemy.orm import exc as orm_exc
 from sqlalchemy.testing import assert_raises_message
 from sqlalchemy.testing import is_
 from sqlalchemy import inspect
-
+from sqlalchemy.testing import expect_warnings
 
 compiler = evaluator.EvaluatorCompiler()
 
@@ -38,7 +38,8 @@ class EvaluateTest(fixtures.MappedTest):
     def define_tables(cls, metadata):
         Table('users', metadata,
               Column('id', Integer, primary_key=True),
-              Column('name', String(64)))
+              Column('name', String(64)),
+              Column('othername', String(64)))
 
     @classmethod
     def setup_classes(cls):
@@ -84,9 +85,24 @@ class EvaluateTest(fixtures.MappedTest):
         eval_eq(User.name == None,  # noqa
                 testcases=[(User(name='foo'), False), (User(name=None), True)])
 
-    def test_raise_on_unannotated_column(self):
+    def test_warn_on_unannotated_matched_column(self):
         User = self.classes.User
 
+        compiler = evaluator.EvaluatorCompiler(User)
+
+        with expect_warnings(
+            r"Evaluating non-mapped column expression 'Column\('othername'.* "
+                "onto ORM instances; this is a deprecated use case."):
+            meth = compiler.process(User.name == Column('othername', String))
+
+        u1 = User(id=5)
+        meth(u1)
+
+    def test_raise_on_unannotated_unmatched_column(self):
+        User = self.classes.User
+
+        compiler = evaluator.EvaluatorCompiler(User)
+
         assert_raises_message(
             evaluator.UnevaluatableError,
             "Cannot evaluate column: foo",