From 63c94343a317e1201101027e2cb32fef2437f75d Mon Sep 17 00:00:00 2001 From: Mike Bayer Date: Sun, 19 Apr 2020 16:52:54 -0400 Subject: [PATCH] Raise informative error when non-object m2o comparison used An informative error message is raised when an ORM many-to-one comparison is attempted against an object that is not an actual mapped instance. Comparisons such as those to scalar subqueries aren't supported; generalized comparison with subqueries is better achieved using :meth:`~.RelationshipProperty.Comparator.has`. Fixes: #5269 Change-Id: I2e23178eb59728c39241a46bfa7411239a87492e (cherry picked from commit 430ce5eab26d46301ae741f9068f13ba09907d8e) --- doc/build/changelog/unreleased_13/5269.rst | 10 ++++++ lib/sqlalchemy/orm/relationships.py | 13 +++++++- test/orm/test_query.py | 39 +++++++++++++++++++++- 3 files changed, 60 insertions(+), 2 deletions(-) create mode 100644 doc/build/changelog/unreleased_13/5269.rst diff --git a/doc/build/changelog/unreleased_13/5269.rst b/doc/build/changelog/unreleased_13/5269.rst new file mode 100644 index 0000000000..90b50f5dc5 --- /dev/null +++ b/doc/build/changelog/unreleased_13/5269.rst @@ -0,0 +1,10 @@ +.. change:: + :tags: bug, orm + :tickets: 5269 + + An informative error message is raised when an ORM many-to-one comparison + is attempted against an object that is not an actual mapped instance. + Comparisons such as those to scalar subqueries aren't supported; + generalized comparison with subqueries is better achieved using + :meth:`~.RelationshipProperty.Comparator.has`. + diff --git a/lib/sqlalchemy/orm/relationships.py b/lib/sqlalchemy/orm/relationships.py index 42a1b2826b..daa85c5661 100644 --- a/lib/sqlalchemy/orm/relationships.py +++ b/lib/sqlalchemy/orm/relationships.py @@ -1610,8 +1610,19 @@ class RelationshipProperty(StrategizedProperty): alias_secondary=True, ): if state is not None: - state = attributes.instance_state(state) + try: + state = inspect(state) + except sa_exc.NoInspectionAvailable: + state = None + if state is None or not getattr(state, "is_instance", False): + raise sa_exc.ArgumentError( + "Mapped instance expected for relationship " + "comparison to object. Classes, queries and other " + "SQL elements are not accepted in this context; for " + "comparison with a subquery, " + "use %s.has(**criteria)." % self + ) reverse_direction = not value_is_parent if state is None: diff --git a/test/orm/test_query.py b/test/orm/test_query.py index 2a877015ee..77a820953c 100644 --- a/test/orm/test_query.py +++ b/test/orm/test_query.py @@ -2947,7 +2947,44 @@ class FilterTest(QueryTest, AssertsCompiledSQL): [Order(id=3)], ) - def test_comparison(self): + @testing.combinations( + lambda sess, User, Address: ( + sess.query(Address).filter( + Address.user == sess.query(User).as_scalar() + ) + ), + lambda sess, User, Address: ( + sess.query(Address).filter_by(user=sess.query(User).as_scalar()) + ), + lambda sess, User, Address: ( + sess.query(Address).filter(Address.user == sess.query(User)) + ), + lambda sess, User, Address: ( + sess.query(Address).filter( + Address.user == sess.query(User).subquery() + ) + ), + lambda sess, User, Address: ( + sess.query(Address).filter_by(user="foo") + ), + ) + def test_object_comparison_needs_object(self, fn): + User, Address = ( + self.classes.User, + self.classes.Address, + ) + + sess = create_session() + assert_raises_message( + sa.exc.ArgumentError, + "Mapped instance expected for relationship comparison to object.", + fn, + sess, + User, + Address, + ), + + def test_object_comparison(self): """test scalar comparison to an object instance""" Item, Order, Dingaling, User, Address = ( -- 2.47.2