diff --git a/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/index/property/PropertyIndexLookup.java b/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/index/property/PropertyIndexLookup.java index ae18d9b..4bea618 --- a/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/index/property/PropertyIndexLookup.java +++ b/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/index/property/PropertyIndexLookup.java @@ -71,7 +71,7 @@ public class PropertyIndexLookup { /** Index storage strategy */ private static final IndexStoreStrategy MIRROR = - new ContentMirrorStoreStrategy(); + new ContentMirrorStoreStrategy(true); /** Index storage strategy */ private static final IndexStoreStrategy UNIQUE = @@ -129,7 +129,7 @@ public class PropertyIndexLookup { return Double.POSITIVE_INFINITY; } return COST_OVERHEAD + - getStrategy(indexMeta).count(indexMeta, encode(value), MAX_COST); + getStrategy(indexMeta).count(filter, indexMeta, encode(value), MAX_COST); } /** diff --git a/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/index/property/PropertyIndexPlan.java b/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/index/property/PropertyIndexPlan.java index 5cc398d..ebbd16a --- a/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/index/property/PropertyIndexPlan.java +++ b/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/index/property/PropertyIndexPlan.java @@ -66,7 +66,7 @@ public class PropertyIndexPlan { /** Index storage strategy */ private static final IndexStoreStrategy MIRROR = - new ContentMirrorStoreStrategy(); + new ContentMirrorStoreStrategy(true); /** Index storage strategy */ private static final IndexStoreStrategy UNIQUE = @@ -135,7 +135,7 @@ public class PropertyIndexPlan { if (restriction != null) { Set values = getValues(restriction); - double cost = strategy.count(definition, values, MAX_COST); + double cost = strategy.count(filter, definition, values, MAX_COST); if (cost < bestCost) { bestDepth = depth; bestValues = values; @@ -152,7 +152,7 @@ public class PropertyIndexPlan { if (constraint instanceof OrImpl) { Set values = findMultiProperty((OrImpl) constraint); if (values != null) { - double cost = strategy.count(definition, values, MAX_COST); + double cost = strategy.count(filter, definition, values, MAX_COST); if (cost < bestCost) { bestDepth = 1; bestValues = values; diff --git a/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/index/property/strategy/ContentMirrorStoreStrategy.java b/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/index/property/strategy/ContentMirrorStoreStrategy.java index be32205..81aac67 --- a/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/index/property/strategy/ContentMirrorStoreStrategy.java +++ b/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/index/property/strategy/ContentMirrorStoreStrategy.java @@ -69,6 +69,15 @@ import com.google.common.collect.Sets; public class ContentMirrorStoreStrategy implements IndexStoreStrategy { static final Logger LOG = LoggerFactory.getLogger(ContentMirrorStoreStrategy.class); + protected boolean isFilterAware; + + public ContentMirrorStoreStrategy() { + this (false); + } + + public ContentMirrorStoreStrategy (boolean isFilterAware) { + this.isFilterAware = isFilterAware; + } @Override public void update( @@ -118,16 +127,26 @@ public class ContentMirrorStoreStrategy implements IndexStoreStrategy { final NodeState indexMeta, final String indexStorageNodeName, final Iterable values) { final NodeState index = indexMeta.getChildNode(indexStorageNodeName); + final boolean shouldDescendDirectly = isFilterAware && filter.getPathRestriction().equals(Filter.PathRestriction.ALL_CHILDREN); return new Iterable() { @Override public Iterator iterator() { - PathIterator it = new PathIterator(filter, indexName); + PathIterator it = new PathIterator(filter, indexName, shouldDescendDirectly); if (values == null) { it.setPathContainsValue(true); it.enqueue(getChildNodeEntries(index).iterator()); } else { for (String p : values) { NodeState property = index.getChildNode(p); + if(shouldDescendDirectly) { + //Descend directly to path restriction inside index tree + String rootPath = filter.getPath(); + for (String pathFragment : PathUtils.elements(rootPath)) { + property = property.getChildNode(pathFragment); + if (!property.exists()) + break; + } + } if (property.exists()) { // we have an entry for this value, so use it it.enqueue(Iterators.singletonIterator( @@ -157,8 +176,18 @@ public class ContentMirrorStoreStrategy implements IndexStoreStrategy { return count(indexMeta, INDEX_CONTENT_NODE_NAME, values, max); } + @Override + public long count(final Filter filter, NodeState indexMeta, Set values, int max) { + return count(filter, indexMeta, INDEX_CONTENT_NODE_NAME, values, max); + } + public long count(NodeState indexMeta, final String indexStorageNodeName, Set values, int max) { + return count(null, indexMeta, indexStorageNodeName, values, max); + } + + public long count(Filter filter, NodeState indexMeta, final String indexStorageNodeName, + Set values, int max) { NodeState index = indexMeta.getChildNode(indexStorageNodeName); int count = 0; if (values == null) { @@ -196,6 +225,10 @@ public class ContentMirrorStoreStrategy implements IndexStoreStrategy { } max = Math.max(10, max / size); int i = 0; + String filterRootPath = null; + if(isFilterAware && filter!=null && filter.getPathRestriction().equals(Filter.PathRestriction.ALL_CHILDREN)) { + filterRootPath = filter.getPath(); + } for (String p : values) { if (count > max && i > 3) { // the total count is extrapolated from the the number @@ -204,6 +237,13 @@ public class ContentMirrorStoreStrategy implements IndexStoreStrategy { break; } NodeState s = index.getChildNode(p); + if (filterRootPath!=null && s.exists()) { + //Descend directly to path restriction inside index tree + for(String pathFragment:PathUtils.elements(filterRootPath)) { + s = s.getChildNode(pathFragment); + if (!s.exists())break; + } + } if (s.exists()) { CountingNodeVisitor v = new CountingNodeVisitor(max); v.visit(s); @@ -227,6 +267,7 @@ public class ContentMirrorStoreStrategy implements IndexStoreStrategy { private int readCount; private boolean init; private boolean closed; + private String rootPath; private String parentPath; private String currentPath; private boolean pathContainsValue; @@ -238,8 +279,13 @@ public class ContentMirrorStoreStrategy implements IndexStoreStrategy { private final long maxMemoryEntries; PathIterator(Filter filter, String indexName) { + this(filter, indexName, false); + } + + PathIterator(Filter filter, String indexName, boolean shouldDescendDirectly) { this.filter = filter; this.indexName = indexName; + this.rootPath = shouldDescendDirectly?PathUtils.relativize("/", filter.getPath()):""; parentPath = ""; currentPath = "/"; this.maxMemoryEntries = filter.getQueryEngineSettings().getLimitInMemory(); @@ -330,7 +376,7 @@ public class ContentMirrorStoreStrategy implements IndexStoreStrategy { fetchNext(); init = true; } - String result = currentPath; + String result = PathUtils.concat(rootPath, currentPath); fetchNext(); return result; } @@ -461,4 +507,4 @@ public class ContentMirrorStoreStrategy implements IndexStoreStrategy { } } } -} \ No newline at end of file +} diff --git a/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/index/property/strategy/IndexStoreStrategy.java b/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/index/property/strategy/IndexStoreStrategy.java index 5864eef..ef2273d --- a/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/index/property/strategy/IndexStoreStrategy.java +++ b/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/index/property/strategy/IndexStoreStrategy.java @@ -43,7 +43,7 @@ public interface IndexStoreStrategy { /** * Search for a given set of values. * - * @param filter the filter (used for logging) + * @param filter the filter (can optionally be used for optmized query execution) * @param indexName the name of the index (for logging) * @param indexMeta the index metadata node (may not be null) * @param values values to look for (null to check for property existence) @@ -62,4 +62,16 @@ public interface IndexStoreStrategy { */ long count(NodeState indexMeta, Set values, int max); + /** + * Count the occurrence of a given set of values. Used in calculating the + * cost of an index. + * + * @param filter the filter which can be used to estimate better cost + * @param indexMeta the index metadata node (may not be null) + * @param values values to look for (null to check for property existence) + * @param max the maximum value to return + * @return the aggregated count of occurrences for each provided value + */ + long count(Filter filter, NodeState indexMeta, Set values, int max); + } diff --git a/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/index/property/strategy/UniqueEntryStoreStrategy.java b/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/index/property/strategy/UniqueEntryStoreStrategy.java index 15111ec..ca80c7e --- a/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/index/property/strategy/UniqueEntryStoreStrategy.java +++ b/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/index/property/strategy/UniqueEntryStoreStrategy.java @@ -166,4 +166,8 @@ public class UniqueEntryStoreStrategy implements IndexStoreStrategy { return count; } + @Override + public long count(final Filter filter, NodeState indexMeta, Set values, int max) { + return count(indexMeta, values, max); + } } diff --git a/oak-core/src/test/java/org/apache/jackrabbit/oak/plugins/index/property/PropertyIndexTest.java b/oak-core/src/test/java/org/apache/jackrabbit/oak/plugins/index/property/PropertyIndexTest.java index 7a49f8f..e4f8500 --- a/oak-core/src/test/java/org/apache/jackrabbit/oak/plugins/index/property/PropertyIndexTest.java +++ b/oak-core/src/test/java/org/apache/jackrabbit/oak/plugins/index/property/PropertyIndexTest.java @@ -105,6 +105,59 @@ public class PropertyIndexTest { assertTrue("cost: " + cost, cost >= MANY); } + /** + * This is essentially same test as {@link #costEstimation()} with one difference that it uses + * path constraint in query and creates similar trees under 2 branches {@code path1} and {@code path2}. + * The cost estimation is then verified to be same as that in {@code costEstimation} for query under {@code path1} + * @throws Exception + */ + @Test + public void pathBasedCostEstimation() throws Exception { + NodeState root = INITIAL_CONTENT; + + // Add index definition + NodeBuilder builder = root.builder(); + createIndexDefinition(builder.child(INDEX_DEFINITIONS_NAME), "foo", + true, false, ImmutableSet.of("foo"), null); + NodeState before = builder.getNodeState(); + + NodeBuilder path1 = builder.child("path1"); + NodeBuilder path2 = builder.child("path2"); + // Add some content and process it through the property index hook + for (int i = 0; i < MANY; i++) { + path1.child("n" + i).setProperty("foo", "x" + i % 20); + path2.child("n" + i).setProperty("foo", "x" + i % 20); + } + NodeState after = builder.getNodeState(); + + NodeState indexed = HOOK.processCommit(before, after, CommitInfo.EMPTY); + + FilterImpl f = createFilter(indexed, NT_BASE); + f.restrictPath("/path1", Filter.PathRestriction.ALL_CHILDREN); + + // Query the index + PropertyIndexLookup lookup = new PropertyIndexLookup(indexed); + double cost; + + cost = lookup.getCost(f, "foo", PropertyValues.newString("x1")); + assertTrue("cost: " + cost, cost >= 6.5 && cost <= 7.5); + + cost = lookup.getCost(f, "foo", PropertyValues.newString( + Arrays.asList("x1", "x2"))); + assertTrue("cost: " + cost, cost >= 11.5 && cost <= 12.5); + + cost = lookup.getCost(f, "foo", PropertyValues.newString( + Arrays.asList("x1", "x2", "x3", "x4", "x5"))); + assertTrue("cost: " + cost, cost >= 26.5 && cost <= 27.5); + + cost = lookup.getCost(f, "foo", PropertyValues.newString( + Arrays.asList("x1", "x2", "x3", "x4", "x5", "x6", "x7", "x8", "x9", "x0"))); + assertTrue("cost: " + cost, cost >= 51.5 && cost <= 52.5); + + cost = lookup.getCost(f, "foo", null); + assertTrue("cost: " + cost, cost >= MANY); + } + @Test public void costMaxEstimation() throws Exception { NodeState root = EmptyNodeState.EMPTY_NODE; @@ -192,6 +245,32 @@ public class PropertyIndexTest { assertTrue("cost: " + cost, cost >= MANY); } + @Test + public void testPathAwarePropertyLookup() throws Exception { + NodeState root = INITIAL_CONTENT; + + // Add index definition + NodeBuilder builder = root.builder(); + createIndexDefinition(builder.child(INDEX_DEFINITIONS_NAME), "foo", + true, false, ImmutableSet.of("foo"), null); + NodeState before = builder.getNodeState(); + + // Add some content and process it through the property index hook + builder.child("a").setProperty("foo", "abc"); + builder.child("b").setProperty("foo", "abc"); + + NodeState after = builder.getNodeState(); + + NodeState indexed = HOOK.processCommit(before, after, CommitInfo.EMPTY); + + FilterImpl f = createFilter(indexed, NT_BASE); + f.restrictPath("/a", Filter.PathRestriction.ALL_CHILDREN); + + // Query the index + PropertyIndexLookup lookup = new PropertyIndexLookup(indexed); + assertEquals(ImmutableSet.of("a"), find(lookup, "foo", "abc", f)); + } + private static Set find(PropertyIndexLookup lookup, String name, String value, Filter filter) { return Sets.newHashSet(lookup.query(filter, name, value == null ? null