[calcite] branch master updated: [CALCITE-3912] Incorrect mapping parsing when properties have same name as reserved keywords in ElasticSearch

[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 3755eb5  [CALCITE-3912] Incorrect mapping parsing when properties have same name as reserved keywords in ElasticSearch
3755eb5 is described below

commit 3755eb5871860f1fd5dc51990129784caa8ac0a4
Author: Andrei Sereda <25...@users.noreply.github.com>
AuthorDate: Fri Apr 10 12:22:22 2020 -0400

    [CALCITE-3912] Incorrect mapping parsing when properties have same name as reserved keywords in ElasticSearch
    
    If a property has name `type` or `properties` ES adapter doesn't correctly process the mapping structure. It doesn't correctly identify the leafs (properties)
    in JSON mapping.
---
 .../adapter/elasticsearch/ElasticsearchJson.java     |  9 +++++++--
 .../adapter/elasticsearch/ElasticsearchJsonTest.java | 20 ++++++++++++++++++++
 2 files changed, 27 insertions(+), 2 deletions(-)

diff --git a/elasticsearch/src/main/java/org/apache/calcite/adapter/elasticsearch/ElasticsearchJson.java b/elasticsearch/src/main/java/org/apache/calcite/adapter/elasticsearch/ElasticsearchJson.java
index e12dcc6..7e1b35f 100644
--- a/elasticsearch/src/main/java/org/apache/calcite/adapter/elasticsearch/ElasticsearchJson.java
+++ b/elasticsearch/src/main/java/org/apache/calcite/adapter/elasticsearch/ElasticsearchJson.java
@@ -45,6 +45,7 @@ import java.util.Optional;
 import java.util.Set;
 import java.util.function.BiConsumer;
 import java.util.function.Consumer;
+import java.util.function.Predicate;
 import java.util.stream.StreamSupport;
 
 import static java.util.Collections.unmodifiableMap;
@@ -107,13 +108,17 @@ final class ElasticsearchJson {
       return;
     }
 
-    if (mapping.has("properties")) {
+    // check if we have reached actual field mapping (leaf of JSON tree)
+    Predicate<JsonNode> isLeaf = node -> node.path("type").isValueNode();
+
+    if (mapping.path("properties").isObject()
+        && !isLeaf.test(mapping.path("properties"))) {
       // recurse
       visitMappingProperties(path, (ObjectNode) mapping.get("properties"), consumer);
       return;
     }
 
-    if (mapping.has("type")) {
+    if (isLeaf.test(mapping)) {
       // this is leaf (register field / type mapping)
       consumer.accept(String.join(".", path), mapping.get("type").asText());
       return;
diff --git a/elasticsearch/src/test/java/org/apache/calcite/adapter/elasticsearch/ElasticsearchJsonTest.java b/elasticsearch/src/test/java/org/apache/calcite/adapter/elasticsearch/ElasticsearchJsonTest.java
index 380a49b..10c6cd5 100644
--- a/elasticsearch/src/test/java/org/apache/calcite/adapter/elasticsearch/ElasticsearchJsonTest.java
+++ b/elasticsearch/src/test/java/org/apache/calcite/adapter/elasticsearch/ElasticsearchJsonTest.java
@@ -18,11 +18,13 @@ package org.apache.calcite.adapter.elasticsearch;
 
 import com.fasterxml.jackson.core.JsonParser;
 import com.fasterxml.jackson.databind.ObjectMapper;
+import com.fasterxml.jackson.databind.node.ObjectNode;
 
 import org.junit.jupiter.api.BeforeEach;
 import org.junit.jupiter.api.Test;
 
 import java.util.ArrayList;
+import java.util.HashMap;
 import java.util.List;
 import java.util.Map;
 
@@ -171,4 +173,22 @@ class ElasticsearchJsonTest {
     assertThat(rows.get(1).get("max"), is(42));
   }
 
+  /**
+   * Validate that property names which are reserved keywords ES
+   * are correctly mapped (eg. {@code type} or {@code properties})
+   */
+  @Test void reservedKeywordMapping() throws Exception {
+    // have special property names: type and properties
+    ObjectNode mapping = mapper.readValue("{properties:{"
+        + "type:{type:'text'},"
+        + "keyword:{type:'keyword'},"
+        + "properties:{type:'long'}"
+        + "}}", ObjectNode.class);
+    Map<String, String> result = new HashMap<>();
+    ElasticsearchJson.visitMappingProperties(mapping, result::put);
+
+    assertThat(result.get("type"), is("text"));
+    assertThat(result.get("keyword"), is("keyword"));
+    assertThat(result.get("properties"), is("long"));
+  }
 }