From e9abaaef9f682941aa424db73e408de3ec5462da Mon Sep 17 00:00:00 2001 From: Mike Bayer Date: Thu, 29 Nov 2007 19:37:05 +0000 Subject: [PATCH] - adjustment to the previous checkin regarding inheritance to not conflict with globals - fix to self-referential eager loading such that if the same mapped instance appears in two or more distinct sets of columns in the same result set, its eagerly loaded collection will be populated regardless of whether or not all of the rows contain a set of "eager" columns for that collection. this would also show up as a KeyError when fetching results with join_depth turned on. --- CHANGES | 7 ++++++ lib/sqlalchemy/orm/query.py | 2 +- lib/sqlalchemy/orm/strategies.py | 15 ++++++++---- test/orm/eager_relations.py | 42 ++++++++++++++++++++++++++++++++ 4 files changed, 60 insertions(+), 6 deletions(-) diff --git a/CHANGES b/CHANGES index 94368253cd..dded89564d 100644 --- a/CHANGES +++ b/CHANGES @@ -32,6 +32,13 @@ CHANGES - fixed endless loop issue when using lazy="dynamic" on both sides of a bi-directional relationship [ticket:872] + - fix to self-referential eager loading such that if the same mapped + instance appears in two or more distinct sets of columns in the same + result set, its eagerly loaded collection will be populated regardless + of whether or not all of the rows contain a set of "eager" columns for + that collection. this would also show up as a KeyError when fetching + results with join_depth turned on. + - fixed bug where Query would not apply a subquery to the SQL when LIMIT was used in conjunction with an inheriting mapper where the eager loader was only in the parent mapper. diff --git a/lib/sqlalchemy/orm/query.py b/lib/sqlalchemy/orm/query.py index e6dc0a9119..054ac04e74 100644 --- a/lib/sqlalchemy/orm/query.py +++ b/lib/sqlalchemy/orm/query.py @@ -46,7 +46,7 @@ class Query(object): self._populate_existing = False self._version_check = False self._autoflush = True - self._eager_loaders = util.Set(chain(*[mapper._eager_loaders for mapper in [m for m in self.mapper.iterate_to_root()]])) + self._eager_loaders = util.Set(chain(*[mp._eager_loaders for mp in [m for m in self.mapper.iterate_to_root()]])) self._attributes = {} self._current_path = () self._primary_adapter=None diff --git a/lib/sqlalchemy/orm/strategies.py b/lib/sqlalchemy/orm/strategies.py index a5f65006df..096a42bb71 100644 --- a/lib/sqlalchemy/orm/strategies.py +++ b/lib/sqlalchemy/orm/strategies.py @@ -591,7 +591,12 @@ class EagerLoader(AbstractRelationLoader): # so that we further descend into properties self.select_mapper._instance(selectcontext, decorated_row, None) else: - if isnew: + appender_key = ('appender', id(instance), self.key) + if isnew or appender_key not in selectcontext.attributes: + # appender_key can be absent from selectcontext.attributes with isnew=False + # when self-referential eager loading is used; the same instance may be present + # in two distinct sets of result columns + if self._should_log_debug: self.logger.debug("initialize UniqueAppender on %s" % mapperutil.attribute_str(instance, self.key)) @@ -599,13 +604,13 @@ class EagerLoader(AbstractRelationLoader): appender = util.UniqueAppender(collection, 'append_without_event') # store it in the "scratch" area, which is local to this load operation. - selectcontext.attributes[('appender', id(instance), self.key)] = appender - result_list = selectcontext.attributes[('appender', id(instance), self.key)] + selectcontext.attributes[appender_key] = appender + + result_list = selectcontext.attributes[appender_key] if self._should_log_debug: self.logger.debug("eagerload list instance on %s" % mapperutil.attribute_str(instance, self.key)) - - self.select_mapper._instance(selectcontext, decorated_row, result_list) + self.select_mapper._instance(selectcontext, decorated_row, result_list) if self._should_log_debug: self.logger.debug("Returning eager instance loader for %s" % str(self)) diff --git a/test/orm/eager_relations.py b/test/orm/eager_relations.py index 076f44005a..acb5e0da73 100644 --- a/test/orm/eager_relations.py +++ b/test/orm/eager_relations.py @@ -582,6 +582,48 @@ class SelfReferentialEagerTest(ORMTest): ]) == d self.assert_sql_count(testbase.db, go, 1) + + def test_lazy_fallback_doesnt_affect_eager(self): + class Node(Base): + def append(self, node): + self.children.append(node) + + mapper(Node, nodes, properties={ + 'children':relation(Node, lazy=False, join_depth=1) + }) + sess = create_session() + n1 = Node(data='n1') + n1.append(Node(data='n11')) + n1.append(Node(data='n12')) + n1.append(Node(data='n13')) + n1.children[1].append(Node(data='n121')) + n1.children[1].append(Node(data='n122')) + n1.children[1].append(Node(data='n123')) + sess.save(n1) + sess.flush() + sess.clear() + + # eager load with join depth 1. when eager load of 'n1' + # hits the children of 'n12', no columns are present, eager loader + # degrades to lazy loader; fine. but then, 'n12' is *also* in the + # first level of columns since we're loading the whole table. + # when those rows arrive, now we *can* eager load its children and an + # eager collection should be initialized. essentially the 'n12' instance + # is present in not just two different rows but two distinct sets of columns + # in this result set. + def go(): + allnodes = sess.query(Node).order_by(Node.data).all() + n12 = allnodes[2] + assert n12.data == 'n12' + print "N12 IS", id(n12) + print [c.data for c in n12.children] + assert [ + Node(data='n121'), + Node(data='n122'), + Node(data='n123') + ] == list(n12.children) + self.assert_sql_count(testbase.db, go, 1) + def test_with_deferred(self): class Node(Base): def append(self, node): -- 2.47.2