jackrabbit-oak-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From thom...@apache.org
Subject svn commit: r1429882 - in /jackrabbit/oak/trunk/oak-core/src: main/java/org/apache/jackrabbit/oak/plugins/index/p2/Property2IndexLookup.java test/java/org/apache/jackrabbit/oak/plugins/index/p2/Property2IndexTest.java
Date Mon, 07 Jan 2013 16:27:43 GMT
Author: thomasm
Date: Mon Jan  7 16:27:43 2013
New Revision: 1429882

URL: http://svn.apache.org/viewvc?rev=1429882&view=rev
Log:
OAK-537 The Property2Index eagerly and unnecessarily fetches all data: remove unused code
(the case where there is no index); make methods static when possible; use PathUtils instead
of parsing the path itself

Modified:
    jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/index/p2/Property2IndexLookup.java
    jackrabbit/oak/trunk/oak-core/src/test/java/org/apache/jackrabbit/oak/plugins/index/p2/Property2IndexTest.java

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=1429882&r1=1429881&r2=1429882&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
Mon Jan  7 16:27:43 2013
@@ -18,6 +18,7 @@ package org.apache.jackrabbit.oak.plugin
 
 import static org.apache.jackrabbit.oak.plugins.index.IndexConstants.INDEX_DEFINITIONS_NAME;
 
+import java.util.Iterator;
 import java.util.Set;
 
 import javax.annotation.Nullable;
@@ -25,6 +26,7 @@ import javax.annotation.Nullable;
 import org.apache.jackrabbit.oak.api.PropertyState;
 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.IndexConstants;
 import org.apache.jackrabbit.oak.plugins.index.p2.strategy.ContentMirrorStoreStrategy;
 import org.apache.jackrabbit.oak.plugins.index.p2.strategy.IndexStoreStrategy;
@@ -72,24 +74,24 @@ public class Property2IndexLookup {
      * @return true if the property is indexed
      */
     public boolean isIndexed(String name, String path) {
-        if (getIndexDefinitionNode(name) != null) {
-            return true;
-        }
-
-        // TODO use PathUtils
-        if (path.startsWith("/")) {
-            path = path.substring(1);
-        }
-        int slash = path.indexOf('/');
-        if (slash == -1) {
-            return false;
+        return isIndexed(root, name, path);
+    }
+    
+    private static boolean isIndexed(NodeState root, String name, String path) {
+        NodeState node = root;
+        Iterator<String> it = PathUtils.elements(path).iterator();
+        while (true) {
+            if (getIndexDefinitionNode(node, name) != null) {
+                return true;
+            }
+            if (!it.hasNext()) {
+                break;
+            }
+            node = node.getChildNode(it.next());
         }
-
-        NodeState child = root.getChildNode(path.substring(0, slash));
-        return new Property2IndexLookup(child).isIndexed(
-                name, path.substring(slash));
+        return false;
     }
-
+    
     /**
      * Searches for a given <code>String<code> value within this index.
      * 
@@ -112,69 +114,33 @@ public class Property2IndexLookup {
      * @return the set of matched paths
      */
     public Set<String> find(String name, PropertyValue value) {
+        NodeState state = getIndexDefinitionNode(root, name);
+        if (state == null || state.getChildNode(":index") == null) {
+            throw new IllegalArgumentException("No index for " + name);
+        }
         Set<String> paths = Sets.newHashSet();
-
-        NodeState state = getIndexDefinitionNode(name);
-        if (state != null && state.getChildNode(":index") != null) {
-            state = state.getChildNode(":index");
-            if (value == null) {
-                paths.addAll(store.find(state, null));
-            } else {
-                paths.addAll(store.find(state, Property2Index.encode(value)));
-            }
+        state = state.getChildNode(":index");
+        if (value == null) {
+            paths.addAll(store.find(state, null));
         } else {
-            // No index available, so first check this node for a match
-            PropertyState property = root.getProperty(name);
-            if (property != null) {
-                if (value == null || value.isArray()) {
-                    // let query engine handle property existence and
-                    // multi-valued look ups;
-                    // simply return all nodes that have this property
-                    paths.add("");
-                } else {
-                    // does it match any of the values of this property?
-                    for (int i = 0; i < property.count(); i++) {
-                        if (property.getValue(value.getType(), i).equals(value.getValue(value.getType())))
{
-                            paths.add("");
-                            // no need to check for more matches in this property
-                            break;
-                        }
-                    }
-                }
-            }
-
-            // ... and then recursively look up from the rest of the tree
-            for (ChildNodeEntry entry : root.getChildNodeEntries()) {
-                String base = entry.getName();
-                Property2IndexLookup lookup =
-                        new Property2IndexLookup(entry.getNodeState());
-                for (String path : lookup.find(name, value)) {
-                    if (path.isEmpty()) {
-                        paths.add(base);
-                    } else {
-                        paths.add(base + "/" + path);
-                    }
-                }
-            }
+            paths.addAll(store.find(state, Property2Index.encode(value)));
         }
-
         return paths;
     }
 
     public double getCost(String name, PropertyValue value) {
-        double cost = 0.0;
         // TODO the cost method is currently reading all the data - 
         // is not supposed to do that, it is only supposed to estimate
-        NodeState state = getIndexDefinitionNode(name);
-        if (state != null && state.getChildNode(":index") != null) {
-            state = state.getChildNode(":index");
-            if (value == null) {
-                cost += store.count(state, null);
-            } else {
-                cost += store.count(state, Property2Index.encode(value));
-            }
+        NodeState state = getIndexDefinitionNode(root, name);
+        if (state == null || state.getChildNode(":index") == null) {
+            return Double.POSITIVE_INFINITY;
+        }
+        state = state.getChildNode(":index");
+        double cost;
+        if (value == null) {
+            cost = store.count(state, null);
         } else {
-            cost = Double.POSITIVE_INFINITY;
+            cost = store.count(state, Property2Index.encode(value));
         }
         return cost;
     }
@@ -187,8 +153,8 @@ public class Property2IndexLookup {
      *         index definition node was found
      */
     @Nullable
-    private NodeState getIndexDefinitionNode(String name) {
-        NodeState state = root.getChildNode(INDEX_DEFINITIONS_NAME);
+    private static NodeState getIndexDefinitionNode(NodeState node, String name) {
+        NodeState state = node.getChildNode(INDEX_DEFINITIONS_NAME);
         if (state != null) {
             for (ChildNodeEntry entry : state.getChildNodeEntries()) {
                 PropertyState type = entry.getNodeState().getProperty(IndexConstants.TYPE_PROPERTY_NAME);

Modified: jackrabbit/oak/trunk/oak-core/src/test/java/org/apache/jackrabbit/oak/plugins/index/p2/Property2IndexTest.java
URL: http://svn.apache.org/viewvc/jackrabbit/oak/trunk/oak-core/src/test/java/org/apache/jackrabbit/oak/plugins/index/p2/Property2IndexTest.java?rev=1429882&r1=1429881&r2=1429882&view=diff
==============================================================================
--- jackrabbit/oak/trunk/oak-core/src/test/java/org/apache/jackrabbit/oak/plugins/index/p2/Property2IndexTest.java
(original)
+++ jackrabbit/oak/trunk/oak-core/src/test/java/org/apache/jackrabbit/oak/plugins/index/p2/Property2IndexTest.java
Mon Jan  7 16:27:43 2013
@@ -27,7 +27,11 @@ import org.apache.jackrabbit.oak.spi.sta
 import org.junit.Test;
 
 import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.fail;
 
+/**
+ * Test the Property2 index mechanism.
+ */
 public class Property2IndexTest {
 
     private static final int MANY = 100;
@@ -54,31 +58,19 @@ public class Property2IndexTest {
         }
         NodeState after = builder.getNodeState();
 
-        // First check lookups without an index
-        Property2IndexLookup lookup = new Property2IndexLookup(after);
-        long withoutIndex = System.nanoTime();
-        assertEquals(ImmutableSet.of("a", "b"), lookup.find("foo", "abc"));
-        assertEquals(ImmutableSet.of("b"), lookup.find("foo", "def"));
-        assertEquals(ImmutableSet.of(), lookup.find("foo", "ghi"));
-        assertEquals(MANY, lookup.find("foo", "xyz").size());
-        withoutIndex = System.nanoTime() - withoutIndex;
-
-        // ... then see how adding an index affects the code
+        // Add an index
         IndexHook p = new Property2IndexDiff(builder);
         after.compareAgainstBaseState(before, p);
         p.apply();
         p.close();
 
-        lookup = new Property2IndexLookup(builder.getNodeState());
-        long withIndex = System.nanoTime();
+        // Query the index
+        Property2IndexLookup lookup = new Property2IndexLookup(builder.getNodeState());
         assertEquals(ImmutableSet.of("a", "b"), lookup.find("foo", "abc"));
         assertEquals(ImmutableSet.of("b"), lookup.find("foo", "def"));
         assertEquals(ImmutableSet.of(), lookup.find("foo", "ghi"));
         assertEquals(MANY, lookup.find("foo", "xyz").size());
-        withIndex = System.nanoTime() - withIndex;
-
-        // System.out.println("Index performance ratio: " + withoutIndex/withIndex);
-        // assertTrue(withoutIndex > withIndex);
+        
     }
 
     @Test
@@ -103,35 +95,27 @@ public class Property2IndexTest {
         }
         NodeState after = builder.getNodeState();
 
-        // First check lookups without an index
-        Property2IndexLookup lookup = new Property2IndexLookup(after);
-        long withoutIndex = System.nanoTime();
-        assertEquals(ImmutableSet.of("a", "b"), lookup.find("foo", "abc"));
-        assertEquals(ImmutableSet.of("b"), lookup.find("foo", "def"));
-        assertEquals(ImmutableSet.of(), lookup.find("foo", "ghi"));
-        assertEquals(MANY, lookup.find("foo", "xyz").size());
-        assertEquals(ImmutableSet.of("a"), lookup.find("extrafoo", "pqr"));
-        assertEquals(ImmutableSet.of(), lookup.find("pqr", "foo"));
-        withoutIndex = System.nanoTime() - withoutIndex;
-
-        // ... then see how adding an index affects the code
+        // Add an index
         IndexHook p = new Property2IndexDiff(builder);
         after.compareAgainstBaseState(before, p);
         p.apply();
         p.close();
 
-        lookup = new Property2IndexLookup(builder.getNodeState());
-        long withIndex = System.nanoTime();
+        // Query the index
+        Property2IndexLookup lookup = new Property2IndexLookup(builder.getNodeState());
         assertEquals(ImmutableSet.of("a", "b"), lookup.find("foo", "abc"));
         assertEquals(ImmutableSet.of("b"), lookup.find("foo", "def"));
         assertEquals(ImmutableSet.of(), lookup.find("foo", "ghi"));
         assertEquals(MANY, lookup.find("foo", "xyz").size());
         assertEquals(ImmutableSet.of("a"), lookup.find("extrafoo", "pqr"));
-        assertEquals(ImmutableSet.of(), lookup.find("pqr", "foo"));
-        withIndex = System.nanoTime() - withIndex;
+        
+        try {
+            assertEquals(ImmutableSet.of(), lookup.find("pqr", "foo"));
+            fail();
+        } catch (IllegalArgumentException e) {
+            // expected: no index for "pqr"
+        }
 
-        // System.out.println("Index performance ratio: " + withoutIndex/withIndex);
-        // assertTrue(withoutIndex > withIndex);
     }
 
 



Mime
View raw message