omit_join=True is not supported

The :paramref:`.relationship.omit_join` flag was not intended to be
manually set to True, and will now emit a warning when this occurs.  The
omit_join optimization is detected automatically, and the ``omit_join``
flag was only intended to disable the optimization in the hypothetical case
that the optimization may have interfered with correct results, which has
not been observed with the modern version of this feature.   Setting the
flag to True when it is not automatically detected may cause the selectin
load feature to not work correctly when a non-default primary join
condition is in use.

Fixes: #4954
Change-Id: I3afebefb13ddaf8f74c5c2b9e6e6a1a261ac5629
(cherry picked from commit 9b774743952fa4f459a914604cc8395175eca07e)

diff --git a/doc/build/changelog/unreleased_13/4954.rst b/doc/build/changelog/unreleased_13/4954.rst
new file mode 100644 (file)
index 0000000..1c147d1
--- /dev/null
+++ b/doc/build/changelog/unreleased_13/4954.rst
@@ -0,0 +1,14 @@
+.. change::
+    :tags: bug, orm
+    :tickets: 4954
+
+    The :paramref:`.relationship.omit_join` flag was not intended to be
+    manually set to True, and will now emit a warning when this occurs.  The
+    omit_join optimization is detected automatically, and the ``omit_join``
+    flag was only intended to disable the optimization in the hypothetical case
+    that the optimization may have interfered with correct results, which has
+    not been observed with the modern version of this feature.   Setting the
+    flag to True when it is not automatically detected may cause the selectin
+    load feature to not work correctly when a non-default primary join
+    condition is in use.
+

diff --git a/lib/sqlalchemy/orm/relationships.py b/lib/sqlalchemy/orm/relationships.py
index ccb0efe606726abf555ec75f02c3fc9c851a4c05..1695ba7c98bed9f1d205bb3626b493b912d11356 100644 (file)
--- a/lib/sqlalchemy/orm/relationships.py
+++ b/lib/sqlalchemy/orm/relationships.py
@@ -830,7 +830,16 @@ class RelationshipProperty(StrategizedProperty):
         :param omit_join:
           Allows manual control over the "selectin" automatic join
           optimization.  Set to ``False`` to disable the "omit join" feature
-          added in SQLAlchemy 1.3.
+          added in SQLAlchemy 1.3; or leave as ``None`` to leave automatic
+          optimization in place.
+
+          .. note:: This flag may only be set to ``False``.   It is not
+             necessary to set it to ``True`` as the "omit_join" optimization is
+             automatically detected; if it is not detected, then the
+             optimization is not supported.
+
+             .. versionchanged:: 1.3.11  setting ``omit_join`` to True will now
+                emit a warning as this was not the intended use of this flag.
 
           .. versionadded:: 1.3
 
@@ -861,6 +870,15 @@ class RelationshipProperty(StrategizedProperty):
         self.doc = doc
         self.active_history = active_history
         self.join_depth = join_depth
+        if omit_join:
+            util.warn(
+                "setting omit_join to True is not supported; selectin "
+                "loading of this relationship may not work correctly if this "
+                "flag is set explicitly.  omit_join optimization is "
+                "automatically detected for conditions under which it is "
+                "supported."
+            )
+
         self.omit_join = omit_join
         self.local_remote_pairs = _local_remote_pairs
         self.extension = extension

diff --git a/lib/sqlalchemy/orm/strategies.py b/lib/sqlalchemy/orm/strategies.py
index 32ebe1081cd1fc9789ee214480bd92a5b004c325..caf708950cc78e73008fed4a4f92fc92a5452998 100644 (file)
--- a/lib/sqlalchemy/orm/strategies.py
+++ b/lib/sqlalchemy/orm/strategies.py
@@ -2112,6 +2112,7 @@ class SelectInLoader(AbstractRelationshipLoader, util.MemoizedSlots):
                     use_proxies=True,
                     equivalents=self.parent._equivalent_columns,
                 )
+
         if self.omit_join:
             if is_m2o:
                 self._query_info = self._init_for_omit_join_m2o()

diff --git a/test/orm/test_selectin_relations.py b/test/orm/test_selectin_relations.py
index 1fc19ed424592a936fa3a7f2ad96ec8ac9cae64e..23f083ad212906beb113eb5868a95f9c3233d29a 100644 (file)
--- a/test/orm/test_selectin_relations.py
+++ b/test/orm/test_selectin_relations.py
@@ -3108,6 +3108,7 @@ class M2OWDegradeTest(
             id = Column(Integer, primary_key=True)
             b_id = Column(ForeignKey("b.id"))
             b = relationship("B")
+            b_no_omit_join = relationship("B", omit_join=False)
             q = Column(Integer)
 
         class B(fixtures.ComparableEntity, Base):
@@ -3133,6 +3134,13 @@ class M2OWDegradeTest(
         )
         s.commit()
 
+    def test_omit_join_warn_on_true(self):
+        with testing.expect_warnings(
+            "setting omit_join to True is not supported; selectin "
+            "loading of this relationship"
+        ):
+            relationship("B", omit_join=True)
+
     def test_use_join_parent_criteria(self):
         A, B = self.classes("A", "B")
         s = Session()
@@ -3228,6 +3236,38 @@ class M2OWDegradeTest(
             ],
         )
 
+    def test_use_join_omit_join_false(self):
+        A, B = self.classes("A", "B")
+        s = Session()
+        q = s.query(A).options(selectinload(A.b_no_omit_join)).order_by(A.id)
+        results = self.assert_sql_execution(
+            testing.db,
+            q.all,
+            CompiledSQL(
+                "SELECT a.id AS a_id, a.b_id AS a_b_id, a.q AS a_q "
+                "FROM a ORDER BY a.id",
+                [{}],
+            ),
+            CompiledSQL(
+                "SELECT a_1.id AS a_1_id, b.id AS b_id, b.x AS b_x, "
+                "b.y AS b_y FROM a AS a_1 JOIN b ON b.id = a_1.b_id "
+                "WHERE a_1.id IN ([EXPANDING_primary_keys]) ORDER BY a_1.id",
+                [{"primary_keys": [1, 2, 3, 4, 5]}],
+            ),
+        )
+
+        b1, b2 = B(id=1, x=5, y=9), B(id=2, x=10, y=8)
+        eq_(
+            results,
+            [
+                A(id=1, b_no_omit_join=b1),
+                A(id=2, b_no_omit_join=b2),
+                A(id=3, b_no_omit_join=b2),
+                A(id=4, b_no_omit_join=None),
+                A(id=5, b_no_omit_join=b1),
+            ],
+        )
+
     def test_use_join_parent_degrade_on_defer(self):
         A, B = self.classes("A", "B")
         s = Session()