Red Hat Bugzilla – Bug 966665
Criteria queries with paging and sorting on non unique fields may return same item on different pages
Last modified: 2013-08-31 06:15:19 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):
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)
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
each resource must appear exactly once among all returned pages
--- 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
master commit 471b8a20f984f79d448acf58c01ec8c38cee8e67
Author: Jay Shaughnessy <firstname.lastname@example.org>
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,
My original case has been solved by this.
I have removed the sortById from
org.rhq.modules.integrationTests.restApi.ResourcesTest#testGetResourcesWithPagingAndUniquenessCheck works now as expected,
Bulk close of old bugs in VERIFIED state.