From 127ead7452f326509cde38fcf7c9f38f69d9ae0a Mon Sep 17 00:00:00 2001
From: Mike Bayer <mike_mp@zzzcomputing.com>
Date: Thu, 4 Jan 2018 16:58:55 -0500
Subject: [PATCH] Don't try to iterate chopped path if it's None

Fixed regression caused by new lazyload caching scheme in :ticket:`3954`
where a query that makes use of loader options with of_type would cause
lazy loads of unrelated paths to fail with a TypeError.

Change-Id: I705ea0ac012bcc3856ca04109454952cb07a2a8b
Fixes: #4153
---
 doc/build/changelog/unreleased_12/4153.rst |  7 ++
 lib/sqlalchemy/orm/strategy_options.py     | 12 ++-
 test/orm/test_options.py                   | 92 ++++++++++++++++++++++
 3 files changed, 110 insertions(+), 1 deletion(-)
 create mode 100644 doc/build/changelog/unreleased_12/4153.rst

diff --git a/doc/build/changelog/unreleased_12/4153.rst b/doc/build/changelog/unreleased_12/4153.rst
new file mode 100644
index 0000000000..b3b0936689
--- /dev/null
+++ b/doc/build/changelog/unreleased_12/4153.rst
@@ -0,0 +1,7 @@
+.. change::
+    :tags: bug, orm
+    :tickets: 4153
+
+    Fixed regression caused by new lazyload caching scheme in :ticket:`3954`
+    where a query that makes use of loader options with of_type would cause
+    lazy loads of unrelated paths to fail with a TypeError.
diff --git a/lib/sqlalchemy/orm/strategy_options.py b/lib/sqlalchemy/orm/strategy_options.py
index 775ed6c97d..13c5100475 100644
--- a/lib/sqlalchemy/orm/strategy_options.py
+++ b/lib/sqlalchemy/orm/strategy_options.py
@@ -85,7 +85,17 @@ class Load(Generative, MapperOption):
 
             endpoint = obj._of_type or obj.path.path[-1]
             chopped = self._chop_path(loader_path, path)
-            if not chopped and not obj._of_type:
+
+            if (
+                # means loader_path and path are unrelated,
+                # this does not need to be part of a cache key
+                chopped is None
+            ) or (
+                # means no additional path with loader_path + path
+                # and the endpoint isn't using of_type so isn't modified into
+                # an alias or other unsafe entity
+                not chopped and not obj._of_type
+            ):
                 continue
 
             serialized_path = []
diff --git a/test/orm/test_options.py b/test/orm/test_options.py
index 44ebe81c37..731aee1e55 100644
--- a/test/orm/test_options.py
+++ b/test/orm/test_options.py
@@ -1093,6 +1093,98 @@ class CacheKeyTest(PathTest, QueryTest):
             None
         )
 
+    def test_unbound_cache_key_excluded_of_type_safe(self):
+        User, Address, Order, Item, SubItem = self.classes(
+            'User', 'Address', 'Order', 'Item', 'SubItem')
+        # query of:
+        #
+        # query(User).options(
+        #       subqueryload(User.orders).
+        #       subqueryload(Order.items.of_type(SubItem)))
+        #
+        #
+        # we are lazy loading Address objects from User.addresses
+        # the path excludes our option so cache key should
+        # be None
+
+        query_path = self._make_path_registry([User, "addresses"])
+
+        opt = subqueryload(User.orders).\
+            subqueryload(Order.items.of_type(SubItem))
+        eq_(
+            opt._generate_cache_key(query_path),
+            None
+        )
+
+    def test_unbound_cache_key_excluded_of_type_unsafe(self):
+        User, Address, Order, Item, SubItem = self.classes(
+            'User', 'Address', 'Order', 'Item', 'SubItem')
+        # query of:
+        #
+        # query(User).options(
+        #       subqueryload(User.orders).
+        #       subqueryload(Order.items.of_type(aliased(SubItem))))
+        #
+        #
+        # we are lazy loading Address objects from User.addresses
+        # the path excludes our option so cache key should
+        # be None
+
+        query_path = self._make_path_registry([User, "addresses"])
+
+        opt = subqueryload(User.orders).\
+            subqueryload(Order.items.of_type(aliased(SubItem)))
+        eq_(
+            opt._generate_cache_key(query_path),
+            None
+        )
+
+    def test_bound_cache_key_excluded_of_type_safe(self):
+        User, Address, Order, Item, SubItem = self.classes(
+            'User', 'Address', 'Order', 'Item', 'SubItem')
+        # query of:
+        #
+        # query(User).options(
+        #       subqueryload(User.orders).
+        #       subqueryload(Order.items.of_type(SubItem)))
+        #
+        #
+        # we are lazy loading Address objects from User.addresses
+        # the path excludes our option so cache key should
+        # be None
+
+        query_path = self._make_path_registry([User, "addresses"])
+
+        opt = Load(User).subqueryload(User.orders).\
+            subqueryload(Order.items.of_type(SubItem))
+        eq_(
+            opt._generate_cache_key(query_path),
+            None
+        )
+
+    def test_bound_cache_key_excluded_of_type_unsafe(self):
+        User, Address, Order, Item, SubItem = self.classes(
+            'User', 'Address', 'Order', 'Item', 'SubItem')
+        # query of:
+        #
+        # query(User).options(
+        #       subqueryload(User.orders).
+        #       subqueryload(Order.items.of_type(aliased(SubItem))))
+        #
+        #
+        # we are lazy loading Address objects from User.addresses
+        # the path excludes our option so cache key should
+        # be None
+
+        query_path = self._make_path_registry([User, "addresses"])
+
+        opt = Load(User).subqueryload(User.orders).\
+            subqueryload(Order.items.of_type(aliased(SubItem)))
+        eq_(
+            opt._generate_cache_key(query_path),
+            None
+        )
+
     def test_unbound_cache_key_included_of_type_safe(self):
         User, Address, Order, Item, SubItem = self.classes(
             'User', 'Address', 'Order', 'Item', 'SubItem')
-- 
2.47.3