Bug 966665 - Criteria queries with paging and sorting on non unique fields may return same item on different pages
Criteria queries with paging and sorting on non unique fields may return same...
Product: RHQ Project
Classification: Other
Component: REST, CLI, Core Server, Core UI (Show other bugs)
Unspecified Unspecified
high Severity high (vote)
: ---
: RHQ 4.8
Assigned To: Jay Shaughnessy
Mike Foley
Depends On: 966559
Blocks: 958772
  Show dependency treegraph
Reported: 2013-05-23 12:00 EDT by Heiko W. Rupp
Modified: 2013-08-31 06:15 EDT (History)
2 users (show)

See Also:
Fixed In Version:
Doc Type: Bug Fix
Doc Text:
Story Points: ---
Clone Of: 966559
Last Closed: 2013-08-31 06:15:19 EDT
Type: Bug
Regression: ---
Mount Type: ---
Documentation: ---
Verified Versions:
Category: ---
oVirt Team: ---
RHEL 7.3 requirements from Atomic Host:
Cloudforms Team: ---

Attachments (Terms of Use)

  None (edit)
Description Heiko W. Rupp 2013-05-23 12:00:27 EDT
This bug is for the general criteria api 

Bug 966559 is just one specific incarnation that uncovered the issue.

+++ This bug was initially created as a clone of Bug #966559 +++

Description of problem: I have a test, that iterates through REST /resources and checks what is returned by server. This test discovered, that server returns a very same resource on different pages

Version-Release number of selected component (if applicable):
RHQ 4.8-master

How reproducible:always

Steps to Reproduce:
1. run GET on /rest/resource with same page size but iterate over pages

2013-05-23 15:03:54,088 - INFO: Request resource?ps=3&page=0 returned [10021, 10148, 10146] (GetResourceTest)
2013-05-23 15:03:54,591 - INFO: Request resource?ps=3&page=1 returned [10041, 10124, 10126] (GetResourceTest)
2013-05-23 15:03:55,246 - INFO: Request resource?ps=3&page=2 returned [10024, 10097, 10133] (GetResourceTest)
2013-05-23 15:03:55,741 - INFO: Request resource?ps=3&page=3 returned [10132, 10134, 10011] (GetResourceTest)
2013-05-23 15:03:56,264 - INFO: Request resource?ps=3&page=4 returned [10011, 10016, 10168] (GetResourceTest)
2013-05-23 15:03:56,802 - INFO: Request resource?ps=3&page=5 returned [10169, 10023, 10029] (GetResourceTest)
2013-05-23 15:03:57,366 - INFO: Request resource?ps=3&page=6 returned [10025, 10160, 10070] (GetResourceTest)
2013-05-23 15:03:57,920 - INFO: Request resource?ps=3&page=7 returned [10174, 10150, 10107] (GetResourceTest)
2013-05-23 15:03:58,488 - INFO: Request resource?ps=3&page=8 returned [10159, 10110, 10155] (GetResourceTest)
2013-05-23 15:03:59,017 - INFO: Request resource?ps=3&page=9 returned [10155, 10026, 10015] (GetResourceTest)
2013-05-23 15:03:59,549 - INFO: Request resource?ps=3&page=10 returned [10076, 10099, 10130] (GetResourceTest)
2013-05-23 15:04:00,086 - INFO: Request resource?ps=3&page=11 returned [10098, 10115, 10130] (GetResourceTest)
2013-05-23 15:04:00,669 - INFO: Request resource?ps=3&page=12 returned [10089, 10105, 10073] (GetResourceTest)
2013-05-23 15:04:01,166 - INFO: Request resource?ps=3&page=13 returned [10022, 10170, 10013] (GetResourceTest)
2013-05-23 15:04:01,659 - INFO: Request resource?ps=3&page=14 returned [10173, 10154, 10053] (GetResourceTest)
2013-05-23 15:04:02,166 - INFO: Request resource?ps=3&page=15 returned [10056, 10136, 10042] (GetResourceTest)
2013-05-23 15:04:02,674 - INFO: Request resource?ps=3&page=16 returned [10043, 10044, 10045] (GetResourceTest)
2013-05-23 15:04:03,191 - INFO: Request resource?ps=3&page=17 returned [10030, 10116, 10096] (GetResourceTest)
2013-05-23 15:04:03,758 - INFO: Request resource?ps=3&page=18 returned [10111, 10121, 10012] (GetResourceTest)
2013-05-23 15:04:04,381 - INFO: Request resource?ps=3&page=19 returned [10171, 10104, 10147] (GetResourceTest)

Actual results:
resource?ps=3&page=3 returned [10132, 10134, 10011]
resource?ps=3&page=4 returned [10011, 10016, 10168]

this means resource with id 10011 is present on both pages 3 and 4 - which is incorrect

Expected results:

each resource must appear exactly once among all returned pages

Additional info:

--- Additional comment from Heiko W. Rupp on 2013-05-23 10:39:23 EDT ---

I assumed that criteria stuff defaults to sort by id if no other sort is given, which is obviously not right.

--- Additional comment from Heiko W. Rupp on 2013-05-23 11:58:12 EDT ---

Actually the code in question already does/did use a sort by name, so should not return the same resource twice.
Perhaps the name is not enough as there are multiple resources with name 'snert' in my inventory and the db is then returning them in random order

17:03:22] <pilhuhn>	         ResourceCriteria criteria = new ResourceCriteria();
[17:03:22] <pilhuhn>	         criteria.addSortName(PageOrdering.ASC);
[17:03:33] <pilhuhn>	 ...
[17:03:34] <pilhuhn>	 PageList<Resource> ret = resMgr.findResourcesByCriteria(caller,criteria);
[17:04:11] <pilhuhn>	 when I run the results with paging over the various pages, I see duplicates like lzoubek  did in Bug 966559
[17:04:14] <rhq-bot>	 BZ 966559 [product=RHQ, priority=unspecified, status=ON_DEV] Paging returns same resource on different pages [ https://bugzilla.redhat.com/966559 ]
[17:04:31] <pilhuhn>	 If I add a criteria.addSortId(PageOrdering.ASC)  the dupe goes away
[17:06:44] <pilhuhn>	 See https://git.fedorahosted.org/cgit/rhq/rhq.git/tree/modules/enterprise/server/jar/src/main/java/org/rhq/enterprise/server/rest/ResourceHandlerBean.java#n190

[17:17:01] <jshaughn>	 pilhuhn: I think that that is likely the issue
[17:17:21] <jshaughn>	 it's not really something we had considered in the past, but I don't see why that wouldn't be a problem
[17:17:41] <jshaughn>	 the db vendor should not necessarily guarantee any ordering other than name
[17:17:57] <jshaughn>	 and if you have dup names then you could get them ordered in any way
[17:18:05] <jshaughn>	 sort of dumb for us not to really think about that
[17:18:20] <jshaughn>	 I agree sort or name, id for safety
[17:18:34] <jshaughn>	 of course you know what this means...
[17:18:41] <jshaughn>	 a sweep
[17:18:43] <pilhuhn>	 We will get dups "naturally", as all agents are named rhq-agent for example
[17:19:08] <jshaughn>	 +1, paging on a field with dups is not going to be safe, I think
[17:19:42] <jshaughn>	 especially after Lukas's recent changes.
[17:19:51] <pilhuhn>	 For the rest api, it is an easy change for me -- but  I guess CLI clients may run into the same issue. And perhaps even the UI
[17:20:50] <jshaughn>	 we may have been getting lucky, if we were paging on a query with join fetch we would have been actually fetching everything and then getting  consistent ordering wuith the in-mem handling
[17:21:13] <jshaughn>	 absolutely, CLI, GUI, perhaps even SLSB calls
[17:21:22] <jshaughn>	 we need a sweep now...
[17:21:38] <jshaughn>	 looking for our sort specifiers all over the place
[17:21:51] <jshaughn>	 and adding id as a secondary sort if necessary
[17:22:16] <jshaughn>	 either that or we build it into base Criteria handling
[17:22:36] <jshaughn>	 but I'm not sure I like to be that heavy-handed
[17:25:26] <pilhuhn>	 We would perhaps need to make our criteria fields know if they are unique. the Criteria execution could then look them up and if there is a unique one (like id) it would just use that. Otherwise it would automagically add a sortById as fallback
[17:27:23] <pilhuhn>	 biab
[17:28:30] <jshaughn>	 I think perhaps just making a jdoc note on the appropriate addSort methods may be sufficient, and adding another "tip" to the criteria tips and tricks page.
[17:30:42] <jshaughn>	 another possibility is always adding an id sort when paging is specified *unless* there is an override set: addSortId(false)
[17:31:05] <jshaughn>	 that may be smart
[17:57:40] <pilhuhn>	 Yep, that sounds good
Comment 1 Jay Shaughnessy 2013-05-24 19:52:44 EDT
master commit 471b8a20f984f79d448acf58c01ec8c38cee8e67
Author: Jay Shaughnessy <jshaughn@redhat.com>
Date:   Fri May 24 19:45:41 2013 -0400

    All paged criteria queries now apply ID as the least significant
    ordering field (see exceptions below).  Since ID is unique, this ensures
    that queries for pages always return the same ordering of rows.

    ID will not be automatically appended as an ordering field if:
    - the paging is unlimited (i.e. no pagesize, all rows returned)
    - the Criteria class does not support an ID sort
    - the caller has set the Criteria class instance to not support the ID sort
    - ID is already specified as an ordering field
    - the max number of ordering fields has already been set (3)

    Note that CriteriaQuery has had its support for implicit ID sorting removed,
    as the new mechanism, which moves support to the CriteriaQueryGenerator,
    replaces it.
Comment 2 Heiko W. Rupp 2013-06-03 07:18:14 EDT
My original case has been solved by this.

I have removed the sortById from
and the 
org.rhq.modules.integrationTests.restApi.ResourcesTest#testGetResourcesWithPagingAndUniquenessCheck works now as expected,
Comment 3 Heiko W. Rupp 2013-08-31 06:15:19 EDT
Bulk close of old bugs in VERIFIED state.

Note You need to log in before you can comment on or make changes to this bug.