jackrabbit-oak-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From thom...@apache.org
Subject svn commit: r1431216 - in /jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak: plugins/index/nodetype/ plugins/index/p2/ plugins/index/p2/strategy/ spi/query/
Date Thu, 10 Jan 2013 09:18:35 GMT
Author: thomasm
Date: Thu Jan 10 09:18:34 2013
New Revision: 1431216

URL: http://svn.apache.org/viewvc?rev=1431216&view=rev
Log:
OAK-537 The Property2Index eagerly and unnecessarily fetches all data

Modified:
    jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/index/nodetype/NodeTypeIndex.java
    jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/index/nodetype/NodeTypeIndexLookup.java
    jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/index/p2/Property2Index.java
    jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/index/p2/Property2IndexLookup.java
    jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/index/p2/strategy/ContentMirrorStoreStrategy.java
    jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/index/p2/strategy/IndexStoreStrategy.java
    jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/spi/query/Cursors.java

Modified: jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/index/nodetype/NodeTypeIndex.java
URL: http://svn.apache.org/viewvc/jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/index/nodetype/NodeTypeIndex.java?rev=1431216&r1=1431215&r2=1431216&view=diff
==============================================================================
--- jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/index/nodetype/NodeTypeIndex.java
(original)
+++ jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/index/nodetype/NodeTypeIndex.java
Thu Jan 10 09:18:34 2013
@@ -70,7 +70,7 @@ class NodeTypeIndex implements QueryInde
             throw new IllegalStateException(
                     "NodeType index is used even when no index is available for filter "
+ filter);
         }
-        return Cursors.newPathCursor(lookup.query(
+        return Cursors.newPathCursorDistinct(lookup.query(
                 resolveNodeType(root, filter.getNodeType())));
     }
     
@@ -79,17 +79,6 @@ class NodeTypeIndex implements QueryInde
         return "nodeType " + filter.getNodeType() + " path " + filter.getPath();
     }
 
-    @Deprecated
-    public Cursor queryOld(Filter filter, NodeState root) {
-        NodeTypeIndexLookup lookup = new NodeTypeIndexLookup(root);
-        if (!hasNodeTypeRestriction(filter) || !lookup.isIndexed(filter.getPath())) {
-            throw new IllegalStateException(
-                    "NodeType index is used even when no index is available for filter "
+ filter);
-        }
-        return Cursors.newPathCursor(lookup.find(
-                resolveNodeType(root, filter.getNodeType())));
-    }
-
     @Override
     public String getIndexName() {
         return "nodeType";

Modified: jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/index/nodetype/NodeTypeIndexLookup.java
URL: http://svn.apache.org/viewvc/jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/index/nodetype/NodeTypeIndexLookup.java?rev=1431216&r1=1431215&r2=1431216&view=diff
==============================================================================
--- jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/index/nodetype/NodeTypeIndexLookup.java
(original)
+++ jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/index/nodetype/NodeTypeIndexLookup.java
Thu Jan 10 09:18:34 2013
@@ -16,8 +16,6 @@
  */
 package org.apache.jackrabbit.oak.plugins.index.nodetype;
 
-import java.util.Set;
-
 import org.apache.jackrabbit.JcrConstants;
 import org.apache.jackrabbit.oak.api.PropertyValue;
 import org.apache.jackrabbit.oak.plugins.index.p2.Property2IndexLookup;
@@ -25,7 +23,6 @@ import org.apache.jackrabbit.oak.spi.que
 import org.apache.jackrabbit.oak.spi.state.NodeState;
 
 import com.google.common.collect.Iterables;
-import com.google.common.collect.Sets;
 
 /**
  * <code>NodeTypeIndexLookup</code> uses {@link PropertyIndexLookup} internally
@@ -73,43 +70,19 @@ class NodeTypeIndexLookup implements Jcr
         return lookup.getCost(JCR_PRIMARYTYPE, ntNames)
                 + lookup.getCost(JCR_MIXINTYPES, ntNames);
     }
-
-    /**
-     * Returns the paths that match the given node types.
-     *
-     * @param nodeTypes the names of the node types to match.
-     * @return the set of matched paths.
-     */
-    @Deprecated
-    public Set<String> find(Iterable<String> nodeTypes) {
-        Set<String> paths = Sets.newHashSet();
-        Property2IndexLookup lookup = new Property2IndexLookup(root);
-        PropertyValue ntNames = PropertyValues.newName(nodeTypes);
-        paths.addAll(lookup.find(JCR_PRIMARYTYPE, ntNames));
-        paths.addAll(lookup.find(JCR_MIXINTYPES, ntNames));
-        return paths;
-    }
     
     /**
      * Returns the paths that match the given node types.
      *
      * @param nodeTypes the names of the node types to match.
-     * @return the set of matched paths.
+     * @return the matched paths (the result might contain duplicate entries)
      */
     public Iterable<String> query(Iterable<String> nodeTypes) {
-        // TODO currently, fetch all data to avoid duplicate entries:
-        // the following code sometimes returns duplicate entries:
-        // final PropertyValue ntNames = PropertyValues.newName(nodeTypes);
-        // Property2IndexLookup lookup = new Property2IndexLookup(root);
-        // return Iterables.concat(
-        //     lookup.query(JCR_PRIMARYTYPE, ntNames),
-        //     lookup.query(JCR_MIXINTYPES, ntNames));
-        Set<String> paths = Sets.newHashSet();
+        final PropertyValue ntNames = PropertyValues.newName(nodeTypes);
         Property2IndexLookup lookup = new Property2IndexLookup(root);
-        PropertyValue ntNames = PropertyValues.newName(nodeTypes);
-        Iterables.addAll(paths, lookup.query(JCR_PRIMARYTYPE, ntNames));
-        Iterables.addAll(paths, lookup.query(JCR_MIXINTYPES, ntNames));
-        return paths;
+        return Iterables.concat(
+                lookup.query(JCR_PRIMARYTYPE, ntNames),
+                lookup.query(JCR_MIXINTYPES, ntNames));
     }
 
 }

Modified: jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/index/p2/Property2Index.java
URL: http://svn.apache.org/viewvc/jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/index/p2/Property2Index.java?rev=1431216&r1=1431215&r2=1431216&view=diff
==============================================================================
--- jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/index/p2/Property2Index.java
(original)
+++ jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/index/p2/Property2Index.java
Thu Jan 10 09:18:34 2013
@@ -20,7 +20,6 @@ import java.io.UnsupportedEncodingExcept
 import java.net.URLEncoder;
 import java.util.ArrayList;
 import java.util.List;
-import java.util.Set;
 
 import org.apache.jackrabbit.oak.api.PropertyValue;
 import org.apache.jackrabbit.oak.api.Type;
@@ -32,7 +31,6 @@ import org.apache.jackrabbit.oak.spi.que
 import org.apache.jackrabbit.oak.spi.state.NodeState;
 
 import com.google.common.base.Charsets;
-import com.google.common.collect.Sets;
 
 /**
  * Provides a QueryIndex that does lookups against a property index
@@ -174,43 +172,6 @@ class Property2Index implements QueryInd
             }
         }
         return buff.toString();
-   }
-    
-    @Deprecated
-    public Cursor queryOld(Filter filter, NodeState root) {
-        Set<String> paths = null;
-
-        Property2IndexLookup lookup = new Property2IndexLookup(root);
-        for (PropertyRestriction pr : filter.getPropertyRestrictions()) {
-            // TODO support indexes on a path
-            // currently, only indexes on the root node are supported
-            if (lookup.isIndexed(pr.propertyName, "/")) {
-                Set<String> set = null;
-                // equality
-                if (pr.firstIncluding && pr.lastIncluding
-                    && pr.first != null && pr.first.equals(pr.last)) {
-                    // "[property] = $value"
-                    // TODO don't load all entries in memory
-                    set = lookup.find(pr.propertyName, pr.first);
-                } else if (pr.first == null && pr.last == null) {
-                    // "[property] is not null"
-                    // TODO don't load all entries in memory
-                    set = lookup.find(pr.propertyName, null);
-                }
-                // only keep the intersection
-                // TODO this requires all paths are loaded in memory
-                if (set != null) {
-                    if (paths == null) {
-                        paths = Sets.newHashSet(set);
-                    } else {
-                        paths.retainAll(set);
-                    }
-                }
-            }
-        }
-        if (paths == null) {
-            throw new IllegalStateException("Property index is used even when no index is
available for filter " + filter);
-        }
-        return Cursors.newPathCursor(paths);
     }
+
 }
\ No newline at end of file

Modified: jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/index/p2/Property2IndexLookup.java
URL: http://svn.apache.org/viewvc/jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/index/p2/Property2IndexLookup.java?rev=1431216&r1=1431215&r2=1431216&view=diff
==============================================================================
--- jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/index/p2/Property2IndexLookup.java
(original)
+++ jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/index/p2/Property2IndexLookup.java
Thu Jan 10 09:18:34 2013
@@ -20,7 +20,6 @@ import static org.apache.jackrabbit.oak.
 
 import java.util.Iterator;
 import java.util.List;
-import java.util.Set;
 
 import javax.annotation.Nullable;
 
@@ -34,8 +33,6 @@ import org.apache.jackrabbit.oak.plugins
 import org.apache.jackrabbit.oak.spi.state.ChildNodeEntry;
 import org.apache.jackrabbit.oak.spi.state.NodeState;
 
-import com.google.common.collect.Sets;
-
 /**
  * Is responsible for querying the property index content.
  * 
@@ -91,28 +88,6 @@ public class Property2IndexLookup {
         }
         return false;
     }
-
-    /**
-     * Searches for a given value within this index.
-     * 
-     * @param name the property name
-     * @param value the property value (null to check for property existence)
-     * @return the set of matched paths
-     */
-    @Deprecated
-    public Set<String> find(String name, PropertyValue value) {
-        NodeState state = getIndexDataNode(root, name);
-        if (state == null) {
-            throw new IllegalArgumentException("No index for " + name);
-        }
-        Set<String> paths = Sets.newHashSet();
-        if (value == null) {
-            paths.addAll(store.find(state, null));
-        } else {
-            paths.addAll(store.find(state, Property2Index.encode(value)));
-        }
-        return paths;
-    }
     
     public Iterable<String> query(String name, PropertyValue value) {
         NodeState state = getIndexDataNode(root, name);

Modified: jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/index/p2/strategy/ContentMirrorStoreStrategy.java
URL: http://svn.apache.org/viewvc/jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/index/p2/strategy/ContentMirrorStoreStrategy.java?rev=1431216&r1=1431215&r2=1431216&view=diff
==============================================================================
--- jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/index/p2/strategy/ContentMirrorStoreStrategy.java
(original)
+++ jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/index/p2/strategy/ContentMirrorStoreStrategy.java
Thu Jan 10 09:18:34 2013
@@ -18,7 +18,6 @@ package org.apache.jackrabbit.oak.plugin
 
 import java.util.Collections;
 import java.util.Deque;
-import java.util.HashSet;
 import java.util.Iterator;
 import java.util.Map;
 import java.util.Set;
@@ -143,26 +142,6 @@ public class ContentMirrorStoreStrategy 
         }
         return count;
     }
-
-    @Override
-    @Deprecated    
-    public Set<String> find(NodeState index, Iterable<String> values) {
-        Set<String> paths = new HashSet<String>();
-        if (values == null) {
-            for (ChildNodeEntry child : index.getChildNodeEntries()) {
-                getMatchingPaths(child.getNodeState(), "", paths);
-            }
-        } else {
-            for (String p : values) {
-                NodeState property = index.getChildNode(p);
-                if (property != null) {
-                    // We have an entry for this value, so use it
-                    getMatchingPaths(property, "", paths);
-                }
-            }
-        }
-        return paths;
-    }
     
     @Override
     public Iterable<String> query(final String indexName, 
@@ -184,15 +163,32 @@ public class ContentMirrorStoreStrategy 
                         }
                     }
                 }
-                // avoid duplicate entries
-                // TODO load entries lazily
-                Set<String> paths = Sets.newHashSet();
-                Iterators.addAll(paths, it);
-                return paths.iterator();
+                return it;
             }
         };
     }
     
+    @Override
+    public int count(NodeState index, Iterable<String> values) {
+        int count = 0;
+        if (values == null) {
+            count += countMatchingLeaves(index);
+        } else {
+            for (String p : values) {
+                NodeState s = index.getChildNode(p);
+                if (s != null) {
+                    count += countMatchingLeaves(s);
+                }
+            }
+        }
+        return count;
+    }
+    
+    static boolean matches(NodeState state) {
+        PropertyState ps = state.getProperty("match");
+        return ps != null && !ps.isArray() && ps.getValue(Type.BOOLEAN);
+    }
+
     /**
      * An iterator over paths within an index node.
      */
@@ -208,6 +204,11 @@ public class ContentMirrorStoreStrategy 
         private String currentPath;
         private boolean pathContainsValue;
         
+        /**
+         * Keep the returned path, to avoid returning duplicate entries.
+         */
+        private final Set<String> knownPaths = Sets.newHashSet();
+        
         PathIterator(String indexName) {
             this.indexName = indexName;
             parentPath = "";
@@ -276,13 +277,21 @@ public class ContentMirrorStoreStrategy 
                 fetchNext();
                 init = true;
             }
-            String result = currentPath;
-            fetchNext();
-            if (pathContainsValue) {
-                String value = PathUtils.elements(result).iterator().next();
-                result = PathUtils.relativize(value, result);
+            while (true) {
+                String result = currentPath;
+                fetchNext();
+                if (pathContainsValue) {
+                    String value = PathUtils.elements(result).iterator().next();
+                    result = PathUtils.relativize(value, result);
+                    // don't return duplicate paths:
+                    // Set.add returns true if the entry was new,
+                    // so if it returns false, it was already known
+                    if (!knownPaths.add(result)) {
+                        continue;
+                    }
+                }
+                return result;
             }
-            return result;
         }
 
         @Override
@@ -292,37 +301,4 @@ public class ContentMirrorStoreStrategy 
         
     }
 
-    private void getMatchingPaths(NodeState state, String path,
-            Set<String> paths) {
-        if (matches(state)) {
-            paths.add(path);
-        }
-        for (ChildNodeEntry c : state.getChildNodeEntries()) {
-            String name = c.getName();
-            NodeState childState = c.getNodeState();
-            getMatchingPaths(childState, PathUtils.concat(path, name), paths);
-        }
-    }
-    
-    static boolean matches(NodeState state) {
-        PropertyState ps = state.getProperty("match");
-        return ps != null && !ps.isArray() && ps.getValue(Type.BOOLEAN);
-    }
-
-    @Override
-    public int count(NodeState index, Iterable<String> values) {
-        int count = 0;
-        if (values == null) {
-            count += countMatchingLeaves(index);
-        } else {
-            for (String p : values) {
-                NodeState s = index.getChildNode(p);
-                if (s != null) {
-                    count += countMatchingLeaves(s);
-                }
-            }
-        }
-        return count;
-    }
-
 }

Modified: jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/index/p2/strategy/IndexStoreStrategy.java
URL: http://svn.apache.org/viewvc/jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/index/p2/strategy/IndexStoreStrategy.java?rev=1431216&r1=1431215&r2=1431216&view=diff
==============================================================================
--- jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/index/p2/strategy/IndexStoreStrategy.java
(original)
+++ jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/index/p2/strategy/IndexStoreStrategy.java
Thu Jan 10 09:18:34 2013
@@ -16,8 +16,6 @@
  */
 package org.apache.jackrabbit.oak.plugins.index.p2.strategy;
 
-import java.util.Set;
-
 import org.apache.jackrabbit.oak.api.CommitFailedException;
 import org.apache.jackrabbit.oak.spi.state.NodeBuilder;
 import org.apache.jackrabbit.oak.spi.state.NodeState;
@@ -51,16 +49,6 @@ public interface IndexStoreStrategy {
      */
     void insert(NodeBuilder index, String key, boolean unique,
             Iterable<String> values) throws CommitFailedException;
-
-    /**
-     * Search for a given set of values.
-     * 
-     * @param index index node (may not be null)
-     * @param values values to look for (null to check for property existence)
-     * @return the set of paths corresponding to the given values
-     */
-    @Deprecated
-    Set<String> find(NodeState index, Iterable<String> values);
     
     /**
      * Search for a given set of values.

Modified: jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/spi/query/Cursors.java
URL: http://svn.apache.org/viewvc/jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/spi/query/Cursors.java?rev=1431216&r1=1431215&r2=1431216&view=diff
==============================================================================
--- jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/spi/query/Cursors.java
(original)
+++ jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/spi/query/Cursors.java
Thu Jan 10 09:18:34 2013
@@ -17,8 +17,11 @@
 package org.apache.jackrabbit.oak.spi.query;
 
 import java.util.Deque;
+import java.util.HashSet;
 import java.util.Iterator;
 
+import javax.annotation.Nullable;
+
 import org.apache.jackrabbit.oak.commons.PathUtils;
 import org.apache.jackrabbit.oak.plugins.memory.MemoryChildNodeEntry;
 import org.apache.jackrabbit.oak.query.index.IndexRowImpl;
@@ -29,6 +32,7 @@ import org.apache.jackrabbit.oak.spi.sta
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
 
+import com.google.common.base.Predicate;
 import com.google.common.collect.Iterators;
 import com.google.common.collect.Queues;
 
@@ -47,11 +51,22 @@ public class Cursors {
     /**
      * Creates a {@link Cursor} over paths.
      *
-     * @param paths the paths to iterate over.
+     * @param paths the paths to iterate over (must return distinct paths)
      * @return the Cursor.
      */
     public static Cursor newPathCursor(Iterable<String> paths) {
-        return new PathCursor(paths);
+        return new PathCursor(paths, true);
+    }
+    
+    /**
+     * Creates a {@link Cursor} over paths, and make the result distinct.
+     * The iterator might return duplicate paths
+     * 
+     * @param paths the paths to iterate over (might contain duplicate entries)
+     * @return the Cursor.
+     */
+    public static Cursor newPathCursorDistinct(Iterable<String> paths) {
+        return new PathCursor(paths, true);
     }
 
     /**
@@ -88,8 +103,22 @@ public class Cursors {
 
         private final Iterator<String> iterator;
 
-        public PathCursor(Iterable<String> paths) {
-            this.iterator = paths.iterator();
+        public PathCursor(Iterable<String> paths, boolean distinct) {
+            Iterator<String> it = paths.iterator();
+            if (distinct) {
+                it = Iterators.filter(it, new Predicate<String>() {
+                    
+                    private final HashSet<String> known = new HashSet<String>();
+
+                    @Override
+                    public boolean apply(@Nullable String input) {
+                        // Set.add returns true for new entries
+                        return known.add(input);
+                    }
+                    
+                });
+            }
+            this.iterator = it;
         }
 
         @Override



Mime
View raw message