From: Mike Bayer Date: Wed, 3 Apr 2019 02:12:09 +0000 (-0400) Subject: Fold entities into existing joins when resolving FROM ambiguity X-Git-Tag: rel_1_3_3~12^2 X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=25a42e93f4ef5ce1a9f9c23fbcdea3e21a7b3f1a;p=thirdparty%2Fsqlalchemy%2Fsqlalchemy.git Fold entities into existing joins when resolving FROM ambiguity Fixed 1.3 regression in new "ambiguous FROMs" query logic introduced in :ref:`change_4365` where a :class:`.Query` that explicitly places an entity in the FROM clause with :meth:`.Query.select_from` and also joins to it using :meth:`.Query.join` would later cause an "ambiguous FROM" error if that entity were used in additional joins, as the entity appears twice in the "from" list of the :class:`.Query`. The fix resolves this ambiguity by folding the standalone entity into the join that it's already a part of in the same way that ultimately happens when the SELECT statement is rendered. Fixes: #4584 Change-Id: Ic62ca09c6d329695b21ed4f1be8621edcbb18e19 --- diff --git a/doc/build/changelog/unreleased_13/4584.rst b/doc/build/changelog/unreleased_13/4584.rst new file mode 100644 index 0000000000..1f40a7014b --- /dev/null +++ b/doc/build/changelog/unreleased_13/4584.rst @@ -0,0 +1,12 @@ +.. change:: + :tags: bug, orm + :tickets: 4584 + + Fixed 1.3 regression in new "ambiguous FROMs" query logic introduced in + :ref:`change_4365` where a :class:`.Query` that explicitly places an entity + in the FROM clause with :meth:`.Query.select_from` and also joins to it + using :meth:`.Query.join` would later cause an "ambiguous FROM" error if + that entity were used in additional joins, as the entity appears twice in + the "from" list of the :class:`.Query`. The fix resolves this ambiguity by + folding the standalone entity into the join that it's already a part of in + the same way that ultimately happens when the SELECT statement is rendered. diff --git a/lib/sqlalchemy/sql/util.py b/lib/sqlalchemy/sql/util.py index 3077840c65..8719b26ec2 100644 --- a/lib/sqlalchemy/sql/util.py +++ b/lib/sqlalchemy/sql/util.py @@ -20,6 +20,7 @@ from .annotation import _shallow_annotate # noqa from .base import _from_objects from .base import ColumnSet from .ddl import sort_tables # noqa +from .elements import _expand_cloned from .elements import _find_columns # noqa from .elements import _label_reference from .elements import _textual_label_reference @@ -149,6 +150,16 @@ def find_left_clause_to_join_from(clauses, join_to, onclause): idx.append(i) break + if len(idx) > 1: + # this is the same "hide froms" logic from + # Selectable._get_display_froms + toremove = set( + chain(*[_expand_cloned(f._hide_froms) for f in clauses]) + ) + idx = [ + i for i in idx if clauses[i] not in toremove + ] + # onclause was given and none of them resolved, so assume # all indexes can match if not idx and onclause is not None: diff --git a/test/orm/test_joins.py b/test/orm/test_joins.py index bb9747cf2f..4fe04f72b1 100644 --- a/test/orm/test_joins.py +++ b/test/orm/test_joins.py @@ -1559,6 +1559,81 @@ class JoinTest(QueryTest, AssertsCompiledSQL): "ON dingalings.address_id = addresses_1.id", ) + def test_clause_present_in_froms_twice_w_onclause(self): + # test [ticket:4584] + Order, Address, Dingaling, User = ( + self.classes.Order, + self.classes.Address, + self.classes.Dingaling, + self.classes.User, + ) + + sess = create_session() + + a1 = aliased(Address) + + q = sess.query(Order).select_from(Order, a1, User) + assert_raises_message( + 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.", + q.outerjoin, + a1, + ) + + # the condition which occurs here is: Query._from_obj contains both + # "a1" by itself as well as a join that "a1" is part of. + # find_left_clause_to_join_from() needs to include removal of froms + # that are in the _hide_froms of joins the same way + # Selectable._get_display_froms does. + q = sess.query(Order).select_from(Order, a1, User) + q = q.outerjoin(a1, a1.id == Order.address_id) + q = q.outerjoin(User, a1.user_id == User.id) + + self.assert_compile( + q, + "SELECT orders.id AS orders_id, orders.user_id AS orders_user_id, " + "orders.address_id AS orders_address_id, " + "orders.description AS orders_description, " + "orders.isopen AS orders_isopen " + "FROM orders " + "LEFT OUTER JOIN addresses AS addresses_1 " + "ON addresses_1.id = orders.address_id " + "LEFT OUTER JOIN users ON addresses_1.user_id = users.id", + ) + + def test_clause_present_in_froms_twice_wo_onclause(self): + # test [ticket:4584] + Order, Address, Dingaling, User = ( + self.classes.Order, + self.classes.Address, + self.classes.Dingaling, + self.classes.User, + ) + + sess = create_session() + + a1 = aliased(Address) + + # the condition which occurs here is: Query._from_obj contains both + # "a1" by itself as well as a join that "a1" is part of. + # find_left_clause_to_join_from() needs to include removal of froms + # that are in the _hide_froms of joins the same way + # Selectable._get_display_froms does. + q = sess.query(User).select_from(Dingaling, a1, User) + q = q.outerjoin(a1, User.id == a1.user_id) + q = q.outerjoin(Dingaling) + + self.assert_compile( + q, + "SELECT users.id AS users_id, users.name AS users_name " + "FROM users LEFT OUTER JOIN addresses AS addresses_1 " + "ON users.id = addresses_1.user_id " + "LEFT OUTER JOIN dingalings " + "ON addresses_1.id = dingalings.address_id", + ) + def test_multiple_adaption(self): Item, Order, User = ( self.classes.Item,