Merge pull request #657 from nfsantos/OAK-9881
OAK-9881 - Unreachable code in the logic that processes like constraints in Elastic
diff --git a/oak-lucene/src/main/java/org/apache/jackrabbit/oak/plugins/index/lucene/LuceneIndex.java b/oak-lucene/src/main/java/org/apache/jackrabbit/oak/plugins/index/lucene/LuceneIndex.java
index d5dc804..38296f8 100644
--- a/oak-lucene/src/main/java/org/apache/jackrabbit/oak/plugins/index/lucene/LuceneIndex.java
+++ b/oak-lucene/src/main/java/org/apache/jackrabbit/oak/plugins/index/lucene/LuceneIndex.java
@@ -765,6 +765,12 @@
}
if (isLike) {
+ // Note: the code below has two problems:
+ // - Does not deal with escaped wildcard characters (OAK-9885).
+ // - Does not apply the prefix query optimization: the first block of the if condition below is
+ // never executed because the guard condition is always false (OAK-9881).
+ // The correct logic is in LucenePropertyIndex#createLikeQuery.
+ // Leaving the code as it is, because this class is deprecated, as it is just for compatVersion=1
first = first.replace('%', WildcardQuery.WILDCARD_STRING);
first = first.replace('_', WildcardQuery.WILDCARD_CHAR);
@@ -773,7 +779,7 @@
int len = first.length();
if (indexOfWS == len || indexOfWC == len) {
- // remove trailing "*" for prefixquery
+ // remove trailing "*" for prefix query
first = first.substring(0, first.length() - 1);
if (JCR_PATH.equals(name)) {
qs.add(new PrefixQuery(newPathTerm(first)));
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 930512e..4c3de80 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
@@ -1193,76 +1193,6 @@
}
@Test
- public void likeQueriesWithString() throws Exception {
- Tree idx = createIndex("test1", of("propa", "propb"));
- idx.addChild(PROP_NODE).addChild("propa");
- root.commit();
-
- Tree test = root.getTree("/").addChild("test");
- test.addChild("a").setProperty("propa", "humpty");
- test.addChild("b").setProperty("propa", "dumpty");
- test.addChild("c").setProperty("propa", "humpy");
- root.commit();
-
- assertQuery("select [jcr:path] from [nt:base] where propa like 'hum%'",
- asList("/test/a", "/test/c"));
- assertQuery("select [jcr:path] from [nt:base] where propa like '%ty'",
- asList("/test/a", "/test/b"));
- assertQuery("select [jcr:path] from [nt:base] where propa like '%ump%'",
- asList("/test/a", "/test/b", "/test/c"));
- }
-
- @Test
- public void likeQueriesWithEscapedChars() throws Exception {
- Tree idx = createIndex("test1", of("propa", "propb"));
- idx.addChild(PROP_NODE).addChild("propa");
- root.commit();
-
- Tree test = root.getTree("/").addChild("test");
- test.addChild("a").setProperty("propa", "foo%");
- test.addChild("b").setProperty("propa", "%bar");
- test.addChild("c").setProperty("propa", "foo%bar");
- test.addChild("d").setProperty("propa", "foo_");
- test.addChild("e").setProperty("propa", "_foo");
- test.addChild("f").setProperty("propa", "foo_bar");
- test.addChild("g").setProperty("propa", "foo%_bar");
- test.addChild("h").setProperty("propa", "foo\\bar");
- test.addChild("i").setProperty("propa", "foo\\\\%bar");
- root.commit();
-
- assertQuery("select [jcr:path] from [nt:base] where propa like 'foo%'",
- asList("/test/a", "/test/c", "/test/d", "/test/f", "/test/g", "/test/h", "/test/i"));
- assertQuery("select [jcr:path] from [nt:base] where propa like '%oo%'",
- asList("/test/a", "/test/c", "/test/d", "/test/e", "/test/f", "/test/g", "/test/h", "/test/i"));
- assertQuery("select [jcr:path] from [nt:base] where propa like 'foo\\%'",
- asList("/test/a"));
- assertQuery("select [jcr:path] from [nt:base] where propa like '%oo\\%'",
- asList("/test/a"));
- assertQuery("select [jcr:path] from [nt:base] where propa like '%oo\\%%'",
- asList("/test/a", "/test/c", "/test/g"));
- assertQuery("select [jcr:path] from [nt:base] where propa like '\\%b%'",
- asList("/test/b"));
- assertQuery("select [jcr:path] from [nt:base] where propa like 'foo_'",
- asList("/test/a", "/test/d"));
- assertQuery("select [jcr:path] from [nt:base] where propa like '_oo_'",
- asList("/test/a", "/test/d"));
- assertQuery("select [jcr:path] from [nt:base] where propa like 'foo\\_'",
- asList("/test/d"));
- assertQuery("select [jcr:path] from [nt:base] where propa like '%oo\\_'",
- asList("/test/d"));
- assertQuery("select [jcr:path] from [nt:base] where propa like '%oo\\_%'",
- asList("/test/d", "/test/f"));
- assertQuery("select [jcr:path] from [nt:base] where propa like '%oo\\%\\_%'",
- asList("/test/g"));
- assertQuery("select [jcr:path] from [nt:base] where propa like 'foo\\\\bar'",
- asList("/test/h"));
- assertQuery("select [jcr:path] from [nt:base] where propa like '%\\\\%'",
- asList("/test/h", "/test/i"));
- assertQuery("select [jcr:path] from [nt:base] where propa like '%\\\\\\%%'",
- asList("/test/i"));
- }
-
- @Test
public void nativeQueries() throws Exception {
Tree idx = createIndex("test1", of("propa", "propb"));
idx.addChild(PROP_NODE).addChild("propa");
diff --git a/oak-search-elastic/src/main/java/org/apache/jackrabbit/oak/plugins/index/elastic/query/ElasticRequestHandler.java b/oak-search-elastic/src/main/java/org/apache/jackrabbit/oak/plugins/index/elastic/query/ElasticRequestHandler.java
index 717dbec..3e51a86 100644
--- a/oak-search-elastic/src/main/java/org/apache/jackrabbit/oak/plugins/index/elastic/query/ElasticRequestHandler.java
+++ b/oak-search-elastic/src/main/java/org/apache/jackrabbit/oak/plugins/index/elastic/query/ElasticRequestHandler.java
@@ -41,7 +41,6 @@
import java.util.List;
import java.util.Map;
import java.util.Objects;
-import java.util.Optional;
import java.util.concurrent.atomic.AtomicReference;
import java.util.function.BiConsumer;
import java.util.function.BiPredicate;
@@ -208,7 +207,7 @@
}
);
}
-
+
public @NotNull List<SortOptions> baseSorts() {
List<QueryIndex.OrderEntry> sortOrder = indexPlan.getSortOrder();
if (sortOrder == null || sortOrder.isEmpty()) {
@@ -275,7 +274,7 @@
return filter.getPropertyRestrictions().stream()
.anyMatch(pr -> QueryConstants.REP_FACET.equals(pr.propertyName));
}
-
+
public Map<String, Aggregation> aggregations() {
return facetFields().collect(Collectors.toMap(Function.identity(), facetProp -> Aggregation.of(af ->
af.terms(tf -> tf.field(elasticIndexDefinition.getElasticKeyword(facetProp))
@@ -757,16 +756,16 @@
first = first.replace('%', WildcardQuery.WILDCARD_STRING);
first = first.replace('_', WildcardQuery.WILDCARD_CHAR);
- int indexOfWS = first.indexOf(WildcardQuery.WILDCARD_STRING);
- int indexOfWC = first.indexOf(WildcardQuery.WILDCARD_CHAR);
- int len = first.length();
+ // If the query ends in a wildcard string (*) and has no other wildcard characters, use a prefix match query
+ boolean hasSingleWildcardStringAtEnd = first.indexOf(WildcardQuery.WILDCARD_STRING) == first.length() - 1;
+ boolean doesNotContainWildcardChar = first.indexOf(WildcardQuery.WILDCARD_CHAR) == -1;
// Non full text (Non analyzed) properties are keyword types in ES. For those field would be equal to name.
// Analyzed properties, however are of text type on which we can't perform wildcard or prefix queries so we use the keyword (sub) field
// by appending .keyword to the name here.
String field = elasticIndexDefinition.getElasticKeyword(name);
- if (indexOfWS == len || indexOfWC == len) {
+ if (hasSingleWildcardStringAtEnd && doesNotContainWildcardChar) {
// remove trailing "*" for prefix query
first = first.substring(0, first.length() - 1);
if (JCR_PATH.equals(name)) {
diff --git a/oak-search-elastic/src/test/java/org/apache/jackrabbit/oak/plugins/index/elastic/ElasticPropertyIndexCommonTest.java b/oak-search-elastic/src/test/java/org/apache/jackrabbit/oak/plugins/index/elastic/ElasticPropertyIndexCommonTest.java
index a5b4980..5f9a985 100644
--- a/oak-search-elastic/src/test/java/org/apache/jackrabbit/oak/plugins/index/elastic/ElasticPropertyIndexCommonTest.java
+++ b/oak-search-elastic/src/test/java/org/apache/jackrabbit/oak/plugins/index/elastic/ElasticPropertyIndexCommonTest.java
@@ -19,6 +19,8 @@
import org.apache.jackrabbit.oak.api.ContentRepository;
import org.apache.jackrabbit.oak.plugins.index.PropertyIndexCommonTest;
import org.junit.ClassRule;
+import org.junit.Ignore;
+import org.junit.Test;
public class ElasticPropertyIndexCommonTest extends PropertyIndexCommonTest {
@@ -41,4 +43,11 @@
setTraversalEnabled(false);
}
+ @Override
+ @Test
+ @Ignore("OAK-9885")
+ public void likeQueriesWithEscapedChars() throws Exception {
+ super.likeQueriesWithEscapedChars();
+ }
+
}
diff --git a/oak-search/src/test/java/org/apache/jackrabbit/oak/plugins/index/PropertyIndexCommonTest.java b/oak-search/src/test/java/org/apache/jackrabbit/oak/plugins/index/PropertyIndexCommonTest.java
index 8ea99ec..87f41d0 100644
--- a/oak-search/src/test/java/org/apache/jackrabbit/oak/plugins/index/PropertyIndexCommonTest.java
+++ b/oak-search/src/test/java/org/apache/jackrabbit/oak/plugins/index/PropertyIndexCommonTest.java
@@ -16,6 +16,7 @@
*/
package org.apache.jackrabbit.oak.plugins.index;
+import com.google.common.collect.ImmutableList;
import org.apache.commons.io.IOUtils;
import org.apache.jackrabbit.JcrConstants;
import org.apache.jackrabbit.oak.api.Tree;
@@ -28,7 +29,6 @@
import java.util.Arrays;
-import static java.util.Arrays.asList;
import static java.util.Collections.singletonList;
import static javax.jcr.PropertyType.TYPENAME_DATE;
import static org.apache.jackrabbit.oak.plugins.index.search.FulltextIndexConstants.PROPDEF_PROP_NODE_NAME;
@@ -202,7 +202,7 @@
String query = "select [jcr:path] from [oak:TestNode] where [propa] is not null";
String explanation = explain(query);
assertThat(explanation, containsString(propertyExistenceQueryWithNullCheckExpectedExplain()));
- assertEventually(() -> assertQuery(query, asList("/test/a", "/test/b")));
+ assertEventually(() -> assertQuery(query, Arrays.asList("/test/a", "/test/b")));
}
protected String propertyExistenceQueryWithNullCheckExpectedExplain() {
@@ -269,12 +269,101 @@
Arrays.asList("/test/a", "/test/b", "/test/d")));
}
+ @Test
+ public void likeQueriesWithString() throws Exception {
+ indexOptions.setIndex(
+ root,
+ "test1",
+ indexOptions.createIndex(indexOptions.createIndexDefinitionBuilder(), false, "propa")
+ );
+
+ Tree test = root.getTree("/").addChild("test");
+ test.addChild("a").setProperty("propa", "humpty");
+ test.addChild("b").setProperty("propa", "dumpty");
+ test.addChild("c").setProperty("propa", "humpy");
+ test.addChild("d").setProperty("propa", "alice");
+ root.commit();
+
+ assertEventually(() -> assertQuery("select [jcr:path] from [nt:base] where propa like 'hum%'",
+ ImmutableList.of("/test/a", "/test/c")));
+ assertQuery("select [jcr:path] from [nt:base] where propa like '%ty'",
+ ImmutableList.of("/test/a", "/test/b"));
+ assertQuery("select [jcr:path] from [nt:base] where propa like '%ump%'",
+ ImmutableList.of("/test/a", "/test/b", "/test/c"));
+ assertQuery("select [jcr:path] from [nt:base] where propa like '_ump%'",
+ ImmutableList.of("/test/a", "/test/b", "/test/c"));
+ assertQuery("select [jcr:path] from [nt:base] where propa like 'a_ice%'",
+ ImmutableList.of("/test/d"));
+ assertQuery("select [jcr:path] from [nt:base] where propa like 'a_i_e%'",
+ ImmutableList.of("/test/d"));
+ assertQuery("select [jcr:path] from [nt:base] where propa like '_____'",
+ ImmutableList.of("/test/c", "/test/d"));
+ assertQuery("select [jcr:path] from [nt:base] where propa like 'h%y'",
+ ImmutableList.of("/test/a", "/test/c"));
+ assertQuery("select [jcr:path] from [nt:base] where propa like 'humpy'",
+ ImmutableList.of("/test/c"));
+ }
+
+ @Test
+ public void likeQueriesWithEscapedChars() throws Exception {
+ indexOptions.setIndex(
+ root,
+ "test1",
+ indexOptions.createIndex(indexOptions.createIndexDefinitionBuilder(), false, "propa")
+ );
+ Tree test = root.getTree("/").addChild("test");
+ test.addChild("a").setProperty("propa", "foo%");
+ test.addChild("b").setProperty("propa", "%bar");
+ test.addChild("c").setProperty("propa", "foo%bar");
+ test.addChild("d").setProperty("propa", "foo_");
+ test.addChild("e").setProperty("propa", "_foo");
+ test.addChild("f").setProperty("propa", "foo_bar");
+ test.addChild("g").setProperty("propa", "foo%_bar");
+ test.addChild("h").setProperty("propa", "foo\\bar");
+ test.addChild("i").setProperty("propa", "foo\\\\%bar");
+ root.commit();
+
+ assertEventually(() ->
+ assertQuery("select [jcr:path] from [nt:base] where propa like 'foo%'",
+ ImmutableList.of("/test/a", "/test/c", "/test/d", "/test/f", "/test/g", "/test/h", "/test/i"))
+ );
+
+ assertQuery("select [jcr:path] from [nt:base] where propa like '%oo%'",
+ ImmutableList.of("/test/a", "/test/c", "/test/d", "/test/e", "/test/f", "/test/g", "/test/h", "/test/i"));
+ assertQuery("select [jcr:path] from [nt:base] where propa like 'foo\\%'",
+ ImmutableList.of("/test/a"));
+ assertQuery("select [jcr:path] from [nt:base] where propa like '%oo\\%'",
+ ImmutableList.of("/test/a"));
+ assertQuery("select [jcr:path] from [nt:base] where propa like '%oo\\%%'",
+ ImmutableList.of("/test/a", "/test/c", "/test/g"));
+ assertQuery("select [jcr:path] from [nt:base] where propa like '\\%b%'",
+ ImmutableList.of("/test/b"));
+ assertQuery("select [jcr:path] from [nt:base] where propa like 'foo_'",
+ ImmutableList.of("/test/a", "/test/d"));
+ assertQuery("select [jcr:path] from [nt:base] where propa like '_oo_'",
+ ImmutableList.of("/test/a", "/test/d"));
+ assertQuery("select [jcr:path] from [nt:base] where propa like 'foo\\_'",
+ ImmutableList.of("/test/d"));
+ assertQuery("select [jcr:path] from [nt:base] where propa like '%oo\\_'",
+ ImmutableList.of("/test/d"));
+ assertQuery("select [jcr:path] from [nt:base] where propa like '%oo\\_%'",
+ ImmutableList.of("/test/d", "/test/f"));
+ assertQuery("select [jcr:path] from [nt:base] where propa like '%oo\\%\\_%'",
+ ImmutableList.of("/test/g"));
+ assertQuery("select [jcr:path] from [nt:base] where propa like 'foo\\\\bar'",
+ ImmutableList.of("/test/h"));
+ assertQuery("select [jcr:path] from [nt:base] where propa like '%\\\\%'",
+ ImmutableList.of("/test/h", "/test/i"));
+ assertQuery("select [jcr:path] from [nt:base] where propa like '%\\\\\\%%'",
+ ImmutableList.of("/test/i"));
+ }
+
protected String explain(String query) {
String explain = "explain " + query;
return executeQuery(explain, "JCR-SQL2").get(0);
}
- protected static Tree createNodeWithType(Tree t, String nodeName, String typeName){
+ protected static Tree createNodeWithType(Tree t, String nodeName, String typeName) {
t = t.addChild(nodeName);
t.setProperty(JcrConstants.JCR_PRIMARYTYPE, typeName, Type.NAME);
return t;
