Author: catholicon Date: Thu May 2 13:57:43 2019 New Revision: 1858544 URL: http://svn.apache.org/viewvc?rev=1858544&view=rev Log: OAK-8166: Index definition with orderable property definitions with and without functions breaks index
Committing fix contributed by Nitin Gupta (@nitigup). Modified: jackrabbit/oak/trunk/oak-lucene/src/test/java/org/apache/jackrabbit/oak/plugins/index/lucene/FunctionIndexTest.java jackrabbit/oak/trunk/oak-search/src/main/java/org/apache/jackrabbit/oak/plugins/index/search/Aggregate.java jackrabbit/oak/trunk/oak-search/src/main/java/org/apache/jackrabbit/oak/plugins/index/search/IndexDefinition.java Modified: jackrabbit/oak/trunk/oak-lucene/src/test/java/org/apache/jackrabbit/oak/plugins/index/lucene/FunctionIndexTest.java URL: http://svn.apache.org/viewvc/jackrabbit/oak/trunk/oak-lucene/src/test/java/org/apache/jackrabbit/oak/plugins/index/lucene/FunctionIndexTest.java?rev=1858544&r1=1858543&r2=1858544&view=diff ============================================================================== --- jackrabbit/oak/trunk/oak-lucene/src/test/java/org/apache/jackrabbit/oak/plugins/index/lucene/FunctionIndexTest.java (original) +++ jackrabbit/oak/trunk/oak-lucene/src/test/java/org/apache/jackrabbit/oak/plugins/index/lucene/FunctionIndexTest.java Thu May 2 13:57:43 2019 @@ -35,6 +35,7 @@ import java.util.Set; import javax.jcr.PropertyType; +import ch.qos.logback.classic.Level; import org.apache.jackrabbit.JcrConstants; import org.apache.jackrabbit.oak.Oak; import org.apache.jackrabbit.oak.api.ContentRepository; @@ -42,6 +43,7 @@ import org.apache.jackrabbit.oak.api.Res import org.apache.jackrabbit.oak.api.ResultRow; import org.apache.jackrabbit.oak.api.Tree; import org.apache.jackrabbit.oak.api.Type; +import org.apache.jackrabbit.oak.commons.junit.LogCustomizer; import org.apache.jackrabbit.oak.plugins.index.lucene.util.IndexDefinitionBuilder; import org.apache.jackrabbit.oak.plugins.index.nodetype.NodeTypeIndexProvider; import org.apache.jackrabbit.oak.plugins.index.property.PropertyIndexEditorProvider; @@ -54,6 +56,7 @@ import org.apache.jackrabbit.oak.spi.com import org.apache.jackrabbit.oak.spi.query.QueryIndexProvider; import org.apache.jackrabbit.oak.spi.security.OpenSecurityProvider; import org.apache.jackrabbit.oak.spi.state.NodeStore; +import org.junit.Assert; import org.junit.Test; import com.google.common.collect.Iterables; @@ -339,6 +342,228 @@ public class FunctionIndexTest extends A "lucene:test1(/oak:index/test1)", asList("/b", "/c", "/a")); } + /* + <OAK-8166> + Given an index def with 2 orderable property definitions(Relative) for same property - one with function and one without + Indexer should not fail to index the nodes covered by this index + */ + @Test + public void sameOrderableRelativePropertyWithAndWithoutFunction() throws Exception { + + LogCustomizer customLogs = LogCustomizer.forLogger(LuceneIndexEditor.class.getName()).enable(Level.WARN).create(); + // Create nodes that will be served by the index definition that follows + Tree test = root.getTree("/").addChild("test"); + test.setProperty("jcr:primaryType", "nt:unstructured", Type.NAME); + + Tree a = test.addChild("jcr:content"); + a.setProperty("jcr:primaryType", "nt:unstructured", Type.NAME); + + Tree b = a.addChild("n"); + + b.setProperty("jcr:primaryType", "nt:unstructured", Type.NAME); + b.setProperty("foo", "bar"); + + root.commit(); + + // Index def with same property - ordered - one with function and one without + Tree luceneIndex = createIndex("upper", Collections.<String>emptySet()); + Tree nonFunc = luceneIndex.addChild(FulltextIndexConstants.INDEX_RULES) + .addChild("nt:unstructured") + .addChild(FulltextIndexConstants.PROP_NODE) + .addChild("foo"); + nonFunc.setProperty(FulltextIndexConstants.PROP_ORDERED,true); + nonFunc.setProperty("name", "jcr:content/n/foo"); + + Tree func = luceneIndex.getChild(FulltextIndexConstants.INDEX_RULES) + .getChild("nt:unstructured") + .getChild(FulltextIndexConstants.PROP_NODE) + .addChild("testOak"); + func.setProperty(FulltextIndexConstants.PROP_ORDERED,true); + func.setProperty(FulltextIndexConstants.PROP_FUNCTION, "fn:upper-case(jcr:content/n/@foo)"); + + // Now do some change in the node that are covered by above index definition + try { + customLogs.starting(); + root.getTree("/").getChild("test").getChild("jcr:content").getChild("n").setProperty("foo","bar2"); + root.commit(); + Assert.assertFalse(customLogs.getLogs().contains("Failed to index the node [/test]")); + Assert.assertTrue(customLogs.getLogs().size() == 0); + } finally { + customLogs.finished(); + } + + } + /* + Given an index def with 2 orderable property definitions(Relative) for same property - one with function and one without + Order by should give correct results + */ + @Test + public void sameOrderableRelPropWithAndWithoutFunc_checkOrdering() throws Exception { + + // Index def with same property - ordered - one with function and one without + Tree luceneIndex = createIndex("upper", Collections.<String>emptySet()); + Tree nonFunc = luceneIndex.addChild(FulltextIndexConstants.INDEX_RULES) + .addChild("nt:base") + .addChild(FulltextIndexConstants.PROP_NODE) + .addChild("foo"); + nonFunc.setProperty(FulltextIndexConstants.PROP_PROPERTY_INDEX,true); + nonFunc.setProperty(FulltextIndexConstants.PROP_ORDERED,true); + nonFunc.setProperty("name", "jcr:content/n/foo"); + + Tree func = luceneIndex.getChild(FulltextIndexConstants.INDEX_RULES) + .getChild("nt:base") + .getChild(FulltextIndexConstants.PROP_NODE) + .addChild("testOak"); + func.setProperty(FulltextIndexConstants.PROP_ORDERED,true); + func.setProperty(FulltextIndexConstants.PROP_FUNCTION, "fn:upper-case(jcr:content/n/@foo)"); + + root.commit(); + + + int i = 1; + // Create nodes that will be served by the index definition that follows + for (String node : asList("a", "c", "b","e","d")) { + + Tree test = root.getTree("/").addChild(node); + test.setProperty("jcr:primaryType", "nt:unstructured", Type.NAME); + + Tree a = test.addChild("jcr:content"); + a.setProperty("jcr:primaryType", "nt:unstructured", Type.NAME); + + Tree b = a.addChild("n"); + + b.setProperty("jcr:primaryType", "nt:unstructured", Type.NAME); + b.setProperty("foo", "bar"+i); + i++; + } + + root.commit(); + + + // Check ordering works for func and non func properties + assertOrderedPlanAndQuery( + "select * from [nt:base] order by upper([jcr:content/n/foo])", + "lucene:upper(/oak:index/upper)", asList("/a","/c","/b","/e","/d")); + + assertOrderedPlanAndQuery( + "select * from [nt:base] order by [jcr:content/n/foo]", + "lucene:upper(/oak:index/upper)", asList("/a","/c","/b","/e","/d")); + + assertOrderedPlanAndQuery( + "select * from [nt:base] order by upper([jcr:content/n/foo]) DESC", + "lucene:upper(/oak:index/upper)", asList("/d","/e","/b","/c","/a")); + + assertOrderedPlanAndQuery( + "select * from [nt:base] order by [jcr:content/n/foo] DESC", + "lucene:upper(/oak:index/upper)", asList("/d","/e","/b","/c","/a")); + + + // Now we change the value of foo on already indexed nodes and see if changes get indexed properly. + + i = 5; + for (String node : asList("a", "c", "b","e","d")) { + + Tree test = root.getTree("/").getChild(node).getChild("jcr:content").getChild("n"); + + test.setProperty("foo", "bar" + i); + i--; + } + root.commit(); + + assertOrderedPlanAndQuery( + "select * from [nt:base] order by upper([jcr:content/n/foo])", + "lucene:upper(/oak:index/upper)", asList("/d","/e","/b","/c","/a")); + + assertOrderedPlanAndQuery( + "select * from [nt:base] order by [jcr:content/n/foo]", + "lucene:upper(/oak:index/upper)", asList("/d","/e","/b","/c","/a")); + + assertOrderedPlanAndQuery( + "select * from [nt:base] order by upper([jcr:content/n/foo]) DESC", + "lucene:upper(/oak:index/upper)", asList("/a","/c","/b","/e","/d")); + + assertOrderedPlanAndQuery( + "select * from [nt:base] order by [jcr:content/n/foo] DESC", + "lucene:upper(/oak:index/upper)", asList("/a","/c","/b","/e","/d")); + + } + + /* + Given an index def with 2 orderable property definitions(non-relative) for same property - one with function and one without + Indexer should index any changes properly and ordering should work as expected. + */ + @Test + public void sameOrderablePropertyWithandWithoutFunction() throws Exception { + LogCustomizer customLogs = LogCustomizer.forLogger(LuceneIndexEditor.class.getName()).enable(Level.WARN).create(); + // Create nodes that will be served by the index definition that follows + int i = 1; + // Create nodes that will be served by the index definition that follows + for (String node : asList("a", "c", "b","e","d")) { + + Tree test = root.getTree("/").addChild(node); + test.setProperty("jcr:primaryType", "nt:unstructured", Type.NAME); + test.setProperty("foo", "bar"+i); + i++; + } + + root.commit(); + + // Index def with same property - ordered - one with function and one without + Tree luceneIndex = createIndex("upper", Collections.<String>emptySet()); + Tree nonFunc = luceneIndex.addChild(FulltextIndexConstants.INDEX_RULES) + .addChild("nt:base") + .addChild(FulltextIndexConstants.PROP_NODE) + .addChild("foo"); + nonFunc.setProperty(FulltextIndexConstants.PROP_ORDERED,true); + nonFunc.setProperty(FulltextIndexConstants.PROP_PROPERTY_INDEX,true); + nonFunc.setProperty("name", "foo"); + + Tree func = luceneIndex.getChild(FulltextIndexConstants.INDEX_RULES) + .getChild("nt:base") + .getChild(FulltextIndexConstants.PROP_NODE) + .addChild("testOak"); + func.setProperty(FulltextIndexConstants.PROP_ORDERED,true); + func.setProperty(FulltextIndexConstants.PROP_FUNCTION, "fn:upper-case(@foo)"); + + // Now do some change in the node that are covered by above index definition + try { + customLogs.starting(); + i = 5; + for (String node : asList("a", "c", "b","e","d")) { + + Tree test = root.getTree("/").addChild(node); + test.setProperty("jcr:primaryType", "nt:unstructured", Type.NAME); + + test.setProperty("foo", "bar"+i); + i--; + } + + root.commit(); + Assert.assertFalse(customLogs.getLogs().contains("Failed to index the node [/test]")); + Assert.assertTrue(customLogs.getLogs().size() == 0); + + assertOrderedPlanAndQuery( + "select * from [nt:base] order by upper([foo])", + "lucene:upper(/oak:index/upper)", asList("/d","/e","/b","/c","/a")); + + assertOrderedPlanAndQuery( + "select * from [nt:base] order by [foo]", + "lucene:upper(/oak:index/upper)", asList("/d","/e","/b","/c","/a")); + + assertOrderedPlanAndQuery( + "select * from [nt:base] order by upper([foo]) DESC", + "lucene:upper(/oak:index/upper)", asList("/a","/c","/b","/e","/d")); + + assertOrderedPlanAndQuery( + "select * from [nt:base] order by [foo] DESC", + "lucene:upper(/oak:index/upper)", asList("/a","/c","/b","/e","/d")); + + } finally { + customLogs.finished(); + } + + } + protected String explain(String query){ String explain = "explain " + query; Modified: jackrabbit/oak/trunk/oak-search/src/main/java/org/apache/jackrabbit/oak/plugins/index/search/Aggregate.java URL: http://svn.apache.org/viewvc/jackrabbit/oak/trunk/oak-search/src/main/java/org/apache/jackrabbit/oak/plugins/index/search/Aggregate.java?rev=1858544&r1=1858543&r2=1858544&view=diff ============================================================================== --- jackrabbit/oak/trunk/oak-search/src/main/java/org/apache/jackrabbit/oak/plugins/index/search/Aggregate.java (original) +++ jackrabbit/oak/trunk/oak-search/src/main/java/org/apache/jackrabbit/oak/plugins/index/search/Aggregate.java Thu May 2 13:57:43 2019 @@ -427,6 +427,19 @@ public class Aggregate { } } + public static class FunctionInclude extends PropertyInclude { + + public FunctionInclude(PropertyDefinition pd) { + super(pd); + } + + @Override + public void collectResults(String nodePath, NodeState nodeState, ResultCollector results) { + // Do Nothing here - Function includes aren't indexed using aggregate of property parameters of the function itself + } + + } + public interface ResultCollector { void onResult(NodeIncludeResult result); Modified: jackrabbit/oak/trunk/oak-search/src/main/java/org/apache/jackrabbit/oak/plugins/index/search/IndexDefinition.java URL: http://svn.apache.org/viewvc/jackrabbit/oak/trunk/oak-search/src/main/java/org/apache/jackrabbit/oak/plugins/index/search/IndexDefinition.java?rev=1858544&r1=1858543&r2=1858544&view=diff ============================================================================== --- jackrabbit/oak/trunk/oak-search/src/main/java/org/apache/jackrabbit/oak/plugins/index/search/IndexDefinition.java (original) +++ jackrabbit/oak/trunk/oak-search/src/main/java/org/apache/jackrabbit/oak/plugins/index/search/IndexDefinition.java Thu May 2 13:57:43 2019 @@ -1183,7 +1183,7 @@ public class IndexDefinition implements for (String p : properties) { if (PathUtils.getDepth(p) > 1) { PropertyDefinition pd2 = new PropertyDefinition(this, p, propDefnNode); - propAggregate.add(new Aggregate.PropertyInclude(pd2)); + propAggregate.add(new Aggregate.FunctionInclude(pd2)); } } // a function index has no other options