From: Mike Bayer Date: Fri, 15 Oct 2021 15:03:46 +0000 (-0400) Subject: fix with_loader_criteria for select(A).join(B) X-Git-Tag: rel_1_4_26~12^2 X-Git-Url: http://git.ipfire.org/?a=commitdiff_plain;h=a9d85047cd73f206314afc8e25954b6fb0920afb;p=thirdparty%2Fsqlalchemy%2Fsqlalchemy.git fix with_loader_criteria for select(A).join(B) Also revise the contains_eager() note that was just added in fec2b6560c14bb28ee7f, as I mistakenly forgot that WLC also affects elements of the query that are there as part of the normal entites/criteria. Fixes: #7189 Change-Id: Ibd8a1826aaee8e91ab6704173df5fccb56b7e785 --- diff --git a/doc/build/changelog/unreleased_14/7189.rst b/doc/build/changelog/unreleased_14/7189.rst new file mode 100644 index 0000000000..adae7749cc --- /dev/null +++ b/doc/build/changelog/unreleased_14/7189.rst @@ -0,0 +1,8 @@ +.. change:: + :tags: bug, orm + :tickets: 7189 + + Fixed issue with :func:`_orm.with_loader_criteria` feature where ON + criteria would not be added to a JOIN for a query of the form + ``select(A).join(B)``, stating a target while making use of an implicit + ON clause. diff --git a/lib/sqlalchemy/orm/util.py b/lib/sqlalchemy/orm/util.py index 8e0fd44cda..67be165f6b 100644 --- a/lib/sqlalchemy/orm/util.py +++ b/lib/sqlalchemy/orm/util.py @@ -1012,12 +1012,54 @@ class LoaderCriteriaOption(CriteriaOption): argument. The given class will expand to include all mapped subclass and need not itself be a mapped class. - .. note:: The :func:`_orm.with_loader_criteria` option only applies to - loader options that **render their own SQL**, which means it does - **not** apply to the :func:`_orm.contains_eager` option. To apply - additional criteria to the JOIN used in conjunction with - :func:`_orm.contains_eager`, specify additional criteria within - the :meth:`_sql.select.join` or :meth:`_orm.Query.join` method. + .. tip:: + + When using :func:`_orm.with_loader_criteria` option in + conjunction with the :func:`_orm.contains_eager` loader option, + it's important to note that :func:`_orm.with_loader_criteria` only + affects the part of the query that determines what SQL is rendered + in terms of the WHERE and FROM clauses. The + :func:`_orm.contains_eager` option does not affect the rendering of + the SELECT statement outside of the columns clause, so does not have + any interaction with the :func:`_orm.with_loader_criteria` option. + However, the way things "work" is that :func:`_orm.contains_eager` + is meant to be used with a query that is already selecting from the + additional entities in some way, where + :func:`_orm.with_loader_criteria` can apply it's additional + criteria. + + In the example below, assuming a mapping relationship as + ``A -> A.bs -> B``, the given :func:`_orm.with_loader_criteria` + option will affect the way in which the JOIN is rendered:: + + stmt = select(A).join(A.bs).options( + contains_eager(A.bs), + with_loader_criteria(B, B.flag == 1) + ) + + Above, the given :func:`_orm.with_loader_criteria` option will + affect the ON clause of the JOIN that is specified by + ``.join(A.bs)``, so is applied as expected. The + :func:`_orm.contains_eager` option has the effect that columns from + ``B`` are added to the columns clause:: + + SELECT + b.id, b.a_id, b.data, b.flag, + a.id AS id_1, + a.data AS data_1 + FROM a JOIN b ON a.id = b.a_id AND b.flag = :flag_1 + + + The use of the :func:`_orm.contains_eager` option within the above + statement has no effect on the behavior of the + :func:`_orm.with_loader_criteria` option. If the + :func:`_orm.contains_eager` option were omitted, the SQL would be + the same as regards the FROM and WHERE clauses, where + :func:`_orm.with_loader_criteria` continues to add its criteria to + the ON clause of the JOIN. The addition of + :func:`_orm.contains_eager` only affects the columns clause, in that + additional columns against ``b`` are added which are then consumed + by the ORM to produce ``B`` instances. .. warning:: The use of a lambda inside of the call to :func:`_orm.with_loader_criteria` is only invoked **once per unique @@ -1647,8 +1689,12 @@ class _ORMJoin(expression.Join): self._target_adapter = target_adapter + augment_onclause = onclause is None and _extra_criteria expression.Join.__init__(self, left, right, onclause, isouter, full) + if augment_onclause: + self.onclause &= sql.and_(*_extra_criteria) + if ( not prop and getattr(right_info, "mapper", None) diff --git a/test/orm/test_relationship_criteria.py b/test/orm/test_relationship_criteria.py index 571d272320..cccf91f7b6 100644 --- a/test/orm/test_relationship_criteria.py +++ b/test/orm/test_relationship_criteria.py @@ -279,6 +279,26 @@ class LoaderCriteriaTest(_Fixtures, testing.AssertsCompiledSQL): "AND addresses.email_address != :email_address_1", ) + def test_select_implicit_join_mapper_mapper_criteria( + self, user_address_fixture + ): + User, Address = user_address_fixture + + stmt = ( + select(User) + .join(Address) + .options( + with_loader_criteria(Address, Address.email_address != "name") + ) + ) + + self.assert_compile( + stmt, + "SELECT users.id, users.name FROM users " + "JOIN addresses ON users.id = addresses.user_id " + "AND addresses.email_address != :email_address_1", + ) + def test_select_joinm2m_mapper_mapper_criteria(self, order_item_fixture): Order, Item = order_item_fixture