diff options
-rw-r--r-- | doc/build/changelog/unreleased_12/4073.rst | 13 | ||||
-rw-r--r-- | lib/sqlalchemy/orm/evaluator.py | 16 | ||||
-rw-r--r-- | lib/sqlalchemy/orm/persistence.py | 8 | ||||
-rw-r--r-- | test/orm/test_evaluator.py | 22 |
4 files changed, 49 insertions, 10 deletions
diff --git a/doc/build/changelog/unreleased_12/4073.rst b/doc/build/changelog/unreleased_12/4073.rst new file mode 100644 index 000000000..8a083df9c --- /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 aff6718d3..3f2a83a02 100644 --- 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 b8fd0c79f..dfa05a85e 100644 --- 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 ffb8d8701..dc96dc15d 100644 --- 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", |