[calcite] branch master updated: [CALCITE-3054] Elasticsearch adapter. Avoid scripting for simple projections

[se...@apache.org]

This is an automated email from the ASF dual-hosted git repository.

sereda pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/calcite.git


The following commit(s) were added to refs/heads/master by this push:
     new f536347  [CALCITE-3054] Elasticsearch adapter. Avoid scripting for simple projections
f536347 is described below

commit f5363478e1feed6a090b2ac0c9fc52743a653ca0
Author: Andrei Sereda <25...@users.noreply.github.com>
AuthorDate: Wed May 8 01:18:02 2019 -0400

    [CALCITE-3054] Elasticsearch adapter. Avoid scripting for simple projections
    
    Simple query like below is using `script` when `_source` is sufficient.
    ```sql
    select  _MAP['a'], _MAP['b.a'] from elastic
    ```
---
 .../elasticsearch/ElasticsearchProject.java        | 18 ++++++++++-------
 .../adapter/elasticsearch/Projection2Test.java     | 23 ++++++++++++++++++++++
 2 files changed, 34 insertions(+), 7 deletions(-)

diff --git a/elasticsearch/src/main/java/org/apache/calcite/adapter/elasticsearch/ElasticsearchProject.java b/elasticsearch/src/main/java/org/apache/calcite/adapter/elasticsearch/ElasticsearchProject.java
index 0fffd49..1e08fc1 100644
--- a/elasticsearch/src/main/java/org/apache/calcite/adapter/elasticsearch/ElasticsearchProject.java
+++ b/elasticsearch/src/main/java/org/apache/calcite/adapter/elasticsearch/ElasticsearchProject.java
@@ -64,18 +64,19 @@ public class ElasticsearchProject extends Project implements ElasticsearchRel {
 
     final List<String> fields = new ArrayList<>();
     final List<String> scriptFields = new ArrayList<>();
+    // registers wherever "select *" is present
     boolean hasSelectStar = false;
     for (Pair<RexNode, String> pair: getNamedProjects()) {
       final String name = pair.right;
       final String expr = pair.left.accept(translator);
 
-      if (ElasticsearchRules.isItem(pair.left)) {
-        implementor.addExpressionItemMapping(name, ElasticsearchRules.stripQuotes(expr));
-      }
-
+      // "select *" present ?
       hasSelectStar |= ElasticsearchConstants.isSelectAll(name);
 
-      if (expr.equals(name)) {
+      if (ElasticsearchRules.isItem(pair.left)) {
+        implementor.addExpressionItemMapping(name, expr);
+        fields.add(expr);
+      } else if (expr.equals(name)) {
         fields.add(name);
       } else if (expr.matches("\"literal\":.+")) {
         scriptFields.add(ElasticsearchRules.quote(name)
@@ -96,9 +97,12 @@ public class ElasticsearchProject extends Project implements ElasticsearchRel {
       return;
     }
 
-    StringBuilder query = new StringBuilder();
+    final StringBuilder query = new StringBuilder();
     if (scriptFields.isEmpty()) {
-      List<String> newList = fields.stream().map(ElasticsearchRules::quote)
+      List<String> newList = fields.stream()
+          // _id field is available implicitly
+          .filter(f -> !ElasticsearchConstants.ID.equals(f))
+          .map(ElasticsearchRules::quote)
           .collect(Collectors.toList());
 
       final String findString = String.join(", ", newList);
diff --git a/elasticsearch/src/test/java/org/apache/calcite/adapter/elasticsearch/Projection2Test.java b/elasticsearch/src/test/java/org/apache/calcite/adapter/elasticsearch/Projection2Test.java
index 881bb81..a39067a 100644
--- a/elasticsearch/src/test/java/org/apache/calcite/adapter/elasticsearch/Projection2Test.java
+++ b/elasticsearch/src/test/java/org/apache/calcite/adapter/elasticsearch/Projection2Test.java
@@ -21,6 +21,7 @@ import org.apache.calcite.schema.SchemaPlus;
 import org.apache.calcite.schema.impl.ViewTable;
 import org.apache.calcite.schema.impl.ViewTableMacro;
 import org.apache.calcite.test.CalciteAssert;
+import org.apache.calcite.test.ElasticsearchChecker;
 import org.apache.calcite.util.TestUtil;
 
 import com.fasterxml.jackson.databind.node.ObjectNode;
@@ -193,6 +194,28 @@ public class Projection2Test {
   }
 
   /**
+   * Avoid using scripting for simple projections
+   *
+   * <p> When projecting simple fields (without expression) no
+   * <a href="https://www.elastic.co/guide/en/elasticsearch/reference/current/modules-scripting.html">scripting</a>
+   * should be used just
+   * <a href="https://www.elastic.co/guide/en/elasticsearch/reference/current/search-request-source-filtering.html">_source</a>
+   */
+  @Test
+  public void simpleProjectionNoScripting() {
+    CalciteAssert.that()
+        .with(newConnectionFactory())
+        .query(
+            String.format(Locale.ROOT, "select _MAP['_id'], _MAP['a'], _MAP['b.a'] from "
+                + " \"elastic\".\"%s\" where _MAP['b.a'] = 2", NAME))
+        .queryContains(
+            ElasticsearchChecker.elasticsearchChecker("'query.constant_score.filter.term.b.a':2",
+            "_source:['a', 'b.a']", "size:5196"))
+        .returns(regexMatch("EXPR$0=\\p{Graph}+; EXPR$1=1; EXPR$2=2"));
+
+  }
+
+  /**
    * Allows values to contain regular expressions instead of exact values.
    * <pre>
    *   {@code