Paginator object doesn't move onto next "page" correctly

Details

Type: Bug
Status: Resolved
Priority: Blocker
Resolution: Fixed
Affects Version/s: 1.1.2
Fix Version/s: 1.1.6
Component/s: library
Security Level: Public
Labels:
None

Description

Customer found his own issue:
I had a look at the source code for the java driver and found the error there. It is in:

com.couchbase.client.protocol.views.Paginator.getNextPage()

Right after calculating the number of "remaining" keys, it sets the limit of the query to the remaining number of keys and re-runs the query. What is missing here is:

q.setSkip(totalDocs);

To actually start from the entry after the last entry on the current page. I have tried this and it works. But there is more code in this method and it is not very easy to follow so important to make sure that this fits.

I am enclosing a diff.

Options
- Sort By Name
- Sort By Date
- Ascending
- Descending
- Download All

Attachments

paginator-java.diff

07/Feb/13 4:02 AM

0.0 kB

Perry Krug

Activity

Ascending order - Click to sort in descending order

Perry Krug made changes - 07/Feb/13 4:04 AM

Field	Original Value	New Value
Description	Customer found his own issue: I had a look at the source code for the java driver and found the error there. It is in: com.couchbase.client.protocol.views.Paginator.getNextPage() Right after calculating the number of "remaining" keys, it sets the limit of the query to the remaining number of keys and re-runs the query. What is missing here is: q.setSkip(totalDocs); To actually start from the entry after the last entry on the current page. I have tried this and it works. But there is more code in this method and it is not very easy to follow so important to make sure that this fits. Got response containing: 50 Done iterating through page, #missing keys: 51 Got response containing: 50 Done iterating through page, #missing keys: 1 Got response containing: 1 Done iterating through page, #missing keys: 0 I am enclosing a diff.	Customer found his own issue: I had a look at the source code for the java driver and found the error there. It is in: com.couchbase.client.protocol.views.Paginator.getNextPage() Right after calculating the number of "remaining" keys, it sets the limit of the query to the remaining number of keys and re-runs the query. What is missing here is: q.setSkip(totalDocs); To actually start from the entry after the last entry on the current page. I have tried this and it works. But there is more code in this method and it is not very easy to follow so important to make sure that this fits. I am enclosing a diff.

Hide

Permalink

Perry Krug added a comment - 07/Feb/13 11:47 AM

Update, but the bug still stands as described.

It seems that my fix is not the correct one. The bug seem to be that although the Paginator specifies setStartkeyDocID, the query doesn't start from this key.

I have debugged the Paginator class – and when resolving the next page it instructs the query to start from a specific doc id, e.g:

Query: ?limit=50&startkey=144&skip=0&stale=false&startkey_docid=144

But as a result of this query I get:

Got key: 1
Got key: 10
Got key: 100
Got key: 101
Got key: 102
Got key: 103
…
Got key: 141
Got key: 142
Got key: 143

Every time. So the bug is in the Query context somewhere.

Also – a note of inefficiency in the Paginator class: why perform two queries every time? One for the content of the "page" and one to find the row for the next page? Would be better to query for page + 1 and keep the last separate from the ViewResponse.

So, what I can conclude is that when the Paginator bug is resolved it should work fine for us. The additional query for the last row could be an issue though, since it uses setSkip(). So could this be treated as a bug as well?

Show

Perry Krug added a comment - 07/Feb/13 11:47 AM Update, but the bug still stands as described. It seems that my fix is not the correct one. The bug seem to be that although the Paginator specifies setStartkeyDocID, the query doesn't start from this key. I have debugged the Paginator class – and when resolving the next page it instructs the query to start from a specific doc id, e.g: Query: ?limit=50&startkey=144&skip=0&stale=false&startkey_docid=144 But as a result of this query I get: Got key: 1 Got key: 10 Got key: 100 Got key: 101 Got key: 102 Got key: 103 … Got key: 141 Got key: 142 Got key: 143 Every time. So the bug is in the Query context somewhere. Also – a note of inefficiency in the Paginator class: why perform two queries every time? One for the content of the "page" and one to find the row for the next page? Would be better to query for page + 1 and keep the last separate from the ViewResponse. So, what I can conclude is that when the Paginator bug is resolved it should work fine for us. The additional query for the last row could be an issue though, since it uses setSkip(). So could this be treated as a bug as well?

Perry Krug made changes - 13/Feb/13 12:17 PM

Link

This issue depends on CBSE-394 [ CBSE-394 ]

Michael Nitschinger made changes - 19/Feb/13 3:16 AM