You are viewing a plain text version of this content. The canonical link for it is here.
Posted to oak-commits@jackrabbit.apache.org by ch...@apache.org on 2014/10/14 12:03:02 UTC

svn commit: r1631704 - in /jackrabbit/oak/trunk/oak-lucene/src: main/java/org/apache/jackrabbit/oak/plugins/index/lucene/ test/java/org/apache/jackrabbit/oak/plugins/index/lucene/

Author: chetanm
Date: Tue Oct 14 10:03:02 2014
New Revision: 1631704

URL: http://svn.apache.org/r1631704
Log:
OAK-2005 - Use separate Lucene index for performing property related queries

Added support for querying based on properties via Lucene
-- Supports long, date, string, boolean, double
-- LucenePropertyIndex only works with non full text Lucene index definitions

Added:
    jackrabbit/oak/trunk/oak-lucene/src/main/java/org/apache/jackrabbit/oak/plugins/index/lucene/IndexPlanner.java   (with props)
    jackrabbit/oak/trunk/oak-lucene/src/test/java/org/apache/jackrabbit/oak/plugins/index/lucene/LucenePropertyIndexTest.java   (with props)
Modified:
    jackrabbit/oak/trunk/oak-lucene/src/main/java/org/apache/jackrabbit/oak/plugins/index/lucene/LuceneIndexProvider.java
    jackrabbit/oak/trunk/oak-lucene/src/main/java/org/apache/jackrabbit/oak/plugins/index/lucene/LucenePropertyIndex.java
    jackrabbit/oak/trunk/oak-lucene/src/test/java/org/apache/jackrabbit/oak/plugins/index/lucene/LuceneIndexEditorTest.java

Added: jackrabbit/oak/trunk/oak-lucene/src/main/java/org/apache/jackrabbit/oak/plugins/index/lucene/IndexPlanner.java
URL: http://svn.apache.org/viewvc/jackrabbit/oak/trunk/oak-lucene/src/main/java/org/apache/jackrabbit/oak/plugins/index/lucene/IndexPlanner.java?rev=1631704&view=auto
==============================================================================
--- jackrabbit/oak/trunk/oak-lucene/src/main/java/org/apache/jackrabbit/oak/plugins/index/lucene/IndexPlanner.java (added)
+++ jackrabbit/oak/trunk/oak-lucene/src/main/java/org/apache/jackrabbit/oak/plugins/index/lucene/IndexPlanner.java Tue Oct 14 10:03:02 2014
@@ -0,0 +1,140 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *   http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied.  See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+package org.apache.jackrabbit.oak.plugins.index.lucene;
+
+import java.util.Collections;
+import java.util.List;
+
+import javax.annotation.CheckForNull;
+
+import org.apache.jackrabbit.oak.query.fulltext.FullTextExpression;
+import org.apache.jackrabbit.oak.spi.query.Filter;
+import org.apache.lucene.index.IndexReader;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+import static com.google.common.collect.Lists.newArrayListWithCapacity;
+import static org.apache.jackrabbit.oak.spi.query.Filter.PropertyRestriction;
+import static org.apache.jackrabbit.oak.spi.query.QueryIndex.IndexPlan;
+import static org.apache.jackrabbit.oak.spi.query.QueryIndex.OrderEntry;
+
+public class IndexPlanner {
+    private final IndexDefinition defn;
+    private final Filter filter;
+    private final String indexPath;
+    private final List<OrderEntry> sortOrder;
+    private IndexNode indexNode;
+
+    public IndexPlanner(IndexNode indexNode,
+                        String indexPath,
+                        Filter filter, List<OrderEntry> sortOrder) {
+        this.indexNode = indexNode;
+        this.indexPath = indexPath;
+        this.defn = indexNode.getDefinition();
+        this.filter = filter;
+        this.sortOrder = sortOrder;
+    }
+
+    IndexPlan getPlan() {
+        IndexPlan.Builder builder = getPlanBuilder();
+        return builder != null ? builder.build() : null;
+    }
+
+    private IndexPlan.Builder getPlanBuilder() {
+        //TODO Support native functions
+
+        FullTextExpression ft = filter.getFullTextConstraint();
+
+        //IndexPlanner is currently for property indexes only and does not
+        //support full text indexes
+        if (ft != null) {
+            return null;
+        }
+
+        List<String> indexedProps = newArrayListWithCapacity(filter.getPropertyRestrictions().size());
+        for (PropertyRestriction pr : filter.getPropertyRestrictions()) {
+            //Only those properties which are included and not tokenized
+            //can be managed by lucene for property restrictions
+            if (defn.includeProperty(pr.propertyName)
+                    && defn.skipTokenization(pr.propertyName)) {
+                indexedProps.add(pr.propertyName);
+            }
+        }
+
+        if (!indexedProps.isEmpty()) {
+            //TODO Need a way to have better cost estimate to indicate that
+            //this index can evaluate more propertyRestrictions natively (if more props are indexed)
+            //For now we reduce cost per entry
+            IndexPlan.Builder plan = defaultPlan();
+            if (plan != null) {
+                return plan.setCostPerEntry(1.0 / indexedProps.size());
+            }
+        }
+
+        //TODO Support for path restrictions
+        //TODO support for NodeTypes
+        //TODO Support for property existence queries
+        //TODO support for nodeName queries
+        return null;
+    }
+
+    private IndexPlan.Builder defaultPlan() {
+        return new IndexPlan.Builder()
+                .setCostPerExecution(1) // we're local. Low-cost
+                .setCostPerEntry(1)
+                .setFulltextIndex(defn.isFullTextEnabled())
+                .setIncludesNodeData(false) // we should not include node data
+                .setFilter(filter)
+                .setSortOrder(createSortOrder())
+                .setDelayed(true) //Lucene is always async
+                .setAttribute(LuceneIndex.ATTR_INDEX_PATH, indexPath)
+                .setEstimatedEntryCount(getReader().numDocs());
+    }
+
+    private IndexReader getReader() {
+        return indexNode.getSearcher().getIndexReader();
+    }
+
+    @CheckForNull
+    private List<OrderEntry> createSortOrder() {
+        //TODO Refine later once we make mixed indexes having both
+        //full text  and property index
+        if (defn.isFullTextEnabled()) {
+            return Collections.emptyList();
+        }
+
+        if (sortOrder == null) {
+            return null;
+        }
+
+        List<OrderEntry> orderEntries = newArrayListWithCapacity(sortOrder.size());
+        for (OrderEntry o : sortOrder) {
+            //sorting can only be done for known/configured properties
+            // and whose types are known
+            //TODO Can sorting be done for array properties
+            if (defn.includeProperty(o.getPropertyName())
+                    && o.getPropertyType() != null
+                    && !o.getPropertyType().isArray()) {
+                orderEntries.add(o); //Lucene can manage any order desc/asc
+            }
+        }
+        return orderEntries;
+    }
+}

Propchange: jackrabbit/oak/trunk/oak-lucene/src/main/java/org/apache/jackrabbit/oak/plugins/index/lucene/IndexPlanner.java
------------------------------------------------------------------------------
    svn:eol-style = native

Modified: jackrabbit/oak/trunk/oak-lucene/src/main/java/org/apache/jackrabbit/oak/plugins/index/lucene/LuceneIndexProvider.java
URL: http://svn.apache.org/viewvc/jackrabbit/oak/trunk/oak-lucene/src/main/java/org/apache/jackrabbit/oak/plugins/index/lucene/LuceneIndexProvider.java?rev=1631704&r1=1631703&r2=1631704&view=diff
==============================================================================
--- jackrabbit/oak/trunk/oak-lucene/src/main/java/org/apache/jackrabbit/oak/plugins/index/lucene/LuceneIndexProvider.java (original)
+++ jackrabbit/oak/trunk/oak-lucene/src/main/java/org/apache/jackrabbit/oak/plugins/index/lucene/LuceneIndexProvider.java Tue Oct 14 10:03:02 2014
@@ -59,13 +59,18 @@ public class LuceneIndexProvider impleme
 
     @Override @Nonnull
     public List<QueryIndex> getQueryIndexes(NodeState nodeState) {
-        return ImmutableList.<QueryIndex> of(newLuceneIndex());
+        return ImmutableList.<QueryIndex> of(newLuceneIndex(), newLucenePropertyIndex());
     }
 
     protected LuceneIndex newLuceneIndex() {
         return new LuceneIndex(tracker, analyzer, aggregator);
     }
 
+    protected LucenePropertyIndex newLucenePropertyIndex() {
+        return new LucenePropertyIndex(tracker, analyzer, aggregator);
+    }
+
+
     /**
      * sets the default analyzer that will be used at query time
      */

Modified: jackrabbit/oak/trunk/oak-lucene/src/main/java/org/apache/jackrabbit/oak/plugins/index/lucene/LucenePropertyIndex.java
URL: http://svn.apache.org/viewvc/jackrabbit/oak/trunk/oak-lucene/src/main/java/org/apache/jackrabbit/oak/plugins/index/lucene/LucenePropertyIndex.java?rev=1631704&r1=1631703&r2=1631704&view=diff
==============================================================================
--- jackrabbit/oak/trunk/oak-lucene/src/main/java/org/apache/jackrabbit/oak/plugins/index/lucene/LucenePropertyIndex.java (original)
+++ jackrabbit/oak/trunk/oak-lucene/src/main/java/org/apache/jackrabbit/oak/plugins/index/lucene/LucenePropertyIndex.java Tue Oct 14 10:03:02 2014
@@ -30,10 +30,16 @@ import java.util.List;
 import java.util.Set;
 import java.util.concurrent.atomic.AtomicReference;
 
+import javax.annotation.CheckForNull;
+import javax.jcr.PropertyType;
+
 import com.google.common.collect.AbstractIterator;
+import com.google.common.collect.Lists;
 import com.google.common.collect.Queues;
 import com.google.common.collect.Sets;
 import org.apache.jackrabbit.oak.api.PropertyValue;
+import org.apache.jackrabbit.oak.api.Type;
+import org.apache.jackrabbit.oak.commons.PathUtils;
 import org.apache.jackrabbit.oak.plugins.index.aggregate.NodeAggregator;
 import org.apache.jackrabbit.oak.plugins.index.lucene.util.MoreLikeThisHelper;
 import org.apache.jackrabbit.oak.query.QueryEngineSettings;
@@ -68,6 +74,7 @@ import org.apache.lucene.search.BooleanQ
 import org.apache.lucene.search.IndexSearcher;
 import org.apache.lucene.search.MatchAllDocsQuery;
 import org.apache.lucene.search.MultiPhraseQuery;
+import org.apache.lucene.search.NumericRangeQuery;
 import org.apache.lucene.search.PhraseQuery;
 import org.apache.lucene.search.PrefixQuery;
 import org.apache.lucene.search.Query;
@@ -85,6 +92,7 @@ import org.slf4j.LoggerFactory;
 import static com.google.common.base.Preconditions.checkState;
 import static org.apache.jackrabbit.JcrConstants.JCR_MIXINTYPES;
 import static org.apache.jackrabbit.JcrConstants.JCR_PRIMARYTYPE;
+import static org.apache.jackrabbit.oak.api.Type.LONG;
 import static org.apache.jackrabbit.oak.api.Type.STRING;
 import static org.apache.jackrabbit.oak.commons.PathUtils.denotesRoot;
 import static org.apache.jackrabbit.oak.commons.PathUtils.getAncestorPath;
@@ -95,7 +103,6 @@ import static org.apache.jackrabbit.oak.
 import static org.apache.jackrabbit.oak.plugins.index.lucene.LuceneIndexConstants.VERSION;
 import static org.apache.jackrabbit.oak.plugins.index.lucene.TermFactory.newFulltextTerm;
 import static org.apache.jackrabbit.oak.plugins.index.lucene.TermFactory.newPathTerm;
-import static org.apache.jackrabbit.oak.plugins.index.lucene.util.LuceneIndexHelper.skipTokenization;
 import static org.apache.jackrabbit.oak.query.QueryImpl.JCR_PATH;
 import static org.apache.jackrabbit.oak.spi.query.QueryIndex.AdvanceFulltextQueryIndex;
 import static org.apache.lucene.search.BooleanClause.Occur.MUST;
@@ -181,42 +188,25 @@ public class LucenePropertyIndex impleme
 
     @Override
     public List<IndexPlan> getPlans(Filter filter, List<OrderEntry> sortOrder, NodeState rootState) {
-        FullTextExpression ft = filter.getFullTextConstraint();
-        if (ft == null) {
-            // no full-text condition: don't use this index,
-            // as there might be a better one
-            return Collections.emptyList();
-        }
-
-        String indexPath = new LuceneIndexLookup(rootState).getFullTextIndexPath(filter, tracker);
-        if (indexPath == null) { // unusable index
-            return Collections.emptyList();
+        Collection<String> indexPaths = new LuceneIndexLookup(rootState).collectIndexNodePaths(filter);
+        List<IndexPlan> plans = Lists.newArrayListWithCapacity(indexPaths.size());
+        IndexNode indexNode = null;
+        for (String path : indexPaths) {
+            try {
+                indexNode = tracker.acquireIndexNode(path);
+                if (indexNode != null) {
+                    IndexPlan plan = new IndexPlanner(indexNode, path, filter, sortOrder).getPlan();
+                    if (plan != null) {
+                        plans.add(plan);
+                    }
+                }
+            } finally {
+                if (indexNode != null) {
+                    indexNode.release();
+                }
+            }
         }
-        Set<String> relPaths = getRelativePaths(ft);
-        if (relPaths.size() > 1) {
-            LOG.warn("More than one relative parent for query " + filter.getQueryStatement());
-            // there are multiple "parents", as in
-            // "contains(a/x, 'hello') and contains(b/x, 'world')"
-            return Collections.emptyList();
-        }
-        String parent = relPaths.iterator().next();
-
-        // no relative properties
-        double cost = 10;
-        if (!parent.isEmpty()) {
-            // all relative properties have the same "parent", as in
-            // "contains(a/x, 'hello') and contains(a/y, 'world')" or
-            // "contains(a/x, 'hello') or contains(a/*, 'world')"
-            // TODO: proper cost calculation
-            // we assume this will cause more read operations,
-            // as we need to read the node and then the parent
-            cost = 15;
-        }
-        return Collections.singletonList(planBuilder(filter)
-                .setCostPerExecution(cost)
-                .setAttribute(ATTR_INDEX_PATH, indexPath)
-                .build());
-
+        return plans;
     }
 
     @Override
@@ -244,11 +234,23 @@ public class LucenePropertyIndex impleme
             // we only restrict non-full-text conditions if there is
             // no relative property in the full-text constraint
             boolean nonFullTextConstraints = parent.isEmpty();
-            String planDesc = getQuery(filter, null, nonFullTextConstraints, analyzer, index.getDefinition()) + " ft:(" + ft + ")";
+            StringBuilder sb = new StringBuilder("lucene:");
+            String path = (String) plan.getAttribute(ATTR_INDEX_PATH);
+            sb.append(getIndexName(plan))
+                    .append("(")
+                    .append(path)
+                    .append(") ");
+            sb.append(getQuery(filter, null, nonFullTextConstraints, analyzer, index.getDefinition()));
+            if(plan.getSortOrder() != null && !plan.getSortOrder().isEmpty()){
+                sb.append(" ordering:").append(plan.getSortOrder());
+            }
+            if (ft != null) {
+                sb.append(" ft:(").append(ft).append(")");
+            }
             if (!parent.isEmpty()) {
-                planDesc += " parent:" + parent;
+                sb.append(" parent:").append(parent);
             }
-            return planDesc;
+            return sb.toString();
         } finally {
             index.release();
         }
@@ -290,6 +292,8 @@ public class LucenePropertyIndex impleme
 
             private LuceneResultRow convertToRow(ScoreDoc doc, IndexSearcher searcher) throws IOException {
                 IndexReader reader = searcher.getIndexReader();
+                //TODO Look into usage of field cache for retrieving the path
+                //instead of reading via reader if no of docs in index are limited
                 PathStoredFieldVisitor visitor = new PathStoredFieldVisitor();
                 reader.document(doc.doc, visitor);
                 String path = visitor.getPath();
@@ -367,15 +371,8 @@ public class LucenePropertyIndex impleme
         return new LucenePathCursor(itr, settings);
     }
 
-    protected static IndexPlan.Builder planBuilder(Filter filter){
-        return new IndexPlan.Builder()
-                .setCostPerExecution(0) // we're local. Low-cost
-                .setCostPerEntry(1)
-                .setFilter(filter)
-                .setFulltextIndex(true)
-                .setEstimatedEntryCount(0) //TODO Fake it to provide constant cost for now
-                .setIncludesNodeData(false) // we should not include node data
-                .setDelayed(true); //Lucene is always async
+    private static String getIndexName(IndexPlan plan){
+        return PathUtils.getName((String) plan.getAttribute(ATTR_INDEX_PATH));
     }
 
     /**
@@ -472,6 +469,9 @@ public class LucenePropertyIndex impleme
                     indexDefinition);
         }
         if (qs.size() == 0) {
+            if (!indexDefinition.isFullTextEnabled()) {
+                throw new IllegalStateException("No query created for filter " + filter);
+            }
             return new MatchAllDocsQuery();
         }
         if (qs.size() == 1) {
@@ -530,9 +530,15 @@ public class LucenePropertyIndex impleme
 
         for (PropertyRestriction pr : filter.getPropertyRestrictions()) {
 
-            if (pr.first == null && pr.last == null) {
+            if (pr.first == null && pr.last == null && pr.list == null) {
                 // ignore property existence checks, Lucene can't to 'property
                 // is not null' queries (OAK-1208)
+
+                //TODO May be this can be relaxed if we have an index which
+                //maintains the list of propertyName present in a node say
+                //against :propNames. Only configured set of properties would
+                //be
+
                 continue;
             }
 
@@ -549,9 +555,12 @@ public class LucenePropertyIndex impleme
                 continue;
             }
 
-            if (skipTokenization(name)) {
-                qs.add(new TermQuery(new Term(name, pr.first
-                        .getValue(STRING))));
+            if (indexDefinition.skipTokenization(name)
+                    && indexDefinition.includeProperty(pr.propertyName)) {
+                Query q = createQuery(pr, indexDefinition);
+                if(q != null) {
+                    qs.add(q);
+                }
                 continue;
             }
 
@@ -618,6 +627,126 @@ public class LucenePropertyIndex impleme
         }
     }
 
+    @CheckForNull
+    private static Query createQuery(PropertyRestriction pr,
+                                     IndexDefinition defn) {
+        int propType = pr.propertyType;
+        if (defn.hasPropertyDefinition(pr.propertyName)) {
+            propType = defn.getPropDefn(pr.propertyName).getPropertyType();
+        }
+        switch (propType) {
+            case PropertyType.DATE: {
+                Long first = pr.first != null ? FieldFactory.dateToLong(pr.first.getValue(Type.DATE)) : null;
+                Long last = pr.last != null ? FieldFactory.dateToLong(pr.last.getValue(Type.DATE)) : null;
+                if (pr.first != null && pr.first.equals(pr.last) && pr.firstIncluding
+                        && pr.lastIncluding) {
+                    // [property]=[value]
+                    return NumericRangeQuery.newLongRange(pr.propertyName, first, first, true, true);
+                } else if (pr.first != null && pr.last != null) {
+                    return NumericRangeQuery.newLongRange(pr.propertyName, first, last,
+                            pr.firstIncluding, pr.lastIncluding);
+                } else if (pr.first != null && pr.last == null) {
+                    // '>' & '>=' use cases
+                    return NumericRangeQuery.newLongRange(pr.propertyName, first, null, pr.firstIncluding, true);
+                } else if (pr.last != null && !pr.last.equals(pr.first)) {
+                    // '<' & '<='
+                    return NumericRangeQuery.newLongRange(pr.propertyName, null, last, true, pr.lastIncluding);
+                } else if (pr.list != null) {
+                    BooleanQuery in = new BooleanQuery();
+                    for (PropertyValue value : pr.list) {
+                        Long dateVal = FieldFactory.dateToLong(value.getValue(Type.DATE));
+                        in.add(NumericRangeQuery.newLongRange(pr.propertyName, dateVal, dateVal, true, true), BooleanClause.Occur.SHOULD);
+                    }
+                    return in;
+                }
+
+                break;
+            }
+            case PropertyType.DOUBLE: {
+                Double first = pr.first != null ? pr.first.getValue(Type.DOUBLE) : null;
+                Double last = pr.last != null ? pr.last.getValue(Type.DOUBLE) : null;
+                if (pr.first != null && pr.first.equals(pr.last) && pr.firstIncluding
+                        && pr.lastIncluding) {
+                    // [property]=[value]
+                    return NumericRangeQuery.newDoubleRange(pr.propertyName, first, first, true, true);
+                } else if (pr.first != null && pr.last != null) {
+                    return NumericRangeQuery.newDoubleRange(pr.propertyName, first, last,
+                            pr.firstIncluding, pr.lastIncluding);
+                } else if (pr.first != null && pr.last == null) {
+                    // '>' & '>=' use cases
+                    return NumericRangeQuery.newDoubleRange(pr.propertyName, first, null, pr.firstIncluding, true);
+                } else if (pr.last != null && !pr.last.equals(pr.first)) {
+                    // '<' & '<='
+                    return NumericRangeQuery.newDoubleRange(pr.propertyName, null, last, true, pr.lastIncluding);
+                } else if (pr.list != null) {
+                    BooleanQuery in = new BooleanQuery();
+                    for (PropertyValue value : pr.list) {
+                        Double doubleVal = value.getValue(Type.DOUBLE);
+                        in.add(NumericRangeQuery.newDoubleRange(pr.propertyName, doubleVal, doubleVal, true, true), BooleanClause.Occur.SHOULD);
+                    }
+                    return in;
+                }
+                break;
+            }
+            case PropertyType.LONG: {
+                Long first = pr.first != null ? pr.first.getValue(LONG) : null;
+                Long last = pr.last != null ? pr.last.getValue(LONG) : null;
+                if (pr.first != null && pr.first.equals(pr.last) && pr.firstIncluding
+                        && pr.lastIncluding) {
+                    // [property]=[value]
+                    return NumericRangeQuery.newLongRange(pr.propertyName, first, first, true, true);
+                } else if (pr.first != null && pr.last != null) {
+                    return NumericRangeQuery.newLongRange(pr.propertyName, first, last,
+                            pr.firstIncluding, pr.lastIncluding);
+                } else if (pr.first != null && pr.last == null) {
+                    // '>' & '>=' use cases
+                    return NumericRangeQuery.newLongRange(pr.propertyName, first, null, pr.firstIncluding, true);
+                } else if (pr.last != null && !pr.last.equals(pr.first)) {
+                    // '<' & '<='
+                    return NumericRangeQuery.newLongRange(pr.propertyName, null, last, true, pr.lastIncluding);
+                } else if (pr.list != null) {
+                    BooleanQuery in = new BooleanQuery();
+                    for (PropertyValue value : pr.list) {
+                        Long longVal = value.getValue(LONG);
+                        in.add(NumericRangeQuery.newLongRange(pr.propertyName, longVal, longVal, true, true), BooleanClause.Occur.SHOULD);
+                    }
+                    return in;
+                }
+                break;
+            }
+            default: {
+                //TODO Support for pr.like
+
+                //TODO Confirm that all other types can be treated as string
+                String first = pr.first != null ? pr.first.getValue(STRING) : null;
+                String last = pr.last != null ? pr.last.getValue(STRING) : null;
+                if (pr.first != null && pr.first.equals(pr.last) && pr.firstIncluding
+                        && pr.lastIncluding) {
+                    // [property]=[value]
+                    return new TermQuery(new Term(pr.propertyName, first));
+                } else if (pr.first != null && pr.last != null) {
+                    return TermRangeQuery.newStringRange(pr.propertyName, first, last,
+                            pr.firstIncluding, pr.lastIncluding);
+                } else if (pr.first != null && pr.last == null) {
+                    // '>' & '>=' use cases
+                    return TermRangeQuery.newStringRange(pr.propertyName, first, null, pr.firstIncluding, true);
+                } else if (pr.last != null && !pr.last.equals(pr.first)) {
+                    // '<' & '<='
+                    return TermRangeQuery.newStringRange(pr.propertyName, null, last, true, pr.lastIncluding);
+                } else if (pr.list != null) {
+                    BooleanQuery in = new BooleanQuery();
+                    for (PropertyValue value : pr.list) {
+                        String strVal = value.getValue(STRING);
+                        in.add(new TermQuery(new Term(pr.propertyName, strVal)), BooleanClause.Occur.SHOULD);
+                    }
+                    return in;
+                }
+            }
+        }
+        throw new IllegalStateException("PropertyRestriction not handled " + pr + " for index " + defn );
+    }
+
+
     private static String tokenizeAndPoll(String token, Analyzer analyzer){
         if (token != null) {
             List<String> tokens = tokenize(token, analyzer);
@@ -636,9 +765,14 @@ public class LucenePropertyIndex impleme
             return true;
         }
 
+        boolean includeProperty = definition.includeProperty(name);
         // check name
-        if(!definition.includeProperty(name)){
+        if (!includeProperty) {
             return true;
+        } else if (!definition.isFullTextEnabled()) {
+            //For property index do not filter on type. If property
+            //is included then that is sufficient
+           return false;
         }
 
         // check type
@@ -677,6 +811,7 @@ public class LucenePropertyIndex impleme
 
     private static void addNodeTypeConstraints(List<Query> qs, Filter filter) {
         BooleanQuery bq = new BooleanQuery();
+        //TODO These condition should only be added if those propertyTypes are indexed
         for (String type : filter.getPrimaryTypes()) {
             bq.add(new TermQuery(new Term(JCR_PRIMARYTYPE, type)), SHOULD);
         }

Modified: jackrabbit/oak/trunk/oak-lucene/src/test/java/org/apache/jackrabbit/oak/plugins/index/lucene/LuceneIndexEditorTest.java
URL: http://svn.apache.org/viewvc/jackrabbit/oak/trunk/oak-lucene/src/test/java/org/apache/jackrabbit/oak/plugins/index/lucene/LuceneIndexEditorTest.java?rev=1631704&r1=1631703&r2=1631704&view=diff
==============================================================================
--- jackrabbit/oak/trunk/oak-lucene/src/test/java/org/apache/jackrabbit/oak/plugins/index/lucene/LuceneIndexEditorTest.java (original)
+++ jackrabbit/oak/trunk/oak-lucene/src/test/java/org/apache/jackrabbit/oak/plugins/index/lucene/LuceneIndexEditorTest.java Tue Oct 14 10:03:02 2014
@@ -179,14 +179,14 @@ public class LuceneIndexEditorTest {
         return indexNode.getSearcher();
     }
 
-    private static Calendar createDate(String dt) throws java.text.ParseException {
-        SimpleDateFormat sdf = new SimpleDateFormat("dd/mm/yyyy");
+    static Calendar createDate(String dt) throws java.text.ParseException {
+        SimpleDateFormat sdf = new SimpleDateFormat("dd/MM/yyyy");
         Calendar cal = Calendar.getInstance();
         cal.setTime(sdf.parse(dt));
         return cal;
     }
 
-    private static long dateToTime(String dt) throws java.text.ParseException {
+    static long dateToTime(String dt) throws java.text.ParseException {
         return FieldFactory.dateToLong(ISO8601.format(createDate(dt)));
     }
 }

Added: jackrabbit/oak/trunk/oak-lucene/src/test/java/org/apache/jackrabbit/oak/plugins/index/lucene/LucenePropertyIndexTest.java
URL: http://svn.apache.org/viewvc/jackrabbit/oak/trunk/oak-lucene/src/test/java/org/apache/jackrabbit/oak/plugins/index/lucene/LucenePropertyIndexTest.java?rev=1631704&view=auto
==============================================================================
--- jackrabbit/oak/trunk/oak-lucene/src/test/java/org/apache/jackrabbit/oak/plugins/index/lucene/LucenePropertyIndexTest.java (added)
+++ jackrabbit/oak/trunk/oak-lucene/src/test/java/org/apache/jackrabbit/oak/plugins/index/lucene/LucenePropertyIndexTest.java Tue Oct 14 10:03:02 2014
@@ -0,0 +1,215 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *   http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied.  See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+package org.apache.jackrabbit.oak.plugins.index.lucene;
+
+import java.text.ParseException;
+import java.util.Set;
+
+import javax.jcr.PropertyType;
+
+import org.apache.jackrabbit.JcrConstants;
+import org.apache.jackrabbit.oak.Oak;
+import org.apache.jackrabbit.oak.api.CommitFailedException;
+import org.apache.jackrabbit.oak.api.ContentRepository;
+import org.apache.jackrabbit.oak.api.Tree;
+import org.apache.jackrabbit.oak.api.Type;
+import org.apache.jackrabbit.oak.plugins.index.lucene.util.LuceneInitializerHelper;
+import org.apache.jackrabbit.oak.plugins.memory.PropertyStates;
+import org.apache.jackrabbit.oak.plugins.nodetype.write.InitialContent;
+import org.apache.jackrabbit.oak.query.AbstractQueryTest;
+import org.apache.jackrabbit.oak.spi.commit.Observer;
+import org.apache.jackrabbit.oak.spi.query.QueryIndexProvider;
+import org.apache.jackrabbit.oak.spi.security.OpenSecurityProvider;
+import org.apache.jackrabbit.util.ISO8601;
+import org.junit.Test;
+
+import static com.google.common.collect.ImmutableSet.of;
+import static java.util.Arrays.asList;
+import static org.apache.jackrabbit.oak.plugins.index.IndexConstants.INDEX_DEFINITIONS_NAME;
+import static org.apache.jackrabbit.oak.plugins.index.IndexConstants.INDEX_DEFINITIONS_NODE_TYPE;
+import static org.apache.jackrabbit.oak.plugins.index.IndexConstants.REINDEX_PROPERTY_NAME;
+import static org.apache.jackrabbit.oak.plugins.index.IndexConstants.TYPE_PROPERTY_NAME;
+import static org.apache.jackrabbit.oak.plugins.index.lucene.LuceneIndexEditorTest.createDate;
+import static org.junit.Assert.assertThat;
+import static org.junit.matchers.JUnitMatchers.containsString;
+
+public class LucenePropertyIndexTest extends AbstractQueryTest {
+
+    @Override
+    protected void createTestIndexNode() throws Exception {
+        setTraversalEnabled(false);
+    }
+
+    @Override
+    protected ContentRepository createRepository() {
+        LuceneIndexProvider provider = new LuceneIndexProvider();
+        return new Oak()
+                .with(new InitialContent())
+                .with(new LuceneInitializerHelper("luceneGlobal", (Set<String>) null))
+                .with(new OpenSecurityProvider())
+                .with((QueryIndexProvider) provider)
+                .with((Observer) provider)
+                .with(new LuceneIndexEditorProvider())
+                .createContentRepository();
+    }
+
+    @Test
+    public void indexSelection() throws Exception {
+        createIndex("test1", of("propa", "propb"));
+        createIndex("test2", of("propc"));
+
+        Tree test = root.getTree("/").addChild("test");
+        test.addChild("a").setProperty("propa", "foo");
+        test.addChild("b").setProperty("propa", "foo");
+        test.addChild("c").setProperty("propa", "foo2");
+        test.addChild("d").setProperty("propc", "foo");
+        test.addChild("e").setProperty("propd", "foo");
+        root.commit();
+
+        String propaQuery = "select [jcr:path] from [nt:base] where [propa] = 'foo'";
+        assertThat(explain(propaQuery), containsString("lucene:test1"));
+        assertThat(explain("select [jcr:path] from [nt:base] where [propc] = 'foo'"), containsString("lucene:test2"));
+
+        assertQuery(propaQuery, asList("/test/a", "/test/b"));
+        assertQuery("select [jcr:path] from [nt:base] where [propa] = 'foo2'", asList("/test/c"));
+        assertQuery("select [jcr:path] from [nt:base] where [propc] = 'foo'", asList("/test/d"));
+    }
+
+    @Test
+    public void rangeQueriesWithLong() throws Exception {
+        Tree idx = createIndex("test1", of("propa", "propb"));
+        Tree propIdx = idx.addChild("propa");
+        propIdx.setProperty(LuceneIndexConstants.PROP_TYPE, PropertyType.TYPENAME_LONG);
+        root.commit();
+
+        Tree test = root.getTree("/").addChild("test");
+        test.addChild("a").setProperty("propa", 10);
+        test.addChild("b").setProperty("propa", 20);
+        test.addChild("c").setProperty("propa", 30);
+        test.addChild("c").setProperty("propb", "foo");
+        test.addChild("d").setProperty("propb", "foo");
+        root.commit();
+
+        assertThat(explain("select [jcr:path] from [nt:base] where [propa] >= 20"), containsString("lucene:test1"));
+
+        assertQuery("select [jcr:path] from [nt:base] where [propa] >= 20", asList("/test/b", "/test/c"));
+        assertQuery("select [jcr:path] from [nt:base] where [propa] >= 20", asList("/test/b", "/test/c"));
+        assertQuery("select [jcr:path] from [nt:base] where [propa] = 20", asList("/test/b"));
+        assertQuery("select [jcr:path] from [nt:base] where [propa] <= 20", asList("/test/b", "/test/a"));
+        assertQuery("select [jcr:path] from [nt:base] where [propa] < 20", asList("/test/a"));
+        assertQuery("select [jcr:path] from [nt:base] where [propa] = 20 or [propa] = 10 ", asList("/test/b", "/test/a"));
+        assertQuery("select [jcr:path] from [nt:base] where [propa] > 10 and [propa] < 30", asList("/test/b"));
+        assertQuery("select [jcr:path] from [nt:base] where [propa] in (10,20)", asList("/test/b", "/test/a"));
+    }
+
+    @Test
+    public void rangeQueriesWithDouble() throws Exception {
+        Tree idx = createIndex("test1", of("propa", "propb"));
+        Tree propIdx = idx.addChild("propa");
+        propIdx.setProperty(LuceneIndexConstants.PROP_TYPE, PropertyType.TYPENAME_DOUBLE);
+        root.commit();
+
+        Tree test = root.getTree("/").addChild("test");
+        test.addChild("a").setProperty("propa", 10.1);
+        test.addChild("b").setProperty("propa", 20.4);
+        test.addChild("c").setProperty("propa", 30.7);
+        test.addChild("c").setProperty("propb", "foo");
+        test.addChild("d").setProperty("propb", "foo");
+        root.commit();
+
+        assertQuery("select [jcr:path] from [nt:base] where [propa] >= 20.3", asList("/test/b", "/test/c"));
+        assertQuery("select [jcr:path] from [nt:base] where [propa] = 20.4", asList("/test/b"));
+        assertQuery("select [jcr:path] from [nt:base] where [propa] <= 20.5", asList("/test/b", "/test/a"));
+        assertQuery("select [jcr:path] from [nt:base] where [propa] < 20.4", asList("/test/a"));
+        assertQuery("select [jcr:path] from [nt:base] where [propa] > 10.5 and [propa] < 30", asList("/test/b"));
+    }
+
+    @Test
+    public void rangeQueriesWithString() throws Exception {
+        Tree idx = createIndex("test1", of("propa", "propb"));
+        idx.addChild("propa");
+        root.commit();
+
+        Tree test = root.getTree("/").addChild("test");
+        test.addChild("a").setProperty("propa", "a");
+        test.addChild("b").setProperty("propa", "b is b");
+        test.addChild("c").setProperty("propa", "c");
+        test.addChild("c").setProperty("propb", "e");
+        test.addChild("d").setProperty("propb", "f");
+        test.addChild("e").setProperty("propb", "g");
+        root.commit();
+
+        assertQuery("select [jcr:path] from [nt:base] where propa = 'a'", asList("/test/a"));
+        //Check that string props are not tokenized
+        assertQuery("select [jcr:path] from [nt:base] where propa = 'b is b'", asList("/test/b"));
+        assertQuery("select [jcr:path] from [nt:base] where propa in ('a', 'c')", asList("/test/a", "/test/c"));
+        assertQuery("select [jcr:path] from [nt:base] where [propb] >= 'f'", asList("/test/d", "/test/e"));
+        assertQuery("select [jcr:path] from [nt:base] where [propb] <= 'f'", asList("/test/c", "/test/d"));
+        assertQuery("select [jcr:path] from [nt:base] where [propb] > 'e'", asList("/test/d", "/test/e"));
+        assertQuery("select [jcr:path] from [nt:base] where [propb] < 'g'", asList("/test/c", "/test/d"));
+    }
+
+
+    @Test
+    public void rangeQueriesWithDate() throws Exception {
+        Tree idx = createIndex("test1", of("propa", "propb"));
+        Tree propIdx = idx.addChild("propa");
+        propIdx.setProperty(LuceneIndexConstants.PROP_TYPE, PropertyType.TYPENAME_DATE);
+        root.commit();
+
+        Tree test = root.getTree("/").addChild("test");
+        test.addChild("a").setProperty("propa", createDate("14/02/2014"));
+        test.addChild("b").setProperty("propa", createDate("14/03/2014"));
+        test.addChild("c").setProperty("propa", createDate("14/04/2014"));
+        test.addChild("c").setProperty("propb", "foo");
+        test.addChild("d").setProperty("propb", "foo");
+        root.commit();
+
+        assertQuery("select [jcr:path] from [nt:base] where [propa] >= " + dt("15/02/2014"), asList("/test/b", "/test/c"));
+        assertQuery("select [jcr:path] from [nt:base] where [propa] <=" + dt("15/03/2014"), asList("/test/b", "/test/a"));
+        assertQuery("select [jcr:path] from [nt:base] where [propa] < " + dt("14/03/2014"), asList("/test/a"));
+        assertQuery("select [jcr:path] from [nt:base] where [propa] > "+ dt("15/02/2014") + " and [propa] < " + dt("13/04/2014"), asList("/test/b"));
+    }
+
+    //TODO Test for range with Date. Check for precision
+
+    private String explain(String query){
+        String explain = "explain " + query;
+        return executeQuery(explain, "JCR-SQL2").get(0);
+    }
+
+    private Tree createIndex(String name, Set<String> propNames) throws CommitFailedException {
+        Tree index = root.getTree("/");
+        Tree def = index.addChild(INDEX_DEFINITIONS_NAME).addChild(name);
+        def.setProperty(JcrConstants.JCR_PRIMARYTYPE,
+                INDEX_DEFINITIONS_NODE_TYPE, Type.NAME);
+        def.setProperty(TYPE_PROPERTY_NAME, LuceneIndexConstants.TYPE_LUCENE);
+        def.setProperty(REINDEX_PROPERTY_NAME, true);
+        def.setProperty(LuceneIndexConstants.FULL_TEXT_ENABLED, false);
+        def.setProperty(PropertyStates.createProperty(LuceneIndexConstants.INCLUDE_PROPERTY_NAMES, propNames, Type.STRINGS));
+        root.commit();
+        return root.getTree("/").getChild(INDEX_DEFINITIONS_NAME).getChild(name);
+    }
+
+
+    private static String dt(String date) throws ParseException {
+        return String.format("CAST ('%s' AS DATE)",ISO8601.format(createDate(date)));
+    }
+}

Propchange: jackrabbit/oak/trunk/oak-lucene/src/test/java/org/apache/jackrabbit/oak/plugins/index/lucene/LucenePropertyIndexTest.java
------------------------------------------------------------------------------
    svn:eol-style = native