[rset] Always complete attribute/relation caches in ResultSet.get_entity
RQL queries are often designed to fill up the ORM's caches when fetching
entities out of the result set. Until now, if an entry already existed
in the entity cache, ResultSet.get_entity would return it unchanged,
not using the new ResultSet's contents to update the attribute cache,
breaking expectations (if the attributes are needed, they'd then be
fetched later one at a time, one entity at a time), resulting in loads
of DB accesses.
So we change ResultSet.get_entity so that:
* if the entity is already cached and has been instantiated from the
same rset, it is returned as-is (to avoid loops)
* if the entity is not yet cached, it is instantiated
* if the entity is cached via another rset, its attribute/relation
caches are completed
Closes #9942503
--- a/cubicweb/rset.py Tue Jan 12 17:36:28 2016 +0100
+++ b/cubicweb/rset.py Tue Jan 12 14:31:14 2016 +0100
@@ -489,13 +489,13 @@
rowvalues = self.rows[row]
eid = rowvalues[col]
assert eid is not None
- # return cached entity if exists. This also avoids potential recursion
- # XXX should we consider updating a cached entity with possible
- # new attributes found in this resultset ?
try:
entity = req.entity_cache(eid)
+ if entity.cw_rset is self:
+ # return entity as is, avoiding recursion
+ return entity
except KeyError:
- pass
+ entity = self._make_entity(row, col)
else:
if entity.cw_rset is None:
# entity has no rset set, this means entity has been created by
@@ -504,8 +504,6 @@
entity.cw_rset = self
entity.cw_row = row
entity.cw_col = col
- return entity
- entity = self._make_entity(row, col)
# try to complete the entity if there are some additional columns
if len(rowvalues) > 1:
eschema = entity.e_schema
--- a/cubicweb/test/unittest_rset.py Tue Jan 12 17:36:28 2016 +0100
+++ b/cubicweb/test/unittest_rset.py Tue Jan 12 14:31:14 2016 +0100
@@ -307,6 +307,11 @@
self.assertEqual(e.cw_col, 0)
self.assertEqual(e.cw_attr_cache['title'], 'zou')
self.assertRaises(KeyError, e.cw_attr_cache.__getitem__, 'path')
+ other_rset = req.execute('Any X, P WHERE X is Bookmark, X path P')
+ # check that get_entity fetches e from the request's cache, and
+ # updates it with attributes from the new rset
+ self.assertIs(other_rset.get_entity(0, 0), e)
+ self.assertIn('path', e.cw_attr_cache)
self.assertEqual(e.view('text'), 'zou')
self.assertEqual(pprelcachedict(e._cw_related_cache), [])