[ 
https://issues.apache.org/jira/browse/JCR-1064?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel#action_12522442
 ] 

Christoph Kiehl commented on JCR-1064:
--------------------------------------

I like the patch so far. Just a few comments:

LuceneQueryBuilder: 
- I would rename getMatchAllQuery() to createMatchAllQuery() to make clear that 
a new instance is created
- I think it's not a valid option to imply the new index format in the old 
createQuery() variant. I would rather imply the old index format since this 
will works with the new index as well (until now). But the optimal solution 
would be to remove this methods. Let's wait what Marcel says.

SearchIndex:
- You seem to have called "Organize Imports". If you could adjust the import 
order to Jackrabbits order this would make the diff smaller
- I would rewrite the format check to:

        // The index is in the new format if either the index already contains
        // the field FieldNames.PROPERTIES_SET in any document or if the index
        // is empty
        Collection allFieldNames = index.getIndexReader().getFieldNames(
                             FieldOption.ALL);
        newIndexFormat = allFieldNames.contains(FieldNames.PROPERTIES_SET)
                         || index.getIndexReader().numDocs() == 0;

        if (!newIndexFormat) {
            log.warn("Index is in old format. This might imply slower queries."
                    + "Re-index if possible");
        }

My line was actually just a quick example. I think it's more readable if it is 
split into two lines at least. The other point is that I prefer to always 
enclose if-blocks with curly braces. This is less error prone when adding new 
statements to the block.

Overall you should take care of using spaces instead of tabs everywhere. In 
case you use eclipse just edit your formatter preferences for that particular 
project and do <Ctrl>+I on the sections in question. This will re-indent those 
sections.

Sounds like a lot of critics but I really like and appreciate your work!

> Optimize queries that check for the existence of a property
> -----------------------------------------------------------
>
>                 Key: JCR-1064
>                 URL: https://issues.apache.org/jira/browse/JCR-1064
>             Project: Jackrabbit
>          Issue Type: Improvement
>          Components: indexing
>    Affects Versions: 1.3.1
>            Reporter: Ard Schrijvers
>            Priority: Minor
>             Fix For: 1.4
>
>         Attachments: JCR-1064.patch
>
>
> //[EMAIL PROTECTED] is transformed into the 
> org.apache.jackrabbit.core.query.lucene.MatchAllQuery, that through the 
> MatchAllWeight uses the MatchAllScorer.  The calculateDocFilter() in 
> MatchAllScorer  does not scale and becomes slow for growing number of nodes. 
> Solution: lucene documents will get a new Field:
> public static final String PROPERTIES_SET = "_:PROPERTIES_SET".intern();
> that holds the available properties of this document. 
> NOTE: Lucene indices build without this performance improvement should still 
> work and fall back to the original implementation

-- 
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.

Reply via email to