Author: catholicon Date: Tue Jun 25 22:49:38 2019 New Revision: 1862093 URL: http://svn.apache.org/viewvc?rev=1862093&view=rev Log: OAK-8437: direct children, exact, and parent path restrictions don't work when path transformation takes place
Added: jackrabbit/oak/trunk/oak-lucene/src/test/java/org/apache/jackrabbit/oak/plugins/index/lucene/LuceneIndexPathRestrictionTest.java (with props) Modified: 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/LucenePropertyIndexTest.java jackrabbit/oak/trunk/oak-search/src/main/java/org/apache/jackrabbit/oak/plugins/index/search/spi/query/FulltextIndexPlanner.java 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=1862093&r1=1862092&r2=1862093&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 Jun 25 22:49:38 2019 @@ -1054,12 +1054,21 @@ public class LucenePropertyIndex extends if (defn.evaluatePathRestrictions()) { BooleanQuery bq = new BooleanQuery(); bq.add(new BooleanClause(new TermQuery(newAncestorTerm(path)), BooleanClause.Occur.MUST)); - bq.add(new BooleanClause(newDepthQuery(path), BooleanClause.Occur.MUST)); + bq.add(new BooleanClause(newDepthQuery(path, planResult), BooleanClause.Occur.MUST)); qs.add(bq); } break; case EXACT: - qs.add(new TermQuery(newPathTerm(path))); + // For transformed paths, we can only add path restriction if absolute path to property can be + // deduced + if (planResult.isPathTransformed()) { + String parentPathSegment = planResult.getParentPathSegment(); + if ( ! Iterables.any(PathUtils.elements(parentPathSegment), "*"::equals)) { + qs.add(new TermQuery(newPathTerm(path + parentPathSegment))); + } + } else { + qs.add(new TermQuery(newPathTerm(path))); + } break; case PARENT: if (denotesRoot(path)) { @@ -1068,7 +1077,16 @@ public class LucenePropertyIndex extends // is no way to say "match no documents" in Lucene qs.add(new TermQuery(new Term(FieldNames.PATH, "///"))); } else { - qs.add(new TermQuery(newPathTerm(getParentPath(path)))); + // For transformed paths, we can only add path restriction if absolute path to property can be + // deduced + if (planResult.isPathTransformed()) { + String parentPathSegment = planResult.getParentPathSegment(); + if ( ! Iterables.any(PathUtils.elements(parentPathSegment), "*"::equals)) { + qs.add(new TermQuery(newPathTerm(getParentPath(path) + parentPathSegment))); + } + } else { + qs.add(new TermQuery(newPathTerm(getParentPath(path)))); + } } break; case NO_RESTRICTION: @@ -1505,8 +1523,8 @@ public class LucenePropertyIndex extends } } - private static Query newDepthQuery(String path) { - int depth = PathUtils.getDepth(path) + 1; + private static Query newDepthQuery(String path, PlanResult planResult) { + int depth = PathUtils.getDepth(path) + planResult.getParentDepth() + 1; return NumericRangeQuery.newIntRange(FieldNames.PATH_DEPTH, depth, depth, true, true); } Added: jackrabbit/oak/trunk/oak-lucene/src/test/java/org/apache/jackrabbit/oak/plugins/index/lucene/LuceneIndexPathRestrictionTest.java URL: http://svn.apache.org/viewvc/jackrabbit/oak/trunk/oak-lucene/src/test/java/org/apache/jackrabbit/oak/plugins/index/lucene/LuceneIndexPathRestrictionTest.java?rev=1862093&view=auto ============================================================================== --- jackrabbit/oak/trunk/oak-lucene/src/test/java/org/apache/jackrabbit/oak/plugins/index/lucene/LuceneIndexPathRestrictionTest.java (added) +++ jackrabbit/oak/trunk/oak-lucene/src/test/java/org/apache/jackrabbit/oak/plugins/index/lucene/LuceneIndexPathRestrictionTest.java Tue Jun 25 22:49:38 2019 @@ -0,0 +1,280 @@ +/* + * 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 com.google.common.collect.Sets; +import org.apache.jackrabbit.oak.InitialContentHelper; +import org.apache.jackrabbit.oak.plugins.index.IndexConstants; +import org.apache.jackrabbit.oak.plugins.index.IndexUpdateProvider; +import org.apache.jackrabbit.oak.plugins.index.lucene.util.IndexDefinitionBuilder; +import org.apache.jackrabbit.oak.plugins.memory.PropertyValues; +import org.apache.jackrabbit.oak.query.NodeStateNodeTypeInfoProvider; +import org.apache.jackrabbit.oak.query.QueryEngineSettings; +import org.apache.jackrabbit.oak.query.ast.NodeTypeInfo; +import org.apache.jackrabbit.oak.query.ast.NodeTypeInfoProvider; +import org.apache.jackrabbit.oak.query.ast.Operator; +import org.apache.jackrabbit.oak.query.ast.SelectorImpl; +import org.apache.jackrabbit.oak.query.index.FilterImpl; +import org.apache.jackrabbit.oak.spi.commit.EditorHook; +import org.apache.jackrabbit.oak.spi.query.Cursor; +import org.apache.jackrabbit.oak.spi.query.Filter; +import org.apache.jackrabbit.oak.spi.query.QueryIndex; +import org.apache.jackrabbit.oak.spi.state.NodeBuilder; +import org.apache.jackrabbit.oak.spi.state.NodeState; +import org.junit.Before; +import org.junit.Test; + +import java.util.List; +import java.util.Set; + +import static com.google.common.collect.ImmutableSet.of; +import static org.apache.jackrabbit.JcrConstants.NT_BASE; +import static org.apache.jackrabbit.oak.spi.commit.CommitInfo.EMPTY; +import static org.junit.Assert.assertEquals; + +public class LuceneIndexPathRestrictionTest { + private static final EditorHook HOOK = new EditorHook( + new IndexUpdateProvider(new LuceneIndexEditorProvider())); + + private NodeState root; + private NodeBuilder rootBuilder; + private IndexTracker tracker; + private LucenePropertyIndex index; + + @Before + public void setup() throws Exception { + root = InitialContentHelper.INITIAL_CONTENT; + rootBuilder = root.builder(); + + tracker = new IndexTracker(); + tracker.update(root); + index = new LucenePropertyIndex(tracker); + + // remove any indexes (that cause commits to fail due to missing provider) + rootBuilder.child(IndexConstants.INDEX_DEFINITIONS_NAME).remove(); + + commit(); + } + + @Test + public void pathTranformationWithNoPathRestriction() throws Exception { + IndexDefinitionBuilder idxBuilder = + new IndexDefinitionBuilder(rootBuilder.child(IndexConstants.INDEX_DEFINITIONS_NAME).child("fooIndex")) + .noAsync().evaluatePathRestrictions(); + idxBuilder.indexRule("nt:base").property("foo").propertyIndex(); + idxBuilder.build(); + commit(); + + NodeBuilder testRootBuilder = rootBuilder.child("test"); + testRootBuilder.child("a").child("j:c").setProperty("foo", "bar"); + testRootBuilder.child("b").setProperty("foo", "bar"); + testRootBuilder.child("c").child("d").child("j:c").setProperty("foo", "bar"); + commit(); + + FilterImpl f; + + // //*[j:c/foo = 'bar'] -> foo:bar -> transform 1 level up + f = createFilter(root, NT_BASE); + f.restrictProperty("j:c/foo", Operator.EQUAL, PropertyValues.newString("bar")); + validateResult(f, of("/test", "/test/a", "/test/c/d")); + + // //*[*/foo = 'bar'] -> foo:bar -> transform 1 level up + f = createFilter(root, NT_BASE); + f.restrictProperty("*/foo", Operator.EQUAL, PropertyValues.newString("bar")); + validateResult(f, of("/test/a", "/test", "/test/c/d")); + + // //*[d/*/foo = 'bar'] -> foo:bar -> transform 2 level up + f = createFilter(root, NT_BASE); + f.restrictProperty("d/*/foo", Operator.EQUAL, PropertyValues.newString("bar")); + validateResult(f, of("/", "/test", "/test/c")); + } + + @Test + public void pathTranformationWithAllChildrenPathRestriction() throws Exception { + IndexDefinitionBuilder idxBuilder = + new IndexDefinitionBuilder(rootBuilder.child(IndexConstants.INDEX_DEFINITIONS_NAME).child("fooIndex")) + .noAsync().evaluatePathRestrictions(); + idxBuilder.indexRule("nt:base").property("foo").propertyIndex(); + idxBuilder.build(); + commit(); + + NodeBuilder testRootBuilder = rootBuilder.child("test"); + testRootBuilder.child("a").child("j:c").setProperty("foo", "bar"); + testRootBuilder.child("b").setProperty("foo", "bar"); + testRootBuilder.child("c").child("d").child("j:c").setProperty("foo", "bar"); + commit(); + + FilterImpl f; + + // /jcr:root/test//*[j:c/foo = 'bar'] -> foo:bar :ancestors:/test -> transform 1 level up + f = createFilter(root, NT_BASE); + f.restrictProperty("j:c/foo", Operator.EQUAL, PropertyValues.newString("bar")); + f.restrictPath("/test", Filter.PathRestriction.ALL_CHILDREN); + validateResult(f, of("/test/a", "/test/c/d")); + + // /jcr:root/test//*[*/foo = 'bar'] -> foo:bar :ancestors:/test -> transform 1 level up + f = createFilter(root, NT_BASE); + f.restrictProperty("*/foo", Operator.EQUAL, PropertyValues.newString("bar")); + f.restrictPath("/test", Filter.PathRestriction.ALL_CHILDREN); + validateResult(f, of("/test/a", "/test/c/d")); + + // /jcr:root/test//*[d/*/foo = 'bar'] -> foo:bar :ancestors:/test -> transform 2 level up + f = createFilter(root, NT_BASE); + f.restrictProperty("d/*/foo", Operator.EQUAL, PropertyValues.newString("bar")); + f.restrictPath("/test", Filter.PathRestriction.ALL_CHILDREN); + validateResult(f, of("/test/c")); + } + + @Test + public void pathTranformationWithDirectChildrenPathRestriction() throws Exception { + IndexDefinitionBuilder idxBuilder = + new IndexDefinitionBuilder(rootBuilder.child(IndexConstants.INDEX_DEFINITIONS_NAME).child("fooIndex")) + .noAsync().evaluatePathRestrictions(); + idxBuilder.indexRule("nt:base").property("foo").propertyIndex(); + idxBuilder.build(); + commit(); + + NodeBuilder testRootBuilder = rootBuilder.child("test"); + testRootBuilder.child("a").child("j:c").setProperty("foo", "bar"); + testRootBuilder.child("b").setProperty("foo", "bar"); + testRootBuilder.child("c").child("d").child("j:c").setProperty("foo", "bar"); + commit(); + + FilterImpl f; + + // /jcr:root/test/*[j:c/foo = 'bar'] -> foo:bar :ancestors:/test :depth:[3 TO 3] -> transform 1 level up + f = createFilter(root, NT_BASE); + f.restrictProperty("j:c/foo", Operator.EQUAL, PropertyValues.newString("bar")); + f.restrictPath("/test", Filter.PathRestriction.DIRECT_CHILDREN); + validateResult(f, of("/test/a")); + + // /jcr:root/test/*[*/foo = 'bar'] -> foo:bar :ancestors:/test :depth:[3 TO 3] -> transform 1 level up + f = createFilter(root, NT_BASE); + f.restrictProperty("*/foo", Operator.EQUAL, PropertyValues.newString("bar")); + f.restrictPath("/test", Filter.PathRestriction.DIRECT_CHILDREN); + validateResult(f, of("/test/a")); + + // /jcr:root/test/*[d/*/foo = 'bar'] -> foo:bar :ancestors:/test :depth:[4 TO 4] -> transform 2 level up + f = createFilter(root, NT_BASE); + f.restrictProperty("d/*/foo", Operator.EQUAL, PropertyValues.newString("bar")); + f.restrictPath("/test", Filter.PathRestriction.DIRECT_CHILDREN); + validateResult(f, of("/test/c")); + } + + @Test + public void pathTranformationWithExactPathRestriction() throws Exception { + IndexDefinitionBuilder idxBuilder = + new IndexDefinitionBuilder(rootBuilder.child(IndexConstants.INDEX_DEFINITIONS_NAME).child("fooIndex")) + .noAsync().evaluatePathRestrictions(); + idxBuilder.indexRule("nt:base").property("foo").propertyIndex(); + idxBuilder.build(); + commit(); + + NodeBuilder testRootBuilder = rootBuilder.child("test"); + testRootBuilder.child("a").child("j:c").setProperty("foo", "bar"); + testRootBuilder.child("b").setProperty("foo", "bar"); + testRootBuilder.child("c").child("d").child("j:c").setProperty("foo", "bar"); + commit(); + + FilterImpl f; + + // /jcr:root/test/a[j:c/foo = 'bar'] -> foo:bar :path:/test/a/j:c -> transform 1 level up + f = createFilter(root, NT_BASE); + f.restrictProperty("j:c/foo", Operator.EQUAL, PropertyValues.newString("bar")); + f.restrictPath("/test/a", Filter.PathRestriction.EXACT); + validateResult(f, of("/test/a")); + + // /jcr:root/test/a[*/foo = 'bar'] -> foo:bar -> transform 1 level up + filter path restriction + f = createFilter(root, NT_BASE); + f.restrictProperty("*/foo", Operator.EQUAL, PropertyValues.newString("bar")); + f.restrictPath("/test/a", Filter.PathRestriction.EXACT); + validateResult(f, of("/test/a")); + + // /jcr:root/test/c[d/*/foo = 'bar'] -> foo:bar -> transform 2 level up + filter path restriction + f = createFilter(root, NT_BASE); + f.restrictProperty("d/*/foo", Operator.EQUAL, PropertyValues.newString("bar")); + f.restrictPath("/test/c", Filter.PathRestriction.EXACT); + validateResult(f, of("/test/c")); + } + + @Test + public void pathTranformationWithParentFilter() throws Exception { + IndexDefinitionBuilder idxBuilder = + new IndexDefinitionBuilder(rootBuilder.child(IndexConstants.INDEX_DEFINITIONS_NAME).child("fooIndex")) + .noAsync().evaluatePathRestrictions(); + idxBuilder.indexRule("nt:base").property("foo").propertyIndex(); + idxBuilder.build(); + commit(); + + NodeBuilder testRootBuilder = rootBuilder.child("test"); + testRootBuilder.child("a").child("j:c").setProperty("foo", "bar"); + testRootBuilder.child("b").setProperty("foo", "bar"); + testRootBuilder.child("c").child("d").child("j:c").setProperty("foo", "bar"); + commit(); + + FilterImpl f; + + // /jcr:root/test/a/b/j:c/..[j:c/foo = 'bar'] -> foo:bar :path:/test/a/b/j:c -> transform 1 level up + f = createFilter(root, NT_BASE); + f.restrictProperty("j:c/foo", Operator.EQUAL, PropertyValues.newString("bar")); + f.restrictPath("/test/c/d/j:c", Filter.PathRestriction.PARENT); + validateResult(f, of("/test/c/d")); + + // /jcr:root/test/a/b/j:c/..[*/foo = 'bar'] -> foo:bar -> transform 1 level up + f = createFilter(root, NT_BASE); + f.restrictProperty("*/foo", Operator.EQUAL, PropertyValues.newString("bar")); + f.restrictPath("/test/a/b/j:c", Filter.PathRestriction.PARENT); + validateResult(f, of("/test", "/test/a", "/test/c/d")); + + // /jcr:root/test/c/d/..[d/*/foo = 'bar'] -> foo:bar -> transform 2 level up + f = createFilter(root, NT_BASE); + f.restrictProperty("d/*/foo", Operator.EQUAL, PropertyValues.newString("bar")); + f.restrictPath("/test/c/d", Filter.PathRestriction.PARENT); + validateResult(f, of("/", "/test", "/test/c")); + } + + private void validateResult(Filter f, Set<String> expected) { + List<QueryIndex.IndexPlan> plans = index.getPlans(f, null, root); + + assertEquals("Only one plan must show up", 1, plans.size()); + + QueryIndex.IndexPlan plan = plans.get(0); + + Cursor cursor = index.query(plan, root); + Set<String> paths = Sets.newHashSet(); + + while (cursor.hasNext()) { + paths.add(cursor.next().getPath()); + } + + assertEquals(f.toString(), expected, paths); + } + + private void commit() throws Exception { + root = HOOK.processCommit(rootBuilder.getBaseState(), rootBuilder.getNodeState(), EMPTY); + rootBuilder = root.builder(); + + tracker.update(root); + } + + private static FilterImpl createFilter(NodeState root, String nodeTypeName) { + NodeTypeInfoProvider nodeTypes = new NodeStateNodeTypeInfoProvider(root); + NodeTypeInfo type = nodeTypes.getNodeTypeInfo(nodeTypeName); + SelectorImpl selector = new SelectorImpl(type, nodeTypeName); + return new FilterImpl(selector, "SELECT * FROM [" + nodeTypeName + "]", new QueryEngineSettings()); + } +} Propchange: jackrabbit/oak/trunk/oak-lucene/src/test/java/org/apache/jackrabbit/oak/plugins/index/lucene/LuceneIndexPathRestrictionTest.java ------------------------------------------------------------------------------ svn:eol-style = native Modified: 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=1862093&r1=1862092&r2=1862093&view=diff ============================================================================== --- jackrabbit/oak/trunk/oak-lucene/src/test/java/org/apache/jackrabbit/oak/plugins/index/lucene/LucenePropertyIndexTest.java (original) +++ jackrabbit/oak/trunk/oak-lucene/src/test/java/org/apache/jackrabbit/oak/plugins/index/lucene/LucenePropertyIndexTest.java Tue Jun 25 22:49:38 2019 @@ -3330,6 +3330,59 @@ public class LucenePropertyIndexTest ext "lucene:fooIndex(/oak:index/fooIndex)", asList("/d")); } + @Test + public void pathTransformationWithEvaluatePathRestriction() throws Exception { + // This test might feel redundant in presence of LuceneIndexPathRestrictionTest, BUT, this test + // gets result via query engine so the results is exactly correct. While in LuceneIndexPathRestrictionTest + // we're interested in how many constraint could the index resolve.. so those results would be super-set of + // what we see here + + IndexDefinitionBuilder idxBuilder = new IndexDefinitionBuilder() + .noAsync().evaluatePathRestrictions(); + idxBuilder.indexRule("nt:base").property("foo").propertyIndex(); + Tree idx = root.getTree("/").getChild("oak:index").addChild("fooIndex"); + idxBuilder.build(idx); + root.commit(); + + Tree rootTree = root.getTree("/").addChild("test"); + rootTree.addChild("a").addChild("j:c").setProperty("foo", "bar"); + rootTree.addChild("b").setProperty("foo", "bar"); + rootTree.addChild("c").addChild("d").addChild("j:c").setProperty("foo", "bar"); + root.commit(); + + // no path restriction + assertPlanAndQueryXPath("//*[j:c/@foo = 'bar']", + "lucene:fooIndex(/oak:index/fooIndex)", asList("/test/a", "/test/c/d")); + assertPlanAndQueryXPath("//*[*/@foo = 'bar']", + "lucene:fooIndex(/oak:index/fooIndex)", asList("/test/a", "/test", "/test/c/d")); + assertPlanAndQueryXPath("//*[d/*/@foo = 'bar']", + "lucene:fooIndex(/oak:index/fooIndex)", asList("/test/c")); + + // any descendant + assertPlanAndQueryXPath("/jcr:root/test//*[j:c/@foo = 'bar']", + "lucene:fooIndex(/oak:index/fooIndex)", asList("/test/a", "/test/c/d")); + assertPlanAndQueryXPath("/jcr:root/test//*[*/@foo = 'bar']", + "lucene:fooIndex(/oak:index/fooIndex)", asList("/test/a", "/test/c/d")); + assertPlanAndQueryXPath("/jcr:root/test//*[d/*/@foo = 'bar']", + "lucene:fooIndex(/oak:index/fooIndex)", asList("/test/c")); + + // direct children + assertPlanAndQueryXPath("/jcr:root/test/*[j:c/@foo = 'bar']", + "lucene:fooIndex(/oak:index/fooIndex)", asList("/test/a")); + assertPlanAndQueryXPath("/jcr:root/test/*[*/@foo = 'bar']", + "lucene:fooIndex(/oak:index/fooIndex)", asList("/test/a")); + assertPlanAndQueryXPath("/jcr:root/test/*[d/*/@foo = 'bar']", + "lucene:fooIndex(/oak:index/fooIndex)", asList("/test/c")); + + // exact path + assertPlanAndQueryXPath("/jcr:root/test/a[j:c/@foo = 'bar']", + "lucene:fooIndex(/oak:index/fooIndex)", asList("/test/a")); + assertPlanAndQueryXPath("/jcr:root/test/a[*/@foo = 'bar']", + "lucene:fooIndex(/oak:index/fooIndex)", asList("/test/a")); + assertPlanAndQueryXPath("/jcr:root/test/c[d/*/@foo = 'bar']", + "lucene:fooIndex(/oak:index/fooIndex)", asList("/test/c")); + } + private void assertPlanAndQueryXPath(String query, String planExpectation, List<String> paths) throws ParseException { assertXpathPlan(query, planExpectation); assertQuery(query, XPATH, paths, false); Modified: jackrabbit/oak/trunk/oak-search/src/main/java/org/apache/jackrabbit/oak/plugins/index/search/spi/query/FulltextIndexPlanner.java URL: http://svn.apache.org/viewvc/jackrabbit/oak/trunk/oak-search/src/main/java/org/apache/jackrabbit/oak/plugins/index/search/spi/query/FulltextIndexPlanner.java?rev=1862093&r1=1862092&r2=1862093&view=diff ============================================================================== --- jackrabbit/oak/trunk/oak-search/src/main/java/org/apache/jackrabbit/oak/plugins/index/search/spi/query/FulltextIndexPlanner.java (original) +++ jackrabbit/oak/trunk/oak-search/src/main/java/org/apache/jackrabbit/oak/plugins/index/search/spi/query/FulltextIndexPlanner.java Tue Jun 25 22:49:38 2019 @@ -1012,6 +1012,14 @@ public class FulltextIndexPlanner { return relativize; } + public int getParentDepth() { + return parentDepth; + } + + public String getParentPathSegment() { + return parentPathSegment; + } + public boolean isUniquePathsRequired() { return uniquePathsRequired; }