simplify unmapped col eval fallback

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.

Fixes: #8656
Change-Id: Idb8b4a86d3caed89f69cde1607886face103cf6a

2 files changed, 47 insertions, 53 deletions

diff --git a/lib/sqlalchemy/orm/evaluator.py b/lib/sqlalchemy/orm/evaluator.py
index 3c0e62ef5..bf6bff037 100644
--- a/lib/sqlalchemy/orm/evaluator.py
+++ b/lib/sqlalchemy/orm/evaluator.py
@@ -14,7 +14,6 @@ from .base import LoaderCallableStatus
 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
@@ -73,61 +72,56 @@ class EvaluatorCompiler:
         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
 
diff --git a/lib/sqlalchemy/orm/relationships.py b/lib/sqlalchemy/orm/relationships.py
index bae381961..86a0f82c5 100644
--- a/lib/sqlalchemy/orm/relationships.py
+++ b/lib/sqlalchemy/orm/relationships.py
@@ -2314,8 +2314,8 @@ class JoinCondition:
         """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`.
 
         """