From: Mike Bayer <mike_mp@zzzcomputing.com>
Date: Mon, 12 Nov 2018 20:09:37 +0000 (-0500)
Subject: Insert primary entity in dynamic "secondary"
X-Git-Tag: rel_1_2_15~20
X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=105d8d2fcaaf77ee2632d8f5ea959b8a3f6b7030;p=thirdparty%2Fsqlalchemy%2Fsqlalchemy.git

Insert primary entity in dynamic "secondary"

Fixed regression caused by :ticket:`4349` where adding the "secondary"
table to the FROM clause for a dynamic loader would affect the ability of
the :class:`.Query` to make a subsequent join to another entity.   The fix
adds the primary entity as the first element of the FROM list since
:meth:`.Query.join` wants to jump from that.   Version 1.3 will have
a more comprehensive solution to this problem as well (:ticket:`4365`).

Fixes: #4363
Change-Id: I1abbb6207722619dc5369e1fd96de43d60a1ee62
(cherry picked from commit 40c1a46e993b5c5ff917ce41c3dca66c139bde94)
---

diff --git a/doc/build/changelog/unreleased_12/4363.rst b/doc/build/changelog/unreleased_12/4363.rst
new file mode 100644
index 0000000000..1a05327636
--- /dev/null
+++ b/doc/build/changelog/unreleased_12/4363.rst
@@ -0,0 +1,13 @@
+.. change::
+    :tags: bug, orm
+    :tickets: 4363
+
+    Fixed regression caused by :ticket:`4349` where adding the "secondary"
+    table to the FROM clause for a dynamic loader would affect the ability of
+    the :class:`.Query` to make a subsequent join to another entity.   The fix
+    adds the primary entity as the first element of the FROM list since
+    :meth:`.Query.join` wants to jump from that.   Version 1.3 will have
+    a more comprehensive solution to this problem as well (:ticket:`4365`).
+
+
+
diff --git a/lib/sqlalchemy/orm/dynamic.py b/lib/sqlalchemy/orm/dynamic.py
index 3c59f61d70..087e7dcc64 100644
--- a/lib/sqlalchemy/orm/dynamic.py
+++ b/lib/sqlalchemy/orm/dynamic.py
@@ -221,7 +221,13 @@ class AppenderMixin(object):
         prop = mapper._props[self.attr.key]
 
         if prop.secondary is not None:
-            self._set_select_from([prop.secondary], False)
+            # this is a hack right now.  The Query only knows how to
+            # make subsequent joins() without a given left-hand side
+            # from self._from_obj[0].  We need to ensure prop.secondary
+            # is in the FROM.  So we purposly put the mapper selectable
+            # in _from_obj[0] to ensure a user-defined join() later on
+            # doesn't fail, and secondary is then in _from_obj[1].
+            self._from_obj = (prop.mapper.selectable, prop.secondary)
 
         self._criterion = prop._with_parent(
             instance,
diff --git a/test/orm/test_dynamic.py b/test/orm/test_dynamic.py
index 5c514ff1a5..5dfb3fde51 100644
--- a/test/orm/test_dynamic.py
+++ b/test/orm/test_dynamic.py
@@ -263,6 +263,7 @@ class DynamicTest(_DynamicFixture, _fixtures.FixtureTest, AssertsCompiledSQL):
         )
 
     def test_secondary_as_join(self):
+        # test [ticket:4349]
         User, users = self.classes.User, self.tables.users
         items, orders, order_items, Item = (self.tables.items,
                                             self.tables.orders,
@@ -292,6 +293,47 @@ class DynamicTest(_DynamicFixture, _fixtures.FixtureTest, AssertsCompiledSQL):
             use_default_dialect=True
         )
 
+    def test_secondary_doesnt_interfere_w_join_to_fromlist(self):
+        # tests that the "secondary" being added to the FROM
+        # as part of [ticket:4349] does not prevent a subsequent join to
+        # an entity that does not provide any "left side".  Query right now
+        # does not know how to join() like this unambiguously if _from_obj is
+        # more than one element long.
+        Order, orders = self.classes.Order, self.tables.orders
+
+        items, order_items, Item = (
+            self.tables.items,
+            self.tables.order_items,
+            self.classes.Item)
+        item_keywords = self.tables.item_keywords
+
+        class ItemKeyword(object):
+            pass
+
+        mapper(Order, orders, properties={
+            'items': relationship(Item, secondary=order_items, lazy='dynamic'),
+        })
+        mapper(
+            ItemKeyword, item_keywords,
+            primary_key=[item_keywords.c.item_id, item_keywords.c.keyword_id])
+        mapper(Item, items, properties={
+            'item_keywords': relationship(ItemKeyword)
+        })
+
+        sess = create_session()
+        order = sess.query(Order).first()
+
+        self.assert_compile(
+            order.items.join(ItemKeyword),
+            "SELECT items.id AS items_id, "
+            "items.description AS items_description "
+            "FROM order_items, items "
+            "JOIN item_keywords ON items.id = item_keywords.item_id "
+            "WHERE :param_1 = order_items.order_id "
+            "AND items.id = order_items.item_id",
+            use_default_dialect=True
+        )
+
     def test_transient_count(self):
         User, Address = self._user_address_fixture()
         u1 = User()