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 2022/09/07 09:05:46 UTC

[jackrabbit-oak] branch trunk updated: Change the way that an in() property restriction in SQL2 is mapped to an Elastic query. Instead of using and or of range queries, one for each value in the set, use the terms query of Elastic. (#689)

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 a0bd343f66 Change the way that an in() property restriction in SQL2 is mapped to an Elastic query. Instead of using and or of range queries, one for each value in the set, use the terms query of Elastic. (#689)
a0bd343f66 is described below

commit a0bd343f668bbd8efbb49459087670a0c954cb1c
Author: Nuno Santos <ns...@adobe.com>
AuthorDate: Wed Sep 7 11:05:40 2022 +0200

    Change the way that an in() property restriction in SQL2 is mapped to an Elastic query. Instead of using and or of range queries, one for each value in the set, use the terms query of Elastic. (#689)
---
 .../elastic/util/TermQueryBuilderFactory.java      | 28 +++++++--
 .../index/elastic/ElasticPropertyIndexTest.java    | 68 +++++++++++++++++++++-
 2 files changed, 89 insertions(+), 7 deletions(-)

diff --git a/oak-search-elastic/src/main/java/org/apache/jackrabbit/oak/plugins/index/elastic/util/TermQueryBuilderFactory.java b/oak-search-elastic/src/main/java/org/apache/jackrabbit/oak/plugins/index/elastic/util/TermQueryBuilderFactory.java
index ffec8b7c90..ef93868b40 100644
--- a/oak-search-elastic/src/main/java/org/apache/jackrabbit/oak/plugins/index/elastic/util/TermQueryBuilderFactory.java
+++ b/oak-search-elastic/src/main/java/org/apache/jackrabbit/oak/plugins/index/elastic/util/TermQueryBuilderFactory.java
@@ -24,7 +24,6 @@ import org.apache.jackrabbit.oak.spi.query.Filter;
 import org.jetbrains.annotations.NotNull;
 
 import co.elastic.clients.elasticsearch._types.FieldValue;
-import co.elastic.clients.elasticsearch._types.query_dsl.BoolQuery;
 import co.elastic.clients.elasticsearch._types.query_dsl.Query;
 import co.elastic.clients.json.JsonData;
 
@@ -94,12 +93,29 @@ public class TermQueryBuilderFactory {
         }));
     }
 
-    private static <R> Query newInQuery(String field, List<R> values) {
-        BoolQuery.Builder bqBuilder = new BoolQuery.Builder();
-        for (R value : values) {
-            bqBuilder.should(newRangeQuery(field, value, value, true, true));
+    private static <R> FieldValue toFieldValue(R value) {
+        if (value instanceof Long) {
+            Long asLong = (Long) value;
+            return FieldValue.of(asLong);
+        } else if (value instanceof Double) {
+            Double asDouble = (Double) value;
+            return FieldValue.of(asDouble);
+        } else if (value instanceof Boolean) {
+            Boolean asBoolean = (Boolean) value;
+            return FieldValue.of(asBoolean);
+        } else {
+            return FieldValue.of(value.toString());
         }
-        return Query.of(q -> q.bool(bqBuilder.build()));
+    }
+
+    private static <R> Query newInQuery(String field, List<R> values) {
+        List<FieldValue> fieldValues = values.stream()
+                .map(TermQueryBuilderFactory::toFieldValue)
+                .collect(Collectors.toList());
+        return Query.of(q -> q.terms(tq -> tq
+                .field(field)
+                .terms(t -> t.value(fieldValues)))
+        );
     }
 
     public static <R> Query newPropertyRestrictionQuery(String propertyName, Filter.PropertyRestriction pr,
diff --git a/oak-search-elastic/src/test/java/org/apache/jackrabbit/oak/plugins/index/elastic/ElasticPropertyIndexTest.java b/oak-search-elastic/src/test/java/org/apache/jackrabbit/oak/plugins/index/elastic/ElasticPropertyIndexTest.java
index f3bdd13c40..db066e1909 100644
--- a/oak-search-elastic/src/test/java/org/apache/jackrabbit/oak/plugins/index/elastic/ElasticPropertyIndexTest.java
+++ b/oak-search-elastic/src/test/java/org/apache/jackrabbit/oak/plugins/index/elastic/ElasticPropertyIndexTest.java
@@ -16,7 +16,10 @@
  */
 package org.apache.jackrabbit.oak.plugins.index.elastic;
 
+import com.google.common.collect.ImmutableList;
+import org.apache.jackrabbit.oak.api.CommitFailedException;
 import org.apache.jackrabbit.oak.api.Tree;
+import org.apache.jackrabbit.oak.plugins.index.elastic.util.ElasticIndexDefinitionBuilder;
 import org.apache.jackrabbit.oak.plugins.index.search.util.IndexDefinitionBuilder;
 import org.junit.Test;
 
@@ -51,7 +54,7 @@ public class ElasticPropertyIndexTest extends ElasticAbstractQueryTest {
 
         // Now we test for 250 < nodes < 500
 
-        for (int i = 250 ; i < 300 ; i ++) {
+        for (int i = 250; i < 300; i++) {
             test.addChild("a" + i).setProperty("propa", "foo" + i);
         }
         root.commit();
@@ -140,4 +143,67 @@ public class ElasticPropertyIndexTest extends ElasticAbstractQueryTest {
                 containsString("elasticsearch:test1")));
     }
 
+
+    @Test
+    public void inOperandStringValues() throws Exception {
+        String query = "select [jcr:path] from [nt:base] where [propa] in(\"a\", \"e\", \"i\")";
+        createIndexOfType("String");
+
+        Tree test = root.getTree("/").addChild("test");
+        for (char ch = 'a'; ch <= 'z'; ch++) {
+            test.addChild("node-" + ch).setProperty("propa", Character.toString(ch));
+        }
+        root.commit();
+        assertEventually(() -> {
+            assertThat(explain(query), containsString("{\"terms\":{\"propa\":[\"a\",\"e\",\"i\"]}}"));
+            assertQuery(query, SQL2, ImmutableList.of("/test/node-a", "/test/node-e", "/test/node-i"));
+        });
+    }
+
+    @Test
+    public void inOperandLongValues() throws Exception {
+        String query = "select [jcr:path] from [nt:base] where [propa] in(2, 3, 5, 7)";
+        createIndexOfType("Long");
+
+        Tree test = root.getTree("/").addChild("test");
+        for (int i = 0; i < 10; i++) {
+            test.addChild("node-" + i).setProperty("propa", Integer.toString(i));
+        }
+        root.commit();
+
+        assertEventually(() -> {
+            assertThat(explain(query), containsString("{\"terms\":{\"propa\":[2,3,5,7]}}"));
+            assertQuery(query, SQL2, ImmutableList.of("/test/node-2", "/test/node-3", "/test/node-5", "/test/node-7"));
+        });
+    }
+
+    @Test
+    public void inOperandDoubleValues() throws Exception {
+        String query = "select [jcr:path] from [nt:base] where [propa] in(2.0, 3.0, 5.0, 7.0)";
+
+        createIndexOfType("Double");
+
+        Tree test = root.getTree("/").addChild("test");
+        for (int i = 0; i < 10; i++) {
+            test.addChild("node-" + i).setProperty("propa", (double)i);
+        }
+        root.commit();
+
+        assertEventually(() -> {
+            assertThat(explain(query), containsString("{\"terms\":{\"propa\":[2.0,3.0,5.0,7.0]}}"));
+            assertQuery(query, SQL2, ImmutableList.of("/test/node-2", "/test/node-3", "/test/node-5", "/test/node-7"));
+        });
+    }
+
+    private void createIndexOfType(String type) throws CommitFailedException {
+        IndexDefinitionBuilder builder = new ElasticIndexDefinitionBuilder();
+        builder.noAsync();
+        builder.indexRule("nt:base")
+                .property("propa")
+                .propertyIndex()
+                .type(type);
+
+        setIndex("test", builder);
+        root.commit();
+    }
 }