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 (file)
index 0000000..adae774
--- /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 8e0fd44cda08d9d3cd051d7519907936214f9556..67be165f6b39a7945f9ae7ecef1d1b25e8936ad7 100644 (file)
--- 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 571d2723200a2e09a4c1a8ba9e85f6ca5e069a6f..cccf91f7b6aadee93ea3e19829271e43cd2d747c 100644 (file)
--- 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