Merge "Omit onclause as source of FROMs from a Join"

3 files changed, 39 insertions, 6 deletions

diff --git a/doc/build/changelog/unreleased_14/4621.rst b/doc/build/changelog/unreleased_14/4621.rst
new file mode 100644
index 000000000..224778342
--- /dev/null
+++ b/doc/build/changelog/unreleased_14/4621.rst
@@ -0,0 +1,12 @@
+.. change::
+    :tags: bug, sql
+    :tickets: 4621
+
+    The :class:`.Join` construct no longer considers the "onclause" as a source
+    of additional FROM objects to be omitted from the FROM list of an enclosing
+    :class:`.Select` object as standalone FROM objects. This applies to an ON
+    clause that includes a reference to another  FROM object outside the JOIN;
+    while this is usually not correct from a SQL perspective, it's also
+    incorrect for it to be omitted, and the behavioral change makes the
+    :class:`.Select` / :class:`.Join` behave a bit more intuitively.
+
diff --git a/lib/sqlalchemy/sql/selectable.py b/lib/sqlalchemy/sql/selectable.py
index 6282cf2ee..6a7413fc0 100644
--- a/lib/sqlalchemy/sql/selectable.py
+++ b/lib/sqlalchemy/sql/selectable.py
@@ -1157,12 +1157,7 @@ class Join(FromClause):
 
     @property
     def _from_objects(self):
-        return (
-            [self]
-            + self.onclause._from_objects
-            + self.left._from_objects
-            + self.right._from_objects
-        )
+        return [self] + self.left._from_objects + self.right._from_objects
 
 
 # FromClause ->
diff --git a/test/sql/test_selectable.py b/test/sql/test_selectable.py
index 00b9b68c7..2bc7ccc93 100644
--- a/test/sql/test_selectable.py
+++ b/test/sql/test_selectable.py
@@ -471,6 +471,32 @@ class SelectableTest(
         criterion = a.c.col1 == table2.c.col2
         self.assert_(criterion.compare(j.onclause))
 
+    def test_join_doesnt_derive_from_onclause(self):
+        # test issue #4621.   the hide froms from the join comes from
+        # Join._from_obj(), which should not include tables in the ON clause
+        t1 = table("t1", column("a"))
+        t2 = table("t2", column("b"))
+        t3 = table("t3", column("c"))
+        t4 = table("t4", column("d"))
+
+        j = t1.join(t2, onclause=t1.c.a == t3.c.c)
+
+        j2 = t4.join(j, onclause=t4.c.d == t2.c.b)
+
+        stmt = select([t1, t2, t3, t4]).select_from(j2)
+        self.assert_compile(
+            stmt,
+            "SELECT t1.a, t2.b, t3.c, t4.d FROM t3, "
+            "t4 JOIN (t1 JOIN t2 ON t1.a = t3.c) ON t4.d = t2.b",
+        )
+
+        stmt = select([t1]).select_from(t3).select_from(j2)
+        self.assert_compile(
+            stmt,
+            "SELECT t1.a FROM t3, t4 JOIN (t1 JOIN t2 ON t1.a = t3.c) "
+            "ON t4.d = t2.b",
+        )
+
     @testing.fails("not supported with rework, need a new approach")
     def test_alias_handles_column_context(self):
         # not quite a use case yet but this is expected to become