Reword implicit left join error; ensure deterministic FROM for columns

Adjusted the error message emitted by :meth:`.Query.join` when a left hand
side can't be located that the :meth:`.Query.select_from` method is the
best way to resolve the issue.  Also, within the 1.3 series, used a
deterministic ordering when determining the FROM clause from a given column
entity passed to :class:`.Query` so that the same expression is determined
each time.

Fixes: #5194
Change-Id: I2e4065fd31e98c57edf2f11d5e831be44d2c1ea2
(cherry picked from commit 7d8c64f9087bcde14dc641fe75f93a04823b86d6)

diff --git a/doc/build/changelog/unreleased_13/5194.rst b/doc/build/changelog/unreleased_13/5194.rst
new file mode 100644 (file)
index 0000000..7047fb3
--- /dev/null
+++ b/doc/build/changelog/unreleased_13/5194.rst
@@ -0,0 +1,11 @@
+.. change::
+    :tags: bug, orm
+    :tickets: 5194
+
+    Adjusted the error message emitted by :meth:`.Query.join` when a left hand
+    side can't be located that the :meth:`.Query.select_from` method is the
+    best way to resolve the issue.  Also, within the 1.3 series, used a
+    deterministic ordering when determining the FROM clause from a given column
+    entity passed to :class:`.Query` so that the same expression is determined
+    each time.
+

diff --git a/lib/sqlalchemy/orm/query.py b/lib/sqlalchemy/orm/query.py
index 4737b16de9e8d7c429ec91962479d63aaf3eabe5..478e70283cb34e99e3ea76d0e90b945b20379746 100644 (file)
--- a/lib/sqlalchemy/orm/query.py
+++ b/lib/sqlalchemy/orm/query.py
@@ -2585,14 +2585,18 @@ class Query(object):
                 raise sa_exc.InvalidRequestError(
                     "Can't determine which FROM clause to join "
                     "from, there are multiple FROMS which can "
-                    "join to this entity. Try adding an explicit ON clause "
-                    "to help resolve the ambiguity."
+                    "join to this entity. Please use the .select_from() "
+                    "method to establish an explicit left side, as well as "
+                    "providing an explcit ON clause if not present already to "
+                    "help resolve the ambiguity."
                 )
             else:
                 raise sa_exc.InvalidRequestError(
-                    "Don't know how to join to %s; please use "
-                    "an ON clause to more clearly establish the left "
-                    "side of this join" % (right,)
+                    "Don't know how to join to %r. "
+                    "Please use the .select_from() "
+                    "method to establish an explicit left side, as well as "
+                    "providing an explcit ON clause if not present already to "
+                    "help resolve the ambiguity." % (right,)
                 )
 
         elif self._entities:
@@ -2627,14 +2631,18 @@ class Query(object):
                 raise sa_exc.InvalidRequestError(
                     "Can't determine which FROM clause to join "
                     "from, there are multiple FROMS which can "
-                    "join to this entity. Try adding an explicit ON clause "
-                    "to help resolve the ambiguity."
+                    "join to this entity. Please use the .select_from() "
+                    "method to establish an explicit left side, as well as "
+                    "providing an explcit ON clause if not present already to "
+                    "help resolve the ambiguity."
                 )
             else:
                 raise sa_exc.InvalidRequestError(
-                    "Don't know how to join to %s; please use "
-                    "an ON clause to more clearly establish the left "
-                    "side of this join" % (right,)
+                    "Don't know how to join to %r. "
+                    "Please use the .select_from() "
+                    "method to establish an explicit left side, as well as "
+                    "providing an explcit ON clause if not present already to "
+                    "help resolve the ambiguity." % (right,)
                 )
         else:
             raise sa_exc.InvalidRequestError(
@@ -4599,7 +4607,8 @@ class _ColumnEntity(_QueryEntity):
         # of FROMs for the overall expression - this helps
         # subqueries which were built from ORM constructs from
         # leaking out their entities into the main select construct
-        self.actual_froms = actual_froms = set(column._from_objects)
+        self.actual_froms = list(column._from_objects)
+        actual_froms = set(self.actual_froms)
 
         if not search_entities:
             self.entity_zero = _entity
@@ -4652,7 +4661,7 @@ class _ColumnEntity(_QueryEntity):
         if self.entity_zero is not None:
             return self.entity_zero
         elif self.actual_froms:
-            return list(self.actual_froms)[0]
+            return self.actual_froms[0]
         else:
             return None
 
@@ -4666,7 +4675,9 @@ class _ColumnEntity(_QueryEntity):
         if "selectable" not in self.__dict__:
             self.selectable = ext_info.selectable
 
-        if self.actual_froms.intersection(ext_info.selectable._from_objects):
+        if set(self.actual_froms).intersection(
+            ext_info.selectable._from_objects
+        ):
             self.froms.add(ext_info.selectable)
 
     def corresponds_to(self, entity):

diff --git a/test/orm/test_joins.py b/test/orm/test_joins.py
index e654cc86c12862f05ff5984b40b8b6851b4e470a..eca4068b4ef6e700c91c6636a7c14b15c1ba3588 100644 (file)
--- a/test/orm/test_joins.py
+++ b/test/orm/test_joins.py
@@ -1338,9 +1338,9 @@ class JoinTest(QueryTest, AssertsCompiledSQL):
 
         assert_raises_message(
             sa.exc.InvalidRequestError,
-            "Don't know how to join to .*Item.*; "
-            "please use an ON clause to more clearly establish the "
-            "left side of this join",
+            "Don't know how to join to .*Item.*. "
+            r"Please use the .select_from\(\) "
+            "method to establish an explicit left side, as well as",
             q.join,
             Item,
         )
@@ -1353,9 +1353,9 @@ class JoinTest(QueryTest, AssertsCompiledSQL):
 
         assert_raises_message(
             sa.exc.InvalidRequestError,
-            "Don't know how to join to .*Item.*; "
-            "please use an ON clause to more clearly establish the "
-            "left side of this join",
+            "Don't know how to join to .*Item.*. "
+            r"Please use the .select_from\(\) "
+            "method to establish an explicit left side, as well as",
             q.join,
             Item,
         )
@@ -1375,9 +1375,9 @@ class JoinTest(QueryTest, AssertsCompiledSQL):
 
         assert_raises_message(
             sa.exc.InvalidRequestError,
-            "Don't know how to join to .*Item.*; "
-            "please use an ON clause to more clearly establish the "
-            "left side of this join",
+            "Don't know how to join to .*Item.*. "
+            r"Please use the .select_from\(\) "
+            "method to establish an explicit left side, as well as",
             q.join,
             Item,
         )
@@ -1473,7 +1473,8 @@ class JoinTest(QueryTest, AssertsCompiledSQL):
             sa.exc.InvalidRequestError,
             "Can't determine which FROM clause to join from, there are "
             "multiple FROMS which can join to this entity. "
-            "Try adding an explicit ON clause to help resolve the ambiguity.",
+            r"Please use the .select_from\(\) "
+            "method to establish an explicit left side, as well as",
             q.join,
             a1,
         )
@@ -1529,7 +1530,8 @@ class JoinTest(QueryTest, AssertsCompiledSQL):
             sa.exc.InvalidRequestError,
             "Can't determine which FROM clause to join from, there are "
             "multiple FROMS which can join to this entity. "
-            "Try adding an explicit ON clause to help resolve the ambiguity.",
+            r"Please use the .select_from\(\) "
+            "method to establish an explicit left side, as well as",
             q.join,
             a1,
         )
@@ -1580,7 +1582,8 @@ class JoinTest(QueryTest, AssertsCompiledSQL):
             sa.exc.InvalidRequestError,
             "Can't determine which FROM clause to join from, there are "
             "multiple FROMS which can join to this entity. "
-            "Try adding an explicit ON clause to help resolve the ambiguity.",
+            r"Please use the .select_from\(\) "
+            "method to establish an explicit left side, as well as",
             q.outerjoin,
             a1,
         )
@@ -2369,14 +2372,18 @@ class JoinTest(QueryTest, AssertsCompiledSQL):
 
         assert_raises_message(
             sa_exc.InvalidRequestError,
-            "Don't know how to join to .*User.* please use an ON clause to ",
+            "Don't know how to join to .*User.*. "
+            r"Please use the .select_from\(\) "
+            "method to establish an explicit left side, as well as",
             sess.query(users.c.id).join,
             User,
         )
 
         assert_raises_message(
             sa_exc.InvalidRequestError,
-            "Don't know how to join to .*User.* please use an ON clause to ",
+            "Don't know how to join to .*User.* "
+            r"Please use the .select_from\(\) "
+            "method to establish an explicit left side, as well as",
             sess.query(users.c.id).select_from(users).join,
             User,
         )