You are viewing a plain text version of this content. The canonical link for it is here.
Posted to oak-commits@jackrabbit.apache.org by fo...@apache.org on 2023/02/24 07:22:08 UTC
[jackrabbit-oak] branch trunk updated: OAK-10118: fix inconsistencies between lucene and elastic on order by queries (#853)
This is an automated email from the ASF dual-hosted git repository.
fortino pushed a commit to branch trunk
in repository https://gitbox.apache.org/repos/asf/jackrabbit-oak.git
The following commit(s) were added to refs/heads/trunk by this push:
new 35d92bbd2e OAK-10118: fix inconsistencies between lucene and elastic on order by queries (#853)
35d92bbd2e is described below
commit 35d92bbd2e12ff73332685089ba31ccf37a78782
Author: Fabrizio Fortino <fa...@gmail.com>
AuthorDate: Fri Feb 24 08:22:03 2023 +0100
OAK-10118: fix inconsistencies between lucene and elastic on order by queries (#853)
* OAK-10118 (wip) refactor order by tests
* OAK-10118: (wip) reverted test with includePropertyNames/orderedProps use
* OAK-10118: (wip) move elastic order by tests in commons
* OAK-10118: (fix) functions on regexp-based properties should not be used in sort options
* OAK-10118: missing licenses
* OAK-10118: add unit test for order by with regexp
* Update oak-search-elastic/src/main/java/org/apache/jackrabbit/oak/plugins/index/elastic/query/ElasticIndexPlanner.java
Co-authored-by: Nuno Santos <ns...@adobe.com>
* OAK-10118: add more meaningful description in ignored test
---------
Co-authored-by: Nuno Santos <ns...@adobe.com>
---
.../plugins/index/lucene/LuceneOrderByTest.java | 58 +++
.../index/lucene/LucenePropertyIndexTest.java | 211 +---------
.../index/elastic/query/ElasticIndexPlanner.java | 7 +
.../plugins/index/elastic/ElasticOrderByTest.java | 155 +-------
.../oak/plugins/index/OrderByCommonTest.java | 438 +++++++++++++++++++++
5 files changed, 520 insertions(+), 349 deletions(-)
diff --git a/oak-lucene/src/test/java/org/apache/jackrabbit/oak/plugins/index/lucene/LuceneOrderByTest.java b/oak-lucene/src/test/java/org/apache/jackrabbit/oak/plugins/index/lucene/LuceneOrderByTest.java
new file mode 100644
index 0000000000..134ed48779
--- /dev/null
+++ b/oak-lucene/src/test/java/org/apache/jackrabbit/oak/plugins/index/lucene/LuceneOrderByTest.java
@@ -0,0 +1,58 @@
+/*
+ * 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 org.apache.jackrabbit.oak.api.ContentRepository;
+import org.apache.jackrabbit.oak.plugins.index.LuceneIndexOptions;
+import org.apache.jackrabbit.oak.plugins.index.OrderByCommonTest;
+import org.junit.Ignore;
+import org.junit.Rule;
+import org.junit.Test;
+import org.junit.rules.TemporaryFolder;
+
+import java.io.File;
+import java.util.concurrent.ExecutorService;
+import java.util.concurrent.Executors;
+
+public class LuceneOrderByTest extends OrderByCommonTest {
+
+ private ExecutorService executorService = Executors.newFixedThreadPool(2);
+ @Rule
+ public TemporaryFolder temporaryFolder = new TemporaryFolder(new File("target"));
+
+ @Override
+ protected ContentRepository createRepository() {
+ repositoryOptionsUtil = new LuceneTestRepositoryBuilder(executorService, temporaryFolder).build();
+ indexOptions = new LuceneIndexOptions();
+ return repositoryOptionsUtil.getOak().createContentRepository();
+ }
+
+ @Ignore("OAK-7370")
+ @Test
+ @Override
+ public void orderByScoreAcrossUnion() throws Exception {
+ super.orderByScoreAcrossUnion();
+ }
+
+ @Ignore("multiple DESC conditions do not produce the expected results in lucene")
+ @Test
+ @Override
+ public void orderByMultiProperties() throws Exception {
+ super.orderByMultiProperties();
+ }
+
+}
diff --git a/oak-lucene/src/test/java/org/apache/jackrabbit/oak/plugins/index/lucene/LucenePropertyIndexTest.java b/oak-lucene/src/test/java/org/apache/jackrabbit/oak/plugins/index/lucene/LucenePropertyIndexTest.java
index 4c3de80cf4..3a5b264f20 100644
--- a/oak-lucene/src/test/java/org/apache/jackrabbit/oak/plugins/index/lucene/LucenePropertyIndexTest.java
+++ b/oak-lucene/src/test/java/org/apache/jackrabbit/oak/plugins/index/lucene/LucenePropertyIndexTest.java
@@ -61,7 +61,6 @@ import org.apache.jackrabbit.oak.api.Result;
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.PathUtils;
import org.apache.jackrabbit.oak.commons.concurrent.ExecutorCloser;
import org.apache.jackrabbit.oak.plugins.index.AsyncIndexInfoService;
import org.apache.jackrabbit.oak.plugins.index.AsyncIndexInfoServiceImpl;
@@ -700,87 +699,6 @@ public class LucenePropertyIndexTest extends AbstractQueryTest {
return t;
}
- @Test
- public void orderByScore() throws Exception {
- Tree idx = createIndex("test1", of("propa"));
- idx.addChild(PROP_NODE).addChild("propa");
- root.commit();
-
- Tree test = root.getTree("/").addChild("test");
- test.addChild("a").setProperty("propa", "a");
- root.commit();
-
- assertQuery("select [jcr:path] from [nt:base] where propa is not null order by [jcr:score]", asList("/test/a"));
- }
-
- // OAK-7370
- @Ignore("OAK-7370")
- @Test
- public void orderByScoreAcrossUnion() throws Exception {
- Tree idx = root.getTree("/").addChild(INDEX_DEFINITIONS_NAME).addChild("test-index");
- IndexDefinitionBuilder builder = new LuceneIndexDefinitionBuilder();
- builder.evaluatePathRestrictions();
- builder.indexRule("nt:base")
- .property("foo").analyzed().propertyIndex().nodeScopeIndex()
- .property("p1").propertyIndex()
- .property("p2").propertyIndex();
- builder.build(idx);
- idx.removeProperty("async");
- root.commit();
-
- Tree testRoot = root.getTree("/").addChild("test");
- Tree path1 = testRoot.addChild("path1");
- Tree path2 = testRoot.addChild("path2");
-
- Tree c1 = path1.addChild("c1");
- c1.setProperty("foo", "bar. some extra stuff");
- c1.setProperty("p1", "d");
-
- Tree c2 = path2.addChild("c2");
- c2.setProperty("foo", "bar");
- c2.setProperty("p2", "d");
-
- Tree c3 = path2.addChild("c3");
- c3.setProperty("foo", "bar. some extra stuff... and then some to make it worse than c1");
- c3.setProperty("p2", "d");
-
- // create more stuff to get num_docs in index large and hence force union optimization
- for (int i = 0; i < 10; i++) {
- testRoot.addChild("extra" + i).setProperty("foo", "stuff");
- }
-
- root.commit();
-
- List<String> expected = asList(c2.getPath(), c1.getPath(), c3.getPath());
-
- String query;
-
- // manual union
- query =
- "select [jcr:path] from [nt:base] where contains(*, 'bar') and isdescendantnode('" + path1.getPath() + "')" +
- " union " +
- "select [jcr:path] from [nt:base] where contains(*, 'bar') and isdescendantnode('" + path2.getPath() + "')" +
- " order by [jcr:score] desc";
-
- assertEquals(expected, executeQuery(query, SQL2));
-
- // no union (estiimated fulltext without union would be same as sub-queries and it won't be optimized
- query = "select [jcr:path] from [nt:base] where contains(*, 'bar')" +
- " and (isdescendantnode('" + path1.getPath() + "') or" +
- " isdescendantnode('" + path2.getPath() + "'))" +
- " order by [jcr:score] desc";
-
- assertEquals(expected, executeQuery(query, SQL2));
-
- // optimization UNION as we're adding constraints to sub-queries that would improve cost of optimized union
- query = "select [jcr:path] from [nt:base] where contains(*, 'bar')" +
- " and ( (p1 = 'd' and isdescendantnode('" + path1.getPath() + "')) or" +
- " (p2 = 'd' and isdescendantnode('" + path2.getPath() + "')))" +
- " order by [jcr:score] desc";
-
- assertEquals(expected, executeQuery(query, SQL2));
- }
-
@Test
public void rangeQueriesWithLong() throws Exception {
Tree idx = createIndex("test1", of("propa", "propb"));
@@ -882,134 +800,6 @@ public class LucenePropertyIndexTest extends AbstractQueryTest {
assertQuery(query, asList("/node2"));
}
- @Test
- public void sortOnNodeName() throws Exception {
- Tree rootTree = root.getTree("/").addChild("test");
-
- IndexDefinitionBuilder fnNameFunctionIndex = new LuceneIndexDefinitionBuilder().noAsync();
- LuceneIndexDefinitionBuilder.PropertyRule rule = fnNameFunctionIndex.indexRule("nt:base")
- .property("nodeName", null)
- .propertyIndex()
- .ordered();
-
- // setup function index on "fn:name()"
- rule.function("fn:name()");
- fnNameFunctionIndex.getBuilderTree().setProperty("tags", of("fnName"), STRINGS);
- fnNameFunctionIndex.build(rootTree.addChild("oak:index").addChild("fnName"));
-
- // same index as above except for function - "name()"
- rule.function("name()");
- fnNameFunctionIndex.getBuilderTree().setProperty("tags", of("name"), STRINGS);
- fnNameFunctionIndex.build(rootTree.addChild("oak:index").addChild("name"));
-
- List<String> expected = Lists.newArrayList("/test/oak:index");
- for (int i = 0; i < 3; i++) {
- String nodeName = "a" + i;
- rootTree.addChild(nodeName);
- expected.add("/test/" + nodeName);
- }
- Collections.sort(expected);
- root.commit();
-
- String query = "/jcr:root/test/* order by fn:name() option(index tag fnName)";
- assertXpathPlan(query, "lucene:fnName(/test/oak:index/fnName)");
- assertEquals(expected, executeQuery(query, XPATH));
-
- query = "/jcr:root/test/* order by fn:name() ascending option(index tag fnName)";
- assertXpathPlan(query, "lucene:fnName(/test/oak:index/fnName)");
- assertEquals(expected, executeQuery(query, XPATH));
-
- query = "/jcr:root/test/* order by fn:name() descending option(index tag fnName)";
- assertXpathPlan(query, "lucene:fnName(/test/oak:index/fnName)");
- assertEquals(Lists.reverse(expected), executeQuery(query, XPATH));
-
- // order by fn:name() although function index is on "name()"
- query = "/jcr:root/test/* order by fn:name() option(index tag name)";
- assertXpathPlan(query, "lucene:name(/test/oak:index/name)");
- assertEquals(expected, executeQuery(query, XPATH));
-
- query = "/jcr:root/test/* order by fn:name() ascending option(index tag name)";
- assertXpathPlan(query, "lucene:name(/test/oak:index/name)");
- assertEquals(expected, executeQuery(query, XPATH));
-
- query = "/jcr:root/test/* order by fn:name() descending option(index tag name)";
- assertXpathPlan(query, "lucene:name(/test/oak:index/name)");
- assertEquals(Lists.reverse(expected), executeQuery(query, XPATH));
- }
-
- @Test
- public void sortOnLocalName() throws Exception {
- Tree rootTree = root.getTree("/").addChild("test");
-
- IndexDefinitionBuilder fnNameFunctionIndex = new LuceneIndexDefinitionBuilder().noAsync();
- IndexDefinitionBuilder.PropertyRule rule = fnNameFunctionIndex.indexRule("nt:base")
- .property("nodeName", null)
- .propertyIndex()
- .ordered();
-
- // setup function index on "fn:name()"
- rule.function("fn:local-name()");
- fnNameFunctionIndex.getBuilderTree().setProperty("tags", of("fnLocalName"), STRINGS);
- fnNameFunctionIndex.build(rootTree.addChild("oak:index").addChild("fnLocalName"));
-
- // same index as above except for function - "name()"
- rule.function("localname()");
- fnNameFunctionIndex.getBuilderTree().setProperty("tags", of("localName"), STRINGS);
- fnNameFunctionIndex.build(rootTree.addChild("oak:index").addChild("localName"));
-
- List<String> expected = Lists.newArrayList("/test/oak:index");
- for (int i = 0; i < 3; i++) {
- String nodeName = "ja" + i;//'j*' should come after (asc) 'index' in sort order
- rootTree.addChild(nodeName);
- expected.add("/test/" + nodeName);
- }
-
- //sort expectation based on local name
- Collections.sort(expected, (s1, s2) -> {
- final StringBuffer sb1 = new StringBuffer();
- PathUtils.elements(s1).forEach(elem -> {
- String[] split = elem.split(":", 2);
- sb1.append(split[split.length - 1]);
- });
- s1 = sb1.toString();
-
- final StringBuffer sb2 = new StringBuffer();
- PathUtils.elements(s2).forEach(elem -> {
- String[] split = elem.split(":", 2);
- sb2.append(split[split.length - 1]);
- });
- s2 = sb2.toString();
-
- return s1.compareTo(s2);
- });
- root.commit();
-
- String query = "/jcr:root/test/* order by fn:local-name() option(index tag fnLocalName)";
- assertXpathPlan(query, "lucene:fnLocalName(/test/oak:index/fnLocalName)");
- assertEquals(expected, executeQuery(query, XPATH));
-
- query = "/jcr:root/test/* order by fn:local-name() ascending option(index tag fnLocalName)";
- assertXpathPlan(query, "lucene:fnLocalName(/test/oak:index/fnLocalName)");
- assertEquals(expected, executeQuery(query, XPATH));
-
- query = "/jcr:root/test/* order by fn:local-name() descending option(index tag fnLocalName)";
- assertXpathPlan(query, "lucene:fnLocalName(/test/oak:index/fnLocalName)");
- assertEquals(Lists.reverse(expected), executeQuery(query, XPATH));
-
- // order by fn:name() although function index is on "name()"
- query = "/jcr:root/test/* order by fn:local-name() option(index tag localName)";
- assertXpathPlan(query, "lucene:localName(/test/oak:index/localName)");
- assertEquals(expected, executeQuery(query, XPATH));
-
- query = "/jcr:root/test/* order by fn:local-name() ascending option(index tag localName)";
- assertXpathPlan(query, "lucene:localName(/test/oak:index/localName)");
- assertEquals(expected, executeQuery(query, XPATH));
-
- query = "/jcr:root/test/* order by fn:local-name() descending option(index tag localName)";
- assertXpathPlan(query, "lucene:localName(/test/oak:index/localName)");
- assertEquals(Lists.reverse(expected), executeQuery(query, XPATH));
- }
-
//OAK-4517
@Test
public void pathIncludeSubrootIndex() throws Exception {
@@ -1356,6 +1146,7 @@ public class LucenePropertyIndexTest extends AbstractQueryTest {
assertOrderedQuery("select [jcr:path] from [nt:base] order by [foo] DESC", getSortedPaths(tuples, OrderDirection.DESC));
}
+
@Test
public void sortQueriesWithLong_NotIndexed_relativeProps() throws Exception {
Tree idx = createIndex("test1", Collections.<String>emptySet());
diff --git a/oak-search-elastic/src/main/java/org/apache/jackrabbit/oak/plugins/index/elastic/query/ElasticIndexPlanner.java b/oak-search-elastic/src/main/java/org/apache/jackrabbit/oak/plugins/index/elastic/query/ElasticIndexPlanner.java
index 885b80228d..abedb1d752 100644
--- a/oak-search-elastic/src/main/java/org/apache/jackrabbit/oak/plugins/index/elastic/query/ElasticIndexPlanner.java
+++ b/oak-search-elastic/src/main/java/org/apache/jackrabbit/oak/plugins/index/elastic/query/ElasticIndexPlanner.java
@@ -24,6 +24,7 @@ import org.apache.jackrabbit.oak.plugins.index.search.IndexNode;
import org.apache.jackrabbit.oak.plugins.index.search.PropertyDefinition;
import org.apache.jackrabbit.oak.plugins.index.search.spi.query.FulltextIndexPlanner;
import org.apache.jackrabbit.oak.spi.query.Filter;
+import org.apache.jackrabbit.oak.spi.query.QueryConstants;
import org.apache.jackrabbit.oak.spi.query.QueryIndex;
import java.util.ArrayList;
@@ -36,6 +37,10 @@ public class ElasticIndexPlanner extends FulltextIndexPlanner {
super(indexNode, indexPath, filter, sortOrder);
}
+ /**
+ * Overrides the basic planner to add support for all the defined properties regardless of the ordered flag since
+ * Elastic supports sorting on all fields without any additional configuration.
+ */
@Override
protected List<QueryIndex.OrderEntry> createSortOrder(IndexDefinition.IndexingRule rule) {
if (sortOrder == null) {
@@ -48,6 +53,8 @@ public class ElasticIndexPlanner extends FulltextIndexPlanner {
PropertyDefinition pd = rule.getConfig(propName);
if (pd != null
&& o.getPropertyType() != null
+ // functions on regexp-based properties must be skipped since the values cannot be indexed
+ && (!pd.isRegexp || !propName.startsWith(QueryConstants.FUNCTION_RESTRICTION_PREFIX))
&& !o.getPropertyType().isArray()) {
orderEntries.add(o); // can manage any order desc/asc
} else if (JcrConstants.JCR_SCORE.equals(propName)) {
diff --git a/oak-search-elastic/src/test/java/org/apache/jackrabbit/oak/plugins/index/elastic/ElasticOrderByTest.java b/oak-search-elastic/src/test/java/org/apache/jackrabbit/oak/plugins/index/elastic/ElasticOrderByTest.java
index 77f5bf5df0..980ff6bf54 100644
--- a/oak-search-elastic/src/test/java/org/apache/jackrabbit/oak/plugins/index/elastic/ElasticOrderByTest.java
+++ b/oak-search-elastic/src/test/java/org/apache/jackrabbit/oak/plugins/index/elastic/ElasticOrderByTest.java
@@ -16,152 +16,29 @@
*/
package org.apache.jackrabbit.oak.plugins.index.elastic;
-import org.apache.jackrabbit.oak.api.Tree;
-import org.apache.jackrabbit.oak.plugins.index.search.util.IndexDefinitionBuilder;
-import org.junit.Test;
+import org.apache.jackrabbit.oak.api.ContentRepository;
+import org.apache.jackrabbit.oak.plugins.index.OrderByCommonTest;
+import org.junit.ClassRule;
-import javax.jcr.PropertyType;
-import java.util.Collections;
-import java.util.UUID;
+public class ElasticOrderByTest extends OrderByCommonTest {
-import static java.util.Arrays.asList;
+ @ClassRule
+ public static final ElasticConnectionRule elasticRule =
+ new ElasticConnectionRule(ElasticTestUtils.ELASTIC_CONNECTION_STRING);
-public class ElasticOrderByTest extends ElasticAbstractQueryTest {
-
- @Test
- public void withoutOrderByClause() throws Exception {
- IndexDefinitionBuilder builder = createIndex("foo");
- builder.indexRule("nt:base")
- .property("foo")
- .analyzed();
-
- setIndex(UUID.randomUUID().toString(), builder);
- root.commit();
-
- Tree test = root.getTree("/").addChild("test");
- test.addChild("a").setProperty("foo", "hello");
- test.addChild("b").setProperty("foo", "hello hello");
- root.commit(Collections.singletonMap("sync-mode", "rt"));
-
- // results are sorted by score desc, node `b` returns first because it has a higher score from a tf/idf perspective
- assertOrderedQuery("select [jcr:path] from [nt:base] where contains(foo, 'hello')", asList("/test/b", "/test/a"));
+ public ElasticOrderByTest() {
+ indexOptions = new ElasticIndexOptions();
}
- @Test
- public void orderByScore() throws Exception {
- IndexDefinitionBuilder builder = createIndex("foo");
- builder.indexRule("nt:base")
- .property("foo")
- .analyzed();
-
- setIndex(UUID.randomUUID().toString(), builder);
-
- Tree test = root.getTree("/").addChild("test");
- test.addChild("a").setProperty("foo", "hello");
- test.addChild("b").setProperty("foo", "hello hello");
- root.commit(Collections.singletonMap("sync-mode", "rt"));
-
- assertOrderedQuery("select [jcr:path] from [nt:base] where contains(foo, 'hello') order by [jcr:score]",
- asList("/test/a", "/test/b"));
-
- assertOrderedQuery("select [jcr:path] from [nt:base] where contains(foo, 'hello') order by [jcr:score] DESC",
- asList("/test/b", "/test/a"));
+ @Override
+ protected ContentRepository createRepository() {
+ repositoryOptionsUtil = new ElasticTestRepositoryBuilder(elasticRule).build();
+ return repositoryOptionsUtil.getOak().createContentRepository();
}
- @Test
- public void orderByPath() throws Exception {
- IndexDefinitionBuilder builder = createIndex("foo");
- builder.indexRule("nt:base").property("foo");
-
- setIndex(UUID.randomUUID().toString(), builder);
-
- Tree test = root.getTree("/").addChild("test");
- test.addChild("a").setProperty("foo", "aaaaaa");
- test.addChild("b").setProperty("foo", "bbbbbb");
- root.commit(Collections.singletonMap("sync-mode", "rt"));
-
- assertOrderedQuery("select [jcr:path] from [nt:base] order by [jcr:path]", asList("/test/a", "/test/b"));
- assertOrderedQuery("select [jcr:path] from [nt:base] order by [jcr:path] DESC", asList("/test/b", "/test/a"));
- }
-
- @Test
- public void orderByProperty() throws Exception {
- IndexDefinitionBuilder builder = createIndex("foo");
- builder.indexRule("nt:base").property("foo");
-
- setIndex(UUID.randomUUID().toString(), builder);
-
- Tree test = root.getTree("/").addChild("test");
- test.addChild("a").setProperty("foo", "zzzzzz");
- test.addChild("b").setProperty("foo", "aaaaaa");
- root.commit(Collections.singletonMap("sync-mode", "rt"));
-
- assertOrderedQuery("select [jcr:path] from [nt:base] order by @foo", asList("/test/b", "/test/a"));
- assertOrderedQuery("select [jcr:path] from [nt:base] order by @foo DESC", asList("/test/a", "/test/b"));
+ @Override
+ protected void createTestIndexNode() {
+ setTraversalEnabled(false);
}
- @Test
- public void orderByAnalyzedProperty() throws Exception {
- IndexDefinitionBuilder builder = createIndex("foo");
- builder.indexRule("nt:base").property("foo").analyzed();
-
- setIndex(UUID.randomUUID().toString(), builder);
-
- Tree test = root.getTree("/").addChild("test");
- test.addChild("a").setProperty("foo", "bbbbb");
- test.addChild("b").setProperty("foo", "hello aaaa");
- root.commit(Collections.singletonMap("sync-mode", "rt"));
-
- // this test verifies we use the keyword multi field when an analyzed properties is specified in order by
- // http://www.technocratsid.com/string-sorting-in-elasticsearch/
-
- assertOrderedQuery("select [jcr:path] from [nt:base] order by @foo", asList("/test/a", "/test/b"));
- assertOrderedQuery("select [jcr:path] from [nt:base] order by @foo DESC", asList("/test/b", "/test/a"));
- }
-
- @Test
- public void orderByNumericProperty() throws Exception {
- IndexDefinitionBuilder builder = createIndex("foo");
- builder.indexRule("nt:base").property("foo").type(PropertyType.TYPENAME_LONG);
-
- setIndex(UUID.randomUUID().toString(), builder);
-
- Tree test = root.getTree("/").addChild("test");
- test.addChild("a").setProperty("foo", "10");
- test.addChild("b").setProperty("foo", "5");
- root.commit(Collections.singletonMap("sync-mode", "rt"));
-
- assertOrderedQuery("select [jcr:path] from [nt:base] order by @foo", asList("/test/b", "/test/a"));
- assertOrderedQuery("select [jcr:path] from [nt:base] order by @foo DESC", asList("/test/a", "/test/b"));
- }
-
- @Test
- public void orderByMultiProperties() throws Exception {
- IndexDefinitionBuilder builder = createIndex("foo", "bar");
- builder.indexRule("nt:base").property("foo");
- builder.indexRule("nt:base").property("bar").type(PropertyType.TYPENAME_LONG);
-
- setIndex(UUID.randomUUID().toString(), builder);
-
- Tree test = root.getTree("/").addChild("test");
- Tree a1 = test.addChild("a1");
- a1.setProperty("foo", "a");
- a1.setProperty("bar", "1");
- Tree a2 = test.addChild("a2");
- a2.setProperty("foo", "a");
- a2.setProperty("bar", "2");
- Tree b = test.addChild("b");
- b.setProperty("foo", "b");
- b.setProperty("bar", "100");
- root.commit(Collections.singletonMap("sync-mode", "rt"));
-
- assertOrderedQuery("select [jcr:path] from [nt:base] order by @foo, @bar",
- asList("/test/a1", "/test/a2", "/test/b"));
-
- assertOrderedQuery("select [jcr:path] from [nt:base] order by @foo, @bar DESC",
- asList("/test/a2", "/test/a1", "/test/b"));
-
- assertOrderedQuery("select [jcr:path] from [nt:base] order by @bar DESC, @foo DESC",
- asList("/test/b", "/test/a2", "/test/a1"));
- }
}
diff --git a/oak-search/src/test/java/org/apache/jackrabbit/oak/plugins/index/OrderByCommonTest.java b/oak-search/src/test/java/org/apache/jackrabbit/oak/plugins/index/OrderByCommonTest.java
new file mode 100644
index 0000000000..327e2539bd
--- /dev/null
+++ b/oak-search/src/test/java/org/apache/jackrabbit/oak/plugins/index/OrderByCommonTest.java
@@ -0,0 +1,438 @@
+/*
+ * 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;
+
+import com.google.common.collect.Iterables;
+import com.google.common.collect.Lists;
+import org.apache.jackrabbit.oak.api.Result;
+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.PathUtils;
+import org.apache.jackrabbit.oak.plugins.index.search.FulltextIndexConstants;
+import org.apache.jackrabbit.oak.plugins.index.search.util.IndexDefinitionBuilder;
+import org.apache.jackrabbit.oak.query.AbstractQueryTest;
+import org.junit.Test;
+
+import javax.jcr.PropertyType;
+import java.text.ParseException;
+import java.util.List;
+import java.util.UUID;
+import java.util.stream.Collectors;
+import java.util.stream.IntStream;
+
+import static com.google.common.collect.ImmutableSet.of;
+import static java.util.Arrays.asList;
+import static org.apache.jackrabbit.oak.api.QueryEngine.NO_BINDINGS;
+import static org.apache.jackrabbit.oak.api.Type.STRINGS;
+import static org.hamcrest.CoreMatchers.containsString;
+import static org.hamcrest.MatcherAssert.assertThat;
+import static org.junit.Assert.assertEquals;
+
+public abstract class OrderByCommonTest extends AbstractQueryTest {
+
+ protected IndexOptions indexOptions;
+ protected TestRepository repositoryOptionsUtil;
+
+ protected void assertEventually(Runnable r) {
+ TestUtil.assertEventually(r,
+ ((repositoryOptionsUtil.isAsync() ? repositoryOptionsUtil.defaultAsyncIndexingTimeInSeconds : 0) + 3000) * 5);
+ }
+
+ @Test
+ public void withoutOrderByClause() throws Exception {
+ IndexDefinitionBuilder builder = indexOptions.createIndexDefinitionBuilder();
+ builder.indexRule("nt:base")
+ .property("foo", null)
+ .analyzed();
+ indexOptions.setIndex(root, UUID.randomUUID().toString(), indexOptions.createIndex(builder, false, "foo"));
+
+ Tree test = root.getTree("/").addChild("test");
+ test.addChild("a").setProperty("foo", "hello");
+ test.addChild("b").setProperty("foo", "hello hello");
+ root.commit();
+
+ // results are sorted by score desc, node `b` returns first because it has a higher score from a tf/idf perspective
+ assertOrderedQuery("select [jcr:path] from [nt:base] where contains(foo, 'hello')", asList("/test/b", "/test/a"));
+ }
+
+ @Test
+ public void orderByScore() throws Exception {
+ IndexDefinitionBuilder builder = indexOptions.createIndexDefinitionBuilder();
+ builder.indexRule("nt:base")
+ .property("foo", null)
+ .analyzed();
+ indexOptions.setIndex(root, UUID.randomUUID().toString(), indexOptions.createIndex(builder, false, "foo"));
+
+ Tree test = root.getTree("/").addChild("test");
+ test.addChild("a").setProperty("foo", "hello");
+ test.addChild("b").setProperty("foo", "hello hello");
+ root.commit();
+
+ assertOrderedQuery("select [jcr:path] from [nt:base] where contains(foo, 'hello') order by [jcr:score]",
+ asList("/test/a", "/test/b"));
+
+ assertOrderedQuery("select [jcr:path] from [nt:base] where contains(foo, 'hello') order by [jcr:score] DESC",
+ asList("/test/b", "/test/a"));
+ }
+
+ @Test
+ public void orderByPath() throws Exception {
+ indexOptions.setIndex(root, UUID.randomUUID().toString(), indexOptions.createIndex(indexOptions.createIndexDefinitionBuilder(), false, "foo"));
+
+ Tree test = root.getTree("/").addChild("test");
+ test.addChild("a").setProperty("foo", "bar");
+ test.addChild("b").setProperty("foo", "bar");
+ root.commit();
+
+ assertOrderedQuery("select [jcr:path] from [nt:base] where foo = 'bar' order by [jcr:path]", asList("/test/a", "/test/b"));
+ assertOrderedQuery("select [jcr:path] from [nt:base] where foo = 'bar' order by [jcr:path] DESC", asList("/test/b", "/test/a"));
+ }
+
+ @Test
+ public void orderByWithRegexProperty() throws Exception {
+ Tree idx = indexOptions.setIndex(root, UUID.randomUUID().toString(), indexOptions.createIndex(indexOptions.createIndexDefinitionBuilder(), false));
+ Tree props = TestUtil.newRulePropTree(idx, "nt:base");
+ TestUtil.enableForFullText(props, FulltextIndexConstants.REGEX_ALL_PROPS, true);
+
+ Tree test = root.getTree("/").addChild("test");
+ Tree a = test.addChild("a");
+ a.setProperty("foo", "bar");
+ a.setProperty("baz", "zzzzzz");
+ Tree b = test.addChild("b");
+ b.setProperty("foo", "bar");
+ b.setProperty("baz", "aaaaaa");
+ root.commit();
+
+ assertOrderedQuery("select [jcr:path] from [nt:base] as a where foo = 'bar' order by @baz", asList("/test/b", "/test/a"));
+ assertOrderedQuery("select [jcr:path] from [nt:base] as a where foo = 'bar' order by @baz DESC", asList("/test/a", "/test/b"));
+ }
+
+ @Test
+ public void orderByFunctionWithRegexProperty() throws Exception {
+ Tree idx = indexOptions.setIndex(root, UUID.randomUUID().toString(), indexOptions.createIndex(indexOptions.createIndexDefinitionBuilder(), false));
+ Tree props = TestUtil.newRulePropTree(idx, "nt:base");
+ TestUtil.enableForFullText(props, FulltextIndexConstants.REGEX_ALL_PROPS, true);
+
+ Tree test = root.getTree("/").addChild("test");
+ test.addChild("A").setProperty("foo", "bar");
+ test.addChild("b").setProperty("foo", "bar");
+ root.commit();
+
+ assertOrderedQuery("select [jcr:path] from [nt:base] as a where foo = 'bar' order by lower(name(a))", asList("/test/A", "/test/b"));
+ assertOrderedQuery("select [jcr:path] from [nt:base] as a where foo = 'bar' order by lower(name(a)) DESC", asList("/test/b", "/test/A"));
+ }
+
+ @Test
+ public void orderByProperty() throws Exception {
+ indexOptions.setIndex(root, UUID.randomUUID().toString(), indexOptions.createIndex(indexOptions.createIndexDefinitionBuilder(), false, "foo", "baz"));
+
+ Tree test = root.getTree("/").addChild("test");
+ Tree a = test.addChild("a");
+ a.setProperty("foo", "bar");
+ a.setProperty("baz", "zzzzzz");
+ Tree b = test.addChild("b");
+ b.setProperty("foo", "bar");
+ b.setProperty("baz", "aaaaaa");
+ root.commit();
+
+ assertOrderedQuery("select [jcr:path] from [nt:base] where foo = 'bar' order by @baz", asList("/test/b", "/test/a"));
+ assertOrderedQuery("select [jcr:path] from [nt:base] where foo = 'bar' order by @baz DESC", asList("/test/a", "/test/b"));
+ }
+
+ @Test
+ public void orderByAnalyzedProperty() throws Exception {
+ IndexDefinitionBuilder builder = indexOptions.createIndexDefinitionBuilder();
+ builder.indexRule("nt:base")
+ .property("baz", null)
+ .analyzed();
+ indexOptions.setIndex(root, UUID.randomUUID().toString(), indexOptions.createIndex(builder, false, "foo", "baz"));
+
+ Tree test = root.getTree("/").addChild("test");
+ Tree a = test.addChild("a");
+ a.setProperty("foo", "bar");
+ a.setProperty("baz", "bbbbb");
+ Tree b = test.addChild("b");
+ b.setProperty("foo", "bar");
+ b.setProperty("baz", "hello aaaa");
+ root.commit();
+
+ // this test verifies we use the keyword multi field when an analyzed properties is specified in order by
+ // http://www.technocratsid.com/string-sorting-in-elasticsearch/
+
+ assertOrderedQuery("select [jcr:path] from [nt:base] where foo = 'bar' order by @baz", asList("/test/a", "/test/b"));
+ assertOrderedQuery("select [jcr:path] from [nt:base] where foo = 'bar' order by @baz DESC", asList("/test/b", "/test/a"));
+ }
+
+ @Test
+ public void orderByNumericProperty() throws Exception {
+ IndexDefinitionBuilder builder = indexOptions.createIndexDefinitionBuilder();
+ builder.indexRule("nt:base")
+ .property("baz", null).propertyIndex().type(PropertyType.TYPENAME_LONG).ordered();
+ indexOptions.setIndex(root, UUID.randomUUID().toString(), indexOptions.createIndex(builder, false, "foo", "baz"));
+
+ Tree test = root.getTree("/").addChild("test");
+ Tree a = test.addChild("a");
+ a.setProperty("foo", "bar");
+ a.setProperty("baz", "10");
+ Tree b = test.addChild("b");
+ b.setProperty("foo", "bar");
+ b.setProperty("baz", "5");
+ root.commit();
+
+ assertOrderedQuery("select [jcr:path] from [nt:base] where foo = 'bar' order by @baz", asList("/test/b", "/test/a"));
+ assertOrderedQuery("select [jcr:path] from [nt:base] where foo = 'bar' order by @baz DESC", asList("/test/a", "/test/b"));
+ }
+
+ @Test
+ public void orderByMultiProperties() throws Exception {
+ IndexDefinitionBuilder builder = indexOptions.createIndexDefinitionBuilder();
+ builder.indexRule("nt:base")
+ .property("bar", null).propertyIndex().type(PropertyType.TYPENAME_LONG).ordered();
+ indexOptions.setIndex(root, UUID.randomUUID().toString(), indexOptions.createIndex(builder, false, "foo", "bar", "baz"));
+
+ Tree test = root.getTree("/").addChild("test");
+ Tree a1 = test.addChild("a1");
+ a1.setProperty("foo", "foobar");
+ a1.setProperty("bar", "1");
+ a1.setProperty("baz", "a");
+ Tree a2 = test.addChild("a2");
+ a2.setProperty("foo", "foobar");
+ a2.setProperty("bar", "2");
+ a2.setProperty("baz", "a");
+ Tree b = test.addChild("b");
+ b.setProperty("foo", "foobar");
+ b.setProperty("bar", "100");
+ b.setProperty("baz", "b");
+ root.commit();
+
+ assertOrderedQuery("select [jcr:path] from [nt:base] where foo = 'foobar' order by @baz, @bar",
+ asList("/test/a1", "/test/a2", "/test/b"));
+
+ assertOrderedQuery("select [jcr:path] from [nt:base] where foo = 'foobar' order by @baz, @bar DESC",
+ asList("/test/a2", "/test/a1", "/test/b"));
+
+ assertOrderedQuery("select [jcr:path] from [nt:base] where foo = 'foobar' order by @bar DESC, @baz DESC",
+ asList("/test/b", "/test/a2", "/test/a1"));
+ }
+
+ @Test
+ public void sortOnNodeName() throws Exception {
+ // setup function index on "fn:name()"
+ IndexDefinitionBuilder idbFnName = indexOptions.createIndexDefinitionBuilder();
+ idbFnName.indexRule("nt:base")
+ .property("nodeName", null)
+ .propertyIndex()
+ .ordered()
+ .function("fn:name()");
+ idbFnName.getBuilderTree().setProperty("tags", of("fnName"), STRINGS);
+ indexOptions.setIndex(root, "fnName", indexOptions.createIndex(idbFnName, false));
+
+ // same index as above except for function - "name()"
+ IndexDefinitionBuilder idbName = indexOptions.createIndexDefinitionBuilder();
+ idbName.indexRule("nt:base")
+ .property("nodeName", null)
+ .propertyIndex()
+ .ordered()
+ .function("name()");
+ idbName.getBuilderTree().setProperty("tags", of("name"), STRINGS);
+ indexOptions.setIndex(root, "name", indexOptions.createIndex(idbName, false));
+
+ Tree testTree = root.getTree("/").addChild("test");
+ List<String> expected = IntStream.range(0, 3).mapToObj(i -> {
+ String nodeName = "a" + i;
+ testTree.addChild(nodeName);
+ return "/test/" + nodeName;
+ }).sorted().collect(Collectors.toList());
+ root.commit();
+
+ String query = "/jcr:root/test/* order by fn:name() option(index tag fnName)";
+ assertXpathPlan(query, indexOptions.getIndexType() + ":fnName(/oak:index/fnName)");
+ assertEquals(expected, executeQuery(query, XPATH));
+
+ query = "/jcr:root/test/* order by fn:name() ascending option(index tag fnName)";
+ assertXpathPlan(query, indexOptions.getIndexType() + ":fnName(/oak:index/fnName)");
+ assertEquals(expected, executeQuery(query, XPATH));
+
+ query = "/jcr:root/test/* order by fn:name() descending option(index tag fnName)";
+ assertXpathPlan(query, indexOptions.getIndexType() + ":fnName(/oak:index/fnName)");
+ assertEquals(Lists.reverse(expected), executeQuery(query, XPATH));
+
+ // order by fn:name() although function index is on "name()"
+ query = "/jcr:root/test/* order by fn:name() option(index tag name)";
+ assertXpathPlan(query, indexOptions.getIndexType() + ":name(/oak:index/name)");
+ assertEquals(expected, executeQuery(query, XPATH));
+
+ query = "/jcr:root/test/* order by fn:name() ascending option(index tag name)";
+ assertXpathPlan(query, indexOptions.getIndexType() + ":name(/oak:index/name)");
+ assertEquals(expected, executeQuery(query, XPATH));
+
+ query = "/jcr:root/test/* order by fn:name() descending option(index tag name)";
+ assertXpathPlan(query, indexOptions.getIndexType() + ":name(/oak:index/name)");
+ assertEquals(Lists.reverse(expected), executeQuery(query, XPATH));
+ }
+
+ @Test
+ public void sortOnLocalName() throws Exception {
+ // setup function index on "fn:local-name()"
+ IndexDefinitionBuilder idbFnLocalName = indexOptions.createIndexDefinitionBuilder();
+ idbFnLocalName.indexRule("nt:base")
+ .property("nodeName", null)
+ .propertyIndex()
+ .ordered()
+ .function("fn:local-name()");
+ idbFnLocalName.getBuilderTree().setProperty("tags", of("fnLocalName"), STRINGS);
+ indexOptions.setIndex(root, "fnLocalName", indexOptions.createIndex(idbFnLocalName, false));
+
+ // same index as above except for function - "localName()"
+ IndexDefinitionBuilder idbLocalName = indexOptions.createIndexDefinitionBuilder();
+ idbLocalName.indexRule("nt:base")
+ .property("nodeName", null)
+ .propertyIndex()
+ .ordered()
+ .function("localname()");
+ idbLocalName.getBuilderTree().setProperty("tags", of("localName"), STRINGS);
+ indexOptions.setIndex(root, "localName", indexOptions.createIndex(idbLocalName, false));
+
+ Tree testTree = root.getTree("/").addChild("test");
+ List<String> expected = IntStream.range(0, 3).mapToObj(i -> {
+ String nodeName = "ja" + i; //'j*' should come after (asc) 'index' in sort order
+ testTree.addChild(nodeName);
+ return "/test/" + nodeName;
+ }).sorted((s1, s2) -> { //sort expectation based on local name
+ final StringBuffer sb1 = new StringBuffer();
+ PathUtils.elements(s1).forEach(elem -> {
+ String[] split = elem.split(":", 2);
+ sb1.append(split[split.length - 1]);
+ });
+ s1 = sb1.toString();
+
+ final StringBuffer sb2 = new StringBuffer();
+ PathUtils.elements(s2).forEach(elem -> {
+ String[] split = elem.split(":", 2);
+ sb2.append(split[split.length - 1]);
+ });
+ s2 = sb2.toString();
+
+ return s1.compareTo(s2);
+ }).collect(Collectors.toList());
+ root.commit();
+
+ String query = "/jcr:root/test/* order by fn:local-name() option(index tag fnLocalName)";
+ assertXpathPlan(query, indexOptions.getIndexType() + ":fnLocalName(/oak:index/fnLocalName)");
+ assertEquals(expected, executeQuery(query, XPATH));
+
+ query = "/jcr:root/test/* order by fn:local-name() ascending option(index tag fnLocalName)";
+ assertXpathPlan(query, indexOptions.getIndexType() + ":fnLocalName(/oak:index/fnLocalName)");
+ assertEquals(expected, executeQuery(query, XPATH));
+
+ query = "/jcr:root/test/* order by fn:local-name() descending option(index tag fnLocalName)";
+ assertXpathPlan(query, indexOptions.getIndexType() + ":fnLocalName(/oak:index/fnLocalName)");
+ assertEquals(Lists.reverse(expected), executeQuery(query, XPATH));
+
+ // order by fn:name() although function index is on "name()"
+ query = "/jcr:root/test/* order by fn:local-name() option(index tag localName)";
+ assertXpathPlan(query, indexOptions.getIndexType() + ":localName(/oak:index/localName)");
+ assertEquals(expected, executeQuery(query, XPATH));
+
+ query = "/jcr:root/test/* order by fn:local-name() ascending option(index tag localName)";
+ assertXpathPlan(query, indexOptions.getIndexType() + ":localName(/oak:index/localName)");
+ assertEquals(expected, executeQuery(query, XPATH));
+
+ query = "/jcr:root/test/* order by fn:local-name() descending option(index tag localName)";
+ assertXpathPlan(query, indexOptions.getIndexType() + ":localName(/oak:index/localName)");
+ assertEquals(Lists.reverse(expected), executeQuery(query, XPATH));
+ }
+
+ @Test
+ public void orderByScoreAcrossUnion() throws Exception {
+ IndexDefinitionBuilder builder = indexOptions.createIndexDefinitionBuilder()
+ .evaluatePathRestrictions().noAsync();
+ builder.indexRule("nt:base")
+ .property("foo").analyzed().propertyIndex().nodeScopeIndex()
+ .property("p1").propertyIndex()
+ .property("p2").propertyIndex();
+ indexOptions.setIndex(root, "test-index", indexOptions.createIndex(builder, false));
+
+ Tree testRoot = root.getTree("/").addChild("test");
+ Tree path1 = testRoot.addChild("path1");
+ Tree path2 = testRoot.addChild("path2");
+
+ Tree c1 = path1.addChild("c1");
+ c1.setProperty("foo", "bar. some extra stuff");
+ c1.setProperty("p1", "d");
+
+ Tree c2 = path2.addChild("c2");
+ c2.setProperty("foo", "bar");
+ c2.setProperty("p2", "d");
+
+ Tree c3 = path2.addChild("c3");
+ c3.setProperty("foo", "bar. some extra stuff... and then some to make it worse than c1");
+ c3.setProperty("p2", "d");
+
+ // create more stuff to get num_docs in index large and hence force union optimization
+ for (int i = 0; i < 10; i++) {
+ testRoot.addChild("extra" + i).setProperty("foo", "stuff");
+ }
+
+ root.commit();
+
+ List<String> expected = asList(c2.getPath(), c1.getPath(), c3.getPath());
+
+ // manual union
+ String query =
+ "select [jcr:path] from [nt:base] where contains(*, 'bar') and isdescendantnode('" + path1.getPath() + "')" +
+ " union " +
+ "select [jcr:path] from [nt:base] where contains(*, 'bar') and isdescendantnode('" + path2.getPath() + "')" +
+ " order by [jcr:score] desc";
+
+ assertEquals(expected, executeQuery(query, SQL2));
+
+ // no union (estimated fulltext without union would be same as sub-queries and it won't be optimized
+ query = "select [jcr:path] from [nt:base] where contains(*, 'bar')" +
+ " and (isdescendantnode('" + path1.getPath() + "') or" +
+ " isdescendantnode('" + path2.getPath() + "'))" +
+ " order by [jcr:score] desc";
+
+ assertEquals(expected, executeQuery(query, SQL2));
+
+ // optimization UNION as we're adding constraints to sub-queries that would improve cost of optimized union
+ query = "select [jcr:path] from [nt:base] where contains(*, 'bar')" +
+ " and ( (p1 = 'd' and isdescendantnode('" + path1.getPath() + "')) or" +
+ " (p2 = 'd' and isdescendantnode('" + path2.getPath() + "')))" +
+ " order by [jcr:score] desc";
+
+ assertEquals(expected, executeQuery(query, SQL2));
+ }
+
+ private void assertXpathPlan(String query, String planExpectation) throws ParseException {
+ assertThat(explainXpath(query), containsString(planExpectation));
+ }
+
+ private String explainXpath(String query) throws ParseException {
+ String explain = "explain " + query;
+ Result result = executeQuery(explain, "xpath", NO_BINDINGS);
+ ResultRow row = Iterables.getOnlyElement(result.getRows());
+ return row.getValue("plan").getValue(Type.STRING);
+ }
+
+ protected void assertOrderedQuery(String sql, List<String> paths) {
+ List<String> result = executeQuery(sql, AbstractQueryTest.SQL2, true, true);
+ assertEquals(paths, result);
+ }
+
+}