From: RamonWill <ramonwilliams@hotmail.co.uk>
Date: Tue, 25 Aug 2020 00:14:15 +0000 (-0400)
Subject: Provide a more detailed error message for Query.join()
X-Git-Tag: rel_1_4_0b1~134^2
X-Git-Url: http://git.ipfire.org/?a=commitdiff_plain;h=17090c004e19afab35c837bf880ea5b328e1fb56;p=thirdparty%2Fsqlalchemy%2Fsqlalchemy.git

Provide a more detailed error message for Query.join()

An :class:`.ArgumentError` with more detail is now raised if the target
parameter for :meth:`_query.Query.join` is set to an unmapped object.
Prior to this change a less detailed ``AttributeError`` was raised.
Pull request courtesy Ramon Williams.

Fixes: #4428
Closes: #5452
Pull-request: https://github.com/sqlalchemy/sqlalchemy/pull/5452
Pull-request-sha: b148df547037e9a254fe331eff8e922c78426261

Change-Id: I873453d1fdb651178216aac698baac63ae5a94e8
---

diff --git a/doc/build/changelog/unreleased_13/4428.rst b/doc/build/changelog/unreleased_13/4428.rst
new file mode 100644
index 0000000000..e677669976
--- /dev/null
+++ b/doc/build/changelog/unreleased_13/4428.rst
@@ -0,0 +1,8 @@
+.. change::
+    :tags: bug, orm
+    :tickets: 4428
+
+    An :class:`.ArgumentError` with more detail is now raised if the target
+    parameter for :meth:`_query.Query.join` is set to an unmapped object.
+    Prior to this change a less detailed ``AttributeError`` was raised.
+    Pull request courtesy Ramon Williams.
diff --git a/lib/sqlalchemy/orm/context.py b/lib/sqlalchemy/orm/context.py
index 0868fb29b4..d9e334d45a 100644
--- a/lib/sqlalchemy/orm/context.py
+++ b/lib/sqlalchemy/orm/context.py
@@ -1170,7 +1170,18 @@ class ORMSelectCompileState(ORMCompileState, SelectState):
                     if of_type:
                         right = of_type
                     else:
-                        right = onclause.property.entity
+                        right = onclause.property
+
+                        try:
+                            right = right.entity
+                        except AttributeError as err:
+                            util.raise_(
+                                sa_exc.ArgumentError(
+                                    "Join target %s does not refer to a "
+                                    "mapped entity" % right
+                                ),
+                                replace_context=err,
+                            )
 
                 left = onclause._parententity
 
@@ -1312,7 +1323,18 @@ class ORMSelectCompileState(ORMCompileState, SelectState):
                     if of_type:
                         right = of_type
                     else:
-                        right = onclause.property.entity
+                        right = onclause.property
+
+                        try:
+                            right = right.entity
+                        except AttributeError as err:
+                            util.raise_(
+                                sa_exc.ArgumentError(
+                                    "Join target %s does not refer to a "
+                                    "mapped entity" % right
+                                ),
+                                replace_context=err,
+                            )
 
                 left = onclause._parententity
 
diff --git a/test/orm/test_joins.py b/test/orm/test_joins.py
index 4ffa5fb9e9..8225214f63 100644
--- a/test/orm/test_joins.py
+++ b/test/orm/test_joins.py
@@ -2220,17 +2220,55 @@ class JoinTest(QueryTest, AssertsCompiledSQL):
             ._compile_context,
         )
 
-    def test_on_clause_no_right_side(self):
+    def test_on_clause_no_right_side_one(self):
         User = self.classes.User
         Address = self.classes.Address
         sess = create_session()
 
+        # coercions does not catch this due to the
+        # legacy=True flag for JoinTargetRole
         assert_raises_message(
             sa_exc.ArgumentError,
             "Expected mapped entity or selectable/table as join target",
             sess.query(User).join(User.id == Address.user_id)._compile_context,
         )
 
+    def test_on_clause_no_right_side_one_future(self):
+        User = self.classes.User
+        Address = self.classes.Address
+
+        # future mode can raise a more specific error at the coercions level
+        assert_raises_message(
+            sa_exc.ArgumentError,
+            "Join target, typically a FROM expression, "
+            "or ORM relationship attribute expected",
+            select(User).join,
+            User.id == Address.user_id,
+        )
+
+    def test_on_clause_no_right_side_two(self):
+        User = self.classes.User
+        Address = self.classes.Address
+        sess = create_session()
+
+        assert_raises_message(
+            sa_exc.ArgumentError,
+            "Join target Address.user_id does not refer to a mapped entity",
+            sess.query(User).join(Address.user_id)._compile_context,
+        )
+
+    def test_on_clause_no_right_side_two_future(self):
+        User = self.classes.User
+        Address = self.classes.Address
+
+        stmt = select(User).join(Address.user_id)
+
+        assert_raises_message(
+            sa_exc.ArgumentError,
+            "Join target Address.user_id does not refer to a mapped entity",
+            stmt.compile,
+        )
+
     def test_select_from(self):
         """Test that the left edge of the join can be set reliably with
         select_from()."""