--- /dev/null
+.. change::
+ :tags: bug, orm
+ :tickets: 8656
+
+ Removed the warning that emits when using ORM-enabled update/delete
+ regarding evaluation of columns by name, first added in :ticket:`4073`;
+ this warning actually covers up a scenario that otherwise could populate
+ the wrong Python value for an ORM mapped attribute depending on what the
+ actual column is, so this deprecated case is removed. In 2.0, ORM enabled
+ update/delete uses "auto" for "synchronize_session", which should do the
+ right thing automatically for any given UPDATE expression.
from .base import PassiveFlag
from .. import exc
from .. import inspect
-from .. import util
from ..sql import and_
from ..sql import operators
from ..sql.sqltypes import Integer
return lambda obj: True
def visit_column(self, clause):
- if "parentmapper" in clause._annotations:
+ try:
parentmapper = clause._annotations["parentmapper"]
- if self.target_cls and not issubclass(
- self.target_cls, parentmapper.class_
- ):
- raise UnevaluatableError(
- "Can't evaluate criteria against "
- f"alternate class {parentmapper.class_}"
- )
-
- try:
- key = parentmapper._columntoproperty[clause].key
- except orm_exc.UnmappedColumnError as err:
- raise UnevaluatableError(
- f"Cannot evaluate expression: {err}"
- ) from err
-
- impl = parentmapper.class_manager[key].impl
-
- if impl is not None:
-
- def get_corresponding_attr(obj):
- if obj is None:
- return _NO_OBJECT
- state = inspect(obj)
- dict_ = state.dict
-
- value = impl.get(
- state, dict_, passive=PassiveFlag.PASSIVE_NO_FETCH
- )
- if value is LoaderCallableStatus.PASSIVE_NO_RESULT:
- return _EXPIRED_OBJECT
- return value
-
- return get_corresponding_attr
- else:
- key = clause.key
- if (
- self.target_cls
- and key in inspect(self.target_cls).column_attrs
- ):
- util.warn(
- f"Evaluating non-mapped column expression '{clause}' onto "
- "ORM instances; this is a deprecated use case. Please "
- "make use of the actual mapped columns in ORM-evaluated "
- "UPDATE / DELETE expressions."
- )
-
- else:
- raise UnevaluatableError(f"Cannot evaluate column: {clause}")
+ except KeyError as ke:
+ raise UnevaluatableError(
+ f"Cannot evaluate column: {clause}"
+ ) from ke
+
+ if self.target_cls and not issubclass(
+ self.target_cls, parentmapper.class_
+ ):
+ raise UnevaluatableError(
+ "Can't evaluate criteria against "
+ f"alternate class {parentmapper.class_}"
+ )
+
+ parentmapper._check_configure()
+
+ # we'd like to use "proxy_key" annotation to get the "key", however
+ # in relationship primaryjoin cases proxy_key is sometimes deannotated
+ # and sometimes apparently not present in the first place (?).
+ # While I can stop it from being deannotated (though need to see if
+ # this breaks other things), not sure right now about cases where it's
+ # not there in the first place. can fix at some later point.
+ # key = clause._annotations["proxy_key"]
+
+ # for now, use the old way
+ try:
+ key = parentmapper._columntoproperty[clause].key
+ except orm_exc.UnmappedColumnError as err:
+ raise UnevaluatableError(
+ f"Cannot evaluate expression: {err}"
+ ) from err
+
+ # note this used to fall back to a simple `getattr(obj, key)` evaluator
+ # if impl was None; as of #8656, we ensure mappers are configured
+ # so that impl is available
+ impl = parentmapper.class_manager[key].impl
def get_corresponding_attr(obj):
if obj is None:
return _NO_OBJECT
- return getattr(obj, key, _EXPIRED_OBJECT)
+ state = inspect(obj)
+ dict_ = state.dict
+
+ value = impl.get(
+ state, dict_, passive=PassiveFlag.PASSIVE_NO_FETCH
+ )
+ if value is LoaderCallableStatus.PASSIVE_NO_RESULT:
+ return _EXPIRED_OBJECT
+ return value
return get_corresponding_attr
"""remove the parententity annotation from our join conditions which
can leak in here based on some declarative patterns and maybe others.
- We'd want to remove "parentmapper" also, but apparently there's
- an exotic use case in _join_fixture_inh_selfref_w_entity
+ "parentmapper" is relied upon both by the ORM evaluator as well as
+ the use case in _join_fixture_inh_selfref_w_entity
that relies upon it being present, see :ticket:`3364`.
"""
from sqlalchemy.testing import assert_raises
from sqlalchemy.testing import assert_raises_message
from sqlalchemy.testing import eq_
-from sqlalchemy.testing import expect_warnings
from sqlalchemy.testing import fixtures
from sqlalchemy.testing import is_
from sqlalchemy.testing.assertions import expect_raises_message
],
)
- def test_warn_on_unannotated_matched_column(self):
+ def test_raise_on_unannotated_matched_column(self):
+ """test originally for warning emitted in #4073,
+ now updated for #8656."""
+
User = self.classes.User
compiler = evaluator.EvaluatorCompiler(User)
- with expect_warnings(
- r"Evaluating non-mapped column expression 'othername' "
- "onto ORM instances; this is a deprecated use case."
+ with expect_raises_message(
+ evaluator.UnevaluatableError,
+ "Cannot evaluate column: othername",
):
- meth = compiler.process(User.name == Column("othername", String))
-
- u1 = User(id=5)
- meth(u1)
+ compiler.process(User.name == Column("othername", String))
def test_raise_on_unannotated_unmatched_column(self):
User = self.classes.User
from sqlalchemy import testing
from sqlalchemy import text
from sqlalchemy import update
+from sqlalchemy import values
from sqlalchemy.orm import backref
from sqlalchemy.orm import exc as orm_exc
from sqlalchemy.orm import joinedload
synchronize_session="fake",
)
+ @testing.requires.table_value_constructor
+ def test_update_against_external_non_mapped_cols(self):
+ """test #8656"""
+ User = self.classes.User
+
+ data = values(
+ column("id", Integer),
+ column("name", String),
+ column("age_int", Integer),
+ name="myvalues",
+ ).data(
+ [
+ (1, "new john", 35),
+ (3, "new jill", 39),
+ ]
+ )
+
+ # this statement will use "fetch" strategy, as "evaluate" will
+ # not be available
+ stmt = (
+ update(User)
+ .where(User.id == data.c.id)
+ .values(age=data.c.age_int, name=data.c.name)
+ )
+ s = fixture_session()
+
+ john, jack, jill, jane = s.scalars(
+ select(User).order_by(User.id)
+ ).all()
+
+ s.execute(stmt)
+
+ eq_(john.name, "new john")
+ eq_(jill.name, "new jill")
+ eq_(john.age, 35)
+ eq_(jill.age, 39)
+
def test_illegal_operations(self):
User = self.classes.User
Address = self.classes.Address
projects / thirdparty / sqlalchemy / sqlalchemy.git / commitdiff
