[PATCH 2/2] xmlrpc: Treat negative max_count as meaning "return last N results"

[Finucane, Stephen]

On 31 Mar 15:08, Damien Lespiau wrote:
> On Fri, Feb 05, 2016 at 05:21:20PM +0000, Stephen Finucane wrote:
> > @@ -602,6 +607,8 @@ def patch_list(filt=None):
> >  
> >          if max_count > 0:
> >              return list(map(patch_to_dict, patches[:max_count]))
> > +        elif max_count < 0:
> > +            return list(map(patch_to_dict, patches[len(patches)+max_count:]))
> 
> This will do something you probably don't want to happen:
> 
>   - len(patches) will actually retrieve all patches in memory and return
>     the length from that data,
> 
>   - the slicing operation will then happen without querying the DB and
>     slice the results cached from the len() evaluation.
> 
> You can use 2 queries to not have to load all patches:
> 
>   patches[patches.count() + max_count:]
> 
> Alternatively, it's possible to use reverse() to not have to depend on
> querying the number of items beforehand (which is what I did in my
> version of the patch) and reverse the list once again once we retrieved
> the elements. Given that we already go through the list of patches to
> map each patch to a dict, iterating in reverse order doesn't add extra
> cost. Something only those lines maybe?
> 
>   query = patches.reverse()[:-max_count]
>   return [patch_to_dict(patch) for patch in reversed(query)]

Yup, this is an oversight. If you (or anyone else) would like to submit
a patch then please do so. I've added this to the bug tracker [1].

Stephen

[1] https://github.com/getpatchwork/patchwork/issues/35