Code review comment for lp:~bac/launchpad/bug-5927

Revision history for this message
Robert Collins (lifeless) wrote :

Following up on recent search performance, I noticed a couple of odd things introduced in this branch. Firstly, the specialised BugTaskResultSet was unnecessary - the addition of assignee to the permission lookups on BugTask searching has an immediate effect of precaching assignee *access* to the bug; none of our bug views show the assignee at the moment, so precaching actual assignee objects is a pessimism. As such I'm rolling that part of the patch backout.

The other odd thing was that I saw the initial patch *had* a pre_iter_hook parameter, which is actually more general and more compact than creating a class : as we get more complex query interactions being able to to compose and reuse eager loading requirements will become more important: I don't think we want a proliferation of result set /types/ to achieve that: simple functions are going to be generally more powerful.

« Back to merge proposal