InstanceState default path is RootRegistry, not tuple

Fixed regression caused in 1.3.13 by :ticket:`5056` where a refactor of the
ORM path registry system made it such that a path could no longer be
compared to an empty tuple, which can occur in a particular kind of joined
eager loading path.   The "empty tuple" use case has been resolved so that
the path registry is compared to a path registry in all cases;  the
:class:`.PathRegistry` object itself now implements ``__eq__()`` and
``__ne__()`` methods which will take place for all equality comparisons and
continue to succeed in the not anticipated case that a non-
:class:`.PathRegistry` object is compared, while emitting a warning that
this object should not be the subject of the comparison.

Fixes: #5110
Change-Id: I6cab6cd771c131d12b17939b369212f12c6bee16
(cherry picked from commit f5eeac3d18892206abcaa30a295d12a799a8fb9b)

diff --git a/doc/build/changelog/unreleased_13/5110.rst b/doc/build/changelog/unreleased_13/5110.rst
new file mode 100644 (file)
index 0000000..54823ab
--- /dev/null
+++ b/doc/build/changelog/unreleased_13/5110.rst
@@ -0,0 +1,16 @@
+.. change::
+    :tags: bug, orm
+    :tickets: 5110
+
+    Fixed regression caused in 1.3.13 by :ticket:`5056` where a refactor of the
+    ORM path registry system made it such that a path could no longer be
+    compared to an empty tuple, which can occur in a particular kind of joined
+    eager loading path.   The "empty tuple" use case has been resolved so that
+    the path registry is compared to a path registry in all cases;  the
+    :class:`.PathRegistry` object itself now implements ``__eq__()`` and
+    ``__ne__()`` methods which will take place for all equality comparisons and
+    continue to succeed in the not anticipated case that a non-
+    :class:`.PathRegistry` object is compared, while emitting a warning that
+    this object should not be the subject of the comparison.
+
+

diff --git a/lib/sqlalchemy/orm/path_registry.py b/lib/sqlalchemy/orm/path_registry.py
index 491bf3337b1edf1a6b3535174486d441eeec6b90..f4e16a9f27206d8c79490dccc78a27d6ed6d23f3 100644 (file)
--- a/lib/sqlalchemy/orm/path_registry.py
+++ b/lib/sqlalchemy/orm/path_registry.py
@@ -60,7 +60,24 @@ class PathRegistry(object):
     is_root = False
 
     def __eq__(self, other):
-        return other is not None and self.path == other.path
+        try:
+            return other is not None and self.path == other.path
+        except AttributeError:
+            util.warn(
+                "Comparison of PathRegistry to %r is not supported"
+                % (type(other))
+            )
+            return False
+
+    def __ne__(self, other):
+        try:
+            return other is None or self.path != other.path
+        except AttributeError:
+            util.warn(
+                "Comparison of PathRegistry to %r is not supported"
+                % (type(other))
+            )
+            return True
 
     def set(self, attributes, key, value):
         log.debug("set '%s' on path '%s' to '%s'", key, self, value)

diff --git a/lib/sqlalchemy/orm/state.py b/lib/sqlalchemy/orm/state.py
index d85592adf45c776fb50bd4e3b65918518cbd275c..4b6d5c53afb7868e8a10588612d1550c8b5b6a4c 100644 (file)
--- a/lib/sqlalchemy/orm/state.py
+++ b/lib/sqlalchemy/orm/state.py
@@ -62,7 +62,7 @@ class InstanceState(interfaces.InspectionAttrInfo):
     key = None
     runid = None
     load_options = util.EMPTY_SET
-    load_path = ()
+    load_path = PathRegistry.root
     insert_order = None
     _strong_obj = None
     modified = False

diff --git a/test/orm/test_loading.py b/test/orm/test_loading.py
index d263b12f2c64d02960651d117c09868b6ed3f9af..5e993a27f9062f2b384b7389667f0708a53d7c6b 100644 (file)
--- a/test/orm/test_loading.py
+++ b/test/orm/test_loading.py
@@ -1,7 +1,10 @@
 from sqlalchemy import exc
 from sqlalchemy import select
+from sqlalchemy import testing
 from sqlalchemy.orm import aliased
 from sqlalchemy.orm import loading
+from sqlalchemy.orm import mapper
+from sqlalchemy.orm import relationship
 from sqlalchemy.orm import Session
 from sqlalchemy.testing import mock
 from sqlalchemy.testing.assertions import assert_raises
@@ -10,10 +13,49 @@ from sqlalchemy.testing.assertions import eq_
 from sqlalchemy.util import KeyedTuple
 from . import _fixtures
 
-
 # class GetFromIdentityTest(_fixtures.FixtureTest):
 # class LoadOnIdentTest(_fixtures.FixtureTest):
-# class InstanceProcessorTest(_fixture.FixtureTest):
+
+
+class InstanceProcessorTest(_fixtures.FixtureTest):
+    def test_state_no_load_path_comparison(self):
+        # test issue #5110
+        User, Order, Address = self.classes("User", "Order", "Address")
+        users, orders, addresses = self.tables("users", "orders", "addresses")
+
+        mapper(
+            User,
+            users,
+            properties={
+                "addresses": relationship(Address, lazy="joined"),
+                "orders": relationship(
+                    Order, lazy="joined", order_by=orders.c.id
+                ),
+            },
+        )
+        mapper(
+            Order,
+            orders,
+            properties={"address": relationship(Address, lazy="joined")},
+        )
+        mapper(Address, addresses)
+
+        s = Session()
+
+        def go():
+            eq_(
+                User(
+                    id=7,
+                    orders=[
+                        Order(id=1, address=Address(id=1)),
+                        Order(id=3, address=Address(id=1)),
+                        Order(id=5, address=None),
+                    ],
+                ),
+                s.query(User).populate_existing().get(7),
+            )
+
+        self.assert_sql_count(testing.db, go, 1)
 
 
 class InstancesTest(_fixtures.FixtureTest):

diff --git a/test/orm/test_utils.py b/test/orm/test_utils.py
index 680862b7b9c77de32199b8d980b5b0fc6bf62508..bbd28770ca1761e882043ce6ab8c6924a8a38015 100644 (file)
--- a/test/orm/test_utils.py
+++ b/test/orm/test_utils.py
@@ -18,6 +18,7 @@ from sqlalchemy.orm.path_registry import RootRegistry
 from sqlalchemy.testing import assert_raises
 from sqlalchemy.testing import AssertsCompiledSQL
 from sqlalchemy.testing import eq_
+from sqlalchemy.testing import expect_warnings
 from sqlalchemy.testing import fixtures
 from sqlalchemy.testing import is_
 from sqlalchemy.util import compat
@@ -621,6 +622,31 @@ class PathRegistryTest(_fixtures.FixtureTest):
         is_(p1 != p4, True)
         is_(p1 != p5, True)
 
+    def test_eq_non_path(self):
+        umapper = inspect(self.classes.User)
+        amapper = inspect(self.classes.Address)
+        u_alias = inspect(aliased(self.classes.User))
+        p1 = PathRegistry.coerce((umapper,))
+        p2 = PathRegistry.coerce((umapper, umapper.attrs.addresses))
+        p3 = PathRegistry.coerce((u_alias, umapper.attrs.addresses))
+        p4 = PathRegistry.coerce((u_alias, umapper.attrs.addresses, amapper))
+        p5 = PathRegistry.coerce((u_alias,)).token(":*")
+
+        non_object = 54.1432
+
+        for obj in [p1, p2, p3, p4, p5]:
+            with expect_warnings(
+                "Comparison of PathRegistry to "
+                "<.* 'float'> is not supported"
+            ):
+                is_(obj == non_object, False)
+
+            with expect_warnings(
+                "Comparison of PathRegistry to "
+                "<.* 'float'> is not supported"
+            ):
+                is_(obj != non_object, True)
+
     def test_contains_mapper(self):
         umapper = inspect(self.classes.User)
         amapper = inspect(self.classes.Address)