JPAProducer: skiptoken implementation is unsafe in the absence of an orderby expression

[GoogleCodeExporter]

What steps will reproduce the problem?

1. Execute a paged (i.e. skiptoken) query without an orderby clause, on a data 
set which has multiple pages of data.

2. In some way modify the query plan chosen by the backend database, such that 
the ordering of the result set changes between individual pages of the data 
(note; the data itself doesn't have to change, just the order that the rows 
return - this could happen if another process is updating different data in the 
same table, such that the RDBMS decides to use a different query plan for the 
2nd or 3rd page).

3. Observe that you may get duplicate, missing, or otherwise strange amounts of 
data in response.

What is the expected output? What do you see instead?
I'd expect to see the same data, every time. Instead, I can see duplicate or 
missing data as I page through the set. 

What version of the product are you using? On what operating system?
0.7. OS is irrelevant. 

Please provide any additional information below.
The skiptoken implementation relies on a stable ordering; e.g. in an orderby 
clause over (a,b), it relies on a-value, b-value being in order in the result 
set. However, it _also_ relies on the key-value being in order, as in the event 
that the last item in the page has the same a,b value as the next, a condition 
is enforced that the key must differ. 

This is fine, except that the GenerateJPQLCommand doesn't enforce an ordering 
over the keys. It is _never_ safe to assume that a database engine will return 
rows in any order in the absense of an ORDER BY clause in SQL - see 
http://blogs.msdn.com/b/conor_cunningham_msft/archive/2008/08/27/no-seatbelt-exp
ecting-order-without-order-by.aspx for example.

The fix here is relatively straightforward - when generating a query, we always 
have a final order over the primary key of the entity. This preserves any 
user-defined orderby clause, while ensuring the safety of the skiptoken. 

I've attached:

1. A patch which brings GenerateJPQLCommand into shape
2. A query which demonstrates the unsafe ordering which Hibernate uses in the 
absence of an order by clause
3. A query which demonstrates the safe ordering used by Hibernate, with this 
patch applied to odata4j. 

Cheers,

steve

Original issue reported on code.google.com by st...@ethx.net on 9 Aug 2013 at 9:48

Attachments:

• fix_skiptoken.patch
• unsafe.sql
• safe.sql

[GoogleCodeExporter]

Thanks Steve.  Can you upload your patch to our review site for integration?

See the development page [1] for details.

[1] https://code.google.com/p/odata4j/wiki/Development

Original comment by john.spurlock on 9 Aug 2013 at 1:31

[GoogleCodeExporter]

Done - apologies for the delay. http://review.odata4j.org/#/c/14/

Original comment by st...@ethx.net on 15 Aug 2013 at 11:15