diff options
| -rw-r--r-- | doc/build/changelog/unreleased_13/4428.rst | 8 | ||||
| -rw-r--r-- | lib/sqlalchemy/orm/context.py | 26 | ||||
| -rw-r--r-- | test/orm/test_joins.py | 40 |
3 files changed, 71 insertions, 3 deletions
diff --git a/doc/build/changelog/unreleased_13/4428.rst b/doc/build/changelog/unreleased_13/4428.rst new file mode 100644 index 000000000..e67766997 --- /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 0868fb29b..d9e334d45 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 4ffa5fb9e..8225214f6 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().""" |