Review Request 125544: Fix limit/offset handling when sort option is no sort.

[Xuetian Weng]

> On Oct. 9, 2015, 11:21 a.m., Vishesh Handa wrote:
> > src/lib/searchstore.cpp, line 103
> > <https://git.reviewboard.kde.org/r/125544/diff/1/?file=410212#file410212line103>
> >
> >     Say we have 50 results
> >     
> >     limit = 10
> >     offset = 5
> >     
> >     So we want results from [5, 14]
> >     
> >     With this code -
> >     for (i = 5; i < 10; i++)
> >     
> >     We we would get [5, 10). Could you please fix this? I think a simple `i < limit + offset` would do.

I just want to confirm, do we want [offset, offset + limit), or [offset, limit), otherwise the semantics of sort looks problematic.

Currently the sort is always applied to [0, limit), if we want [offset, offset + limit), shouldn't we always sort the whole set, and then slice [offset, offset + limit)?


- Xuetian


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/125544/#review86535
-----------------------------------------------------------


On Oct. 9, 2015, 11:21 a.m., Xuetian Weng wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/125544/
> -----------------------------------------------------------
> 
> (Updated Oct. 9, 2015, 11:21 a.m.)
> 
> 
> Review request for Baloo, Release Team and Vishesh Handa.
> 
> 
> Repository: baloo
> 
> 
> Description
> -------
> 
> The bug is obvious, old code uses limit instead of (limit > 0), which gracefully handles limit < 0 case (means everything).
> 
> After offset is introduced, limit < 0 case is not handled properly.
> 
> This patch contains following changes:
> 1. make offset uint, m_offset is uint in Query already.
> 2. handle limit < 0 case properly.
> 
> 
> Diffs
> -----
> 
>   src/lib/searchstore.h 17b4263 
>   src/lib/searchstore.cpp a2e7749 
> 
> Diff: https://git.reviewboard.kde.org/r/125544/diff/
> 
> 
> Testing
> -------
> 
> baloosearch:/ doesn't return anything because of this, now it properly returns result.
> 
> 
> Thanks,
> 
> Xuetian Weng
> 
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/release-team/attachments/20151009/481b93eb/attachment-0001.html>