[MAGNOLIA-3331] Provide for maxResultSize in Query-API Created: 21/Oct/10 Updated: 20/Feb/15 Resolved: 01/Dec/10 |
|
| Status: | Closed |
| Project: | Magnolia |
| Component/s: | core |
| Affects Version/s: | None |
| Fix Version/s: | 4.4 |
| Type: | Improvement | Priority: | Major |
| Reporter: | Joerg von Frantzius | Assignee: | Philipp Bärfuss |
| Resolution: | Fixed | Votes: | 0 |
| Labels: | None | ||
| Remaining Estimate: | Not Specified | ||
| Time Spent: | Not Specified | ||
| Original Estimate: | Not Specified | ||
| Attachments: |
|
||||||||
| Issue Links: |
|
||||||||
| Template: |
|
||||||||
| Patch included: |
Yes
|
||||||||
| Acceptance criteria: |
Empty
|
||||||||
| Task DoD: |
[ ]*
Doc/release notes changes? Comment present?
[ ]*
Downstream builds green?
[ ]*
Solution information and context easily available?
[ ]*
Tests
[ ]*
FixVersion filled and not yet released
[ ] 
Architecture Decision Record (ADR)
|
||||||||
| Testcase included: |
Yes
|
||||||||
| Date of First Response: | |||||||||
| Description |
|
Currently when executing a query through the Magnolia query API, there is no way of preventing the result set from being iterated completely. When e.g. only the first couple of objects from a large result set are of interest, then thousands of objects are instantiated unnecessarily, resulting in a huge performance penalty. These problems have been addressed in the openmind Magnolia Criteria API, but this cannot be used e.g. for the STK implementation. So in order to optimize e.g. the STK, the core query API must be extended. |
| Comments |
| Comment by Joerg von Frantzius [ 21/Oct/10 ] |
|
The provided patch applies to the 4.4 branch http://svn.magnolia-cms.com/svn/community/magnolia/branches/magnolia-4.4 |
| Comment by Joerg von Frantzius [ 21/Oct/10 ] |
|
now checkstyle compliant |
| Comment by Fabrizio Giustina [ 21/Oct/10 ] |
|
> These problems have been addressed in the openmind Magnolia Criteria API, but this cannot be used e.g. for the STK implementation. why not? there is nothing that prevents criteria APIs to be used with STK... the magnolia forum for example is build with STK and criteria. |
| Comment by Philipp Bärfuss [ 21/Oct/10 ] |
|
Thanks for the patch. I preferred if the QueryResult.getContent() would return a lazy collection. Doing so the benefit would be much bigger than by adding a new method. By adding a new method it is probably hard to benefit from the improvement in STK without rewriting tons of models. But then I just wonder how we could handle collection.size(). Either this causes then a full read operation or it returns the value of the JCR QueryResult, which might not be correct due to permission restrictions (the query could for instance return 100 items but the current user can only read 1). WDYT FYI: In 5.0 we will start to use the JCR API and iterators to make every thing more performant. |
| Comment by Joerg von Frantzius [ 21/Oct/10 ] |
|
A lazy collection seems like a nice idea indeed. But AFAIK, Jackrabbit does some crazy default ordering of nodes in Java when no "ORDER BY" had been provided, already iterating over all result nodes before Magnolia does. I also don't see a solution to the problem of size() still doing full iteration without breaking API contract. What I had in mind originally wasn't a general performance boost for anything that uses query API to retrieve only a couple of objects. Our particular usecase that required me to subclass QueryResultImpl was to display a latest news teaser showing the first handful out of thousands of articles ordered by date. I was mostly thinking of certain subclasses of AbstractListItemModel to perform sorting, filtering and shrinking via JCR query rather than in Java. These classes will have to be touched anyway in order to be able to work on thousands of nodes, next to some STKUtil method signatures needing additional parameters for an AND-clause and an ORDER-BY-clause. Of course the original method signatures would be retained. |
| Comment by Joerg von Frantzius [ 26/Oct/10 ] |
|
Result from discussing with Philipp:
|
| Comment by Joerg von Frantzius [ 27/Oct/10 ] |
|
Query.setLimit(long) instead of Query.execute(int) |
| Comment by Joerg von Frantzius [ 27/Oct/10 ] |
|
also test QueryUtil |