You are viewing a plain text version of this content. The canonical link for it is here.
Posted to oak-issues@jackrabbit.apache.org by "Thomas Mueller (JIRA)" <ji...@apache.org> on 2014/03/04 10:48:33 UTC

[jira] [Comment Edited] (OAK-1263) optimize oak index to support 'fast ORDER BY' queries to support sorting & pagination for large set of children

    [ https://issues.apache.org/jira/browse/OAK-1263?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13919193#comment-13919193 ] 

Thomas Mueller edited comment on OAK-1263 at 3/4/14 9:47 AM:
-------------------------------------------------------------

Please note my comments are all about formatting, naming conventions, and so on. The patch itself looks good!

But still, some more comments:

{noformat}
import static org.apache.jackrabbit.oak.plugins.index.property.OrderedIndex.*;
{noformat}

I personally don't like static imports (but this is a matter of taste), but I think we agreed we don't use star imports.

OrderedPropertyIndexEditor and OrderedPropertyIndexEditorProvider: the license header is missing.

OrderedPropertyIndexEditorProvider, variable "_editor": we don't use the underline prefix (for anything). Just rename it to "editor". The same for other variables, for example "NodeBuilder _key".

{noformat}
   /* (non-Javadoc)
    * @see org.apache.jackrabbit.oak.spi.query.QueryIndexProvider#getQueryIndexes(org.apache.jackrabbit.oak.spi.state.NodeState)
    */
{noformat}

I don't see a value in that comment. Could you remove it please?

Could you add class level javadoc comments (for example for for OrderedContentMirrorStoreStrategy)?

{noformat}
    public void firstAndOnlyItem() {
        final String PATH = "/foo/bar";
{noformat}

The naming convention for variables is to use lowercase. Could you rename the variable to "path" please?

{noformat}
        assertEquals("Expecting 2 items in the index", 2, Iterators.size(children.iterator())); // how
                                                                                                // many
                                                                                                // entries
                                                                                                // do
                                                                                                // we
                                                                                                // have?
{noformat}

Could you remove, or move the line comment to _before_ the line please? For example:

{noformat}
        // how many entries do we have?
        assertEquals("Expecting 2 items in the index", 2, Iterators.size(children.iterator())); 
{noformat}



was (Author: tmueller):
Please note my comments are all about formatting, naming conventions, and so on. The patch itself looks good!

But still, some more comments:

{noformat}
import static org.apache.jackrabbit.oak.plugins.index.property.OrderedIndex.*;
{noformat}

I personally don't like static imports (but this is a matter of taste), but I think we agreed we don't use star imports.

OrderedPropertyIndexEditor and OrderedPropertyIndexEditorProvider: the license header is missing.

OrderedPropertyIndexEditorProvider, variable "_editor": we don't use the underline prefix (for anything). Just rename it to "editor". The same for other variables, for example "NodeBuilder _key".

{noformat}
   /* (non-Javadoc)
    * @see org.apache.jackrabbit.oak.spi.query.QueryIndexProvider#getQueryIndexes(org.apache.jackrabbit.oak.spi.state.NodeState)
    */
{noformat}

I don't see a value in that comment. Could you remove it please?

Could you add class level javadoc comments (for example for for OrderedContentMirrorStoreStrategy)?

{noformat}
public static final String NEXT = ":next";
{noformat}

This is the wrong order, could you use:
{noformat}
public final static String NEXT = ":next";
{noformat}

{noformat}
    public void firstAndOnlyItem() {
        final String PATH = "/foo/bar";
{noformat}

The naming convention for variables is to use lowercase. Could you rename the variable to "path" please?

{noformat}
        assertEquals("Expecting 2 items in the index", 2, Iterators.size(children.iterator())); // how
                                                                                                // many
                                                                                                // entries
                                                                                                // do
                                                                                                // we
                                                                                                // have?
{noformat}

Could you remove, or move the line comment to _before_ the line please? For example:

{noformat}
        // how many entries do we have?
        assertEquals("Expecting 2 items in the index", 2, Iterators.size(children.iterator())); 
{noformat}


> optimize oak index to support 'fast ORDER BY' queries to support sorting & pagination for large set of children
> ---------------------------------------------------------------------------------------------------------------
>
>                 Key: OAK-1263
>                 URL: https://issues.apache.org/jira/browse/OAK-1263
>             Project: Jackrabbit Oak
>          Issue Type: Improvement
>          Components: query
>    Affects Versions: 0.12
>            Reporter: Stefan Egli
>            Assignee: Alex Parvulescu
>             Fix For: 0.18
>
>         Attachments: OAK-1263-r1.patch, OAK-1263-r1a.patch, benchmark-20140228112150.log, benchmark-20140228120718.log, benchmark-20140228125248.log
>
>
> We have a use case where we'd like to be able to use an index in a "pagination-like" fashion. That is, we'd like to be able to have an index on a subtree on a certain property, and then run a query which does ORDER BY that property combined with OFFSET. That way, essentially allowing pagination of child nodes of a particular parent based on 'sorted by a certain property'.
> AFAIU currently the oak index is not optimized to support ORDER BY queries in a fast manner. The index keeps 'the child nodes unsorted', ie to process an ORDER BY, the child nodes would have to be 'manually sorted' which can result in bad performance given a large number of child nodes.



--
This message was sent by Atlassian JIRA
(v6.2#6252)