This is an automated email from the ASF dual-hosted git repository. jhyde pushed a commit to branch main in repository https://gitbox.apache.org/repos/asf/calcite.git
commit d0dae9bae0839c366344cdb4cc96b327aa058949 Author: Tim Grein <[email protected]> AuthorDate: Thu Jul 25 18:59:02 2024 +0200 [CALCITE-6498] Elasticsearch multi-field mappings do not work Handle creation of multi-field and nested mappings correctly in EmbeddedElasticsearchPolicy and ElasticsearchJson. Close apache/calcite#3885 --- .../adapter/elasticsearch/ElasticsearchJson.java | 39 ++++-- .../elasticsearch/ElasticsearchJsonTest.java | 114 +++++++++++++++++ .../elasticsearch/EmbeddedElasticsearchPolicy.java | 13 +- .../EmbeddedElasticsearchPolicyTest.java | 139 +++++++++++++++++++++ .../adapter/elasticsearch/Projection2Test.java | 10 +- 5 files changed, 304 insertions(+), 11 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 17ebde1cd0..284e1e95f0 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 @@ -67,7 +67,8 @@ final class ElasticsearchJson { /** * Visits leaves of the aggregation where all values are stored. */ - static void visitValueNodes(Aggregations aggregations, Consumer<Map<String, Object>> consumer) { + static void visitValueNodes(Aggregations aggregations, + Consumer<Map<String, Object>> consumer) { requireNonNull(aggregations, "aggregations"); requireNonNull(consumer, "consumer"); @@ -95,7 +96,14 @@ final class ElasticsearchJson { * Visits Elasticsearch * <a href="https://www.elastic.co/guide/en/elasticsearch/reference/current/mapping.html";>mapping * properties</a> and calls consumer for each {@code field / type} pair. - * Nested fields are represented as {@code foo.bar.qux}. + * + * <p>Nested fields are represented as {@code foo.bar.qux}. + * + * <p>Also supports + * <a href="https://www.elastic.co/guide/en/elasticsearch/reference/current/multi-fields.html";> + * multi-field mappings</a>. + * These fields are also represented as {@code foo.bar} with the difference + * that the type of the parent cannot be "nested". */ static void visitMappingProperties(ObjectNode mapping, BiConsumer<String, String> consumer) { @@ -113,19 +121,30 @@ final class ElasticsearchJson { return; } - // check if we have reached actual field mapping (leaf of JSON tree) + // check if we've reached a leaf Predicate<JsonNode> isLeaf = node -> node.path("type").isValueNode(); + // "properties" is present under the root or under "nested" fields if (mapping.path("properties").isObject() && !isLeaf.test(mapping.path("properties"))) { - // recurse - visitMappingProperties(path, (ObjectNode) mapping.get("properties"), consumer); + // recurse on "nested" field + visitMappingProperties(path, (ObjectNode) mapping.get("properties"), + consumer); + return; + } + + // "fields" is used for multi-fields + if (mapping.path("fields").isObject() + && !isLeaf.test(mapping.path("fields"))) { + // recurse on multi-field + visitMappingProperties(path, (ObjectNode) mapping.get("fields"), + consumer); return; } if (isLeaf.test(mapping)) { - // this is leaf (register field / type mapping) - consumer.accept(String.join(".", path), mapping.get("type").asText()); + // if we reached a leaf we can stop as we've already registered the type + // mapping return; } @@ -135,6 +154,12 @@ final class ElasticsearchJson { final String name = entry.getKey(); final ObjectNode node = (ObjectNode) entry.getValue(); path.add(name); + + // type is present + if (node.get("type") != null) { + consumer.accept(String.join(".", path), node.get("type").asText()); + } + visitMappingProperties(path, node, consumer); path.removeLast(); } 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 3cf7dabcf5..5ee45c86a5 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 @@ -16,8 +16,10 @@ */ package org.apache.calcite.adapter.elasticsearch; import com.fasterxml.jackson.core.JsonParser; +import com.fasterxml.jackson.core.JsonProcessingException; import com.fasterxml.jackson.databind.ObjectMapper; import com.fasterxml.jackson.databind.node.ObjectNode; +import com.google.common.collect.ImmutableMap; import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Test; @@ -218,4 +220,116 @@ class ElasticsearchJsonTest { ElasticsearchJson.visitMappingProperties(mapping, result::put); assertThat(result, anEmptyMap()); } + + /** Test case for + * <a href="https://issues.apache.org/jira/browse/CALCITE-6498";>[CALCITE-6498] + * Elasticsearch multi-field mappings do not work</a>. */ + @Test void testVisitMappingPropertiesWithMultipleSingleFieldMappings() + throws JsonProcessingException { + ObjectNode mapping = + mapper.readValue("{'properties':{" + + "'title':{'type':'text'}," + + "'name':{'type':'keyword'}" + + "}}", ObjectNode.class); + + Map<String, String> result = getMappingAsMap(mapping); + + assertThat(result.get("title"), is("text")); + assertThat(result.get("name"), is("keyword")); + } + + /** Test case for + * <a href="https://issues.apache.org/jira/browse/CALCITE-6498";>[CALCITE-6498] + * Elasticsearch multi-field mappings do not work</a>. */ + @Test void testVisitMappingPropertiesWithMultipleMultiFieldMappings() + throws Exception { + ObjectNode mapping = + mapper.readValue("{'properties':{" + + "'title':{'type':'text'," + + "'fields':{'keyword':{'type': 'keyword'}}" + + "}," + + "'name':{'type':'text'," + + "'fields':{'name_keyword':{'type': 'keyword'}}" + + "}" + + "}}", ObjectNode.class); + + Map<String, String> result = getMappingAsMap(mapping); + + assertThat(result.get("title"), is("text")); + assertThat(result.get("title.keyword"), is("keyword")); + assertThat(result.get("name"), is("text")); + assertThat(result.get("name.name_keyword"), is("keyword")); + } + + /** Test case for + * <a href="https://issues.apache.org/jira/browse/CALCITE-6498";>[CALCITE-6498] + * Elasticsearch multi-field mappings do not work</a>. */ + @Test void testVisitMappingPropertiesWithMultipleNestedFieldMappings() + throws Exception { + ObjectNode mapping = + mapper.readValue("{properties:{" + + "'author':{'type':'nested'," + + "'properties':{" + + "'name':{'type':'text'}," + + "'age':{'type':'integer'}" + + "}}," + + "'address':{'type':'nested'," + + "'properties':{" + + "'street':{'type':'keyword'}," + + "'zip':{'type':'integer'}" + + "}}" + + "}}", ObjectNode.class); + + Map<String, String> result = getMappingAsMap(mapping); + + assertThat(result.get("author"), is("nested")); + assertThat(result.get("author.name"), is("text")); + assertThat(result.get("author.age"), is("integer")); + + assertThat(result.get("address"), is("nested")); + assertThat(result.get("address.street"), is("keyword")); + assertThat(result.get("address.zip"), is("integer")); + } + + /** Test case for + * <a href="https://issues.apache.org/jira/browse/CALCITE-6498";>[CALCITE-6498] + * Elasticsearch multi-field mappings do not work</a>. */ + @Test void testVisitMappingPropertiesWithNestedAndMultiFieldMappings() + throws Exception { + // 'title' is a multi-mapped field + // 'author' is a nested field ('author.name' is multi-mapped) + ObjectNode mapping = + mapper.readValue("{properties:{" + + "'title':{'type':'text'," + + "'fields':{'keyword':{'type': 'keyword'}}" + + "}," + + "'author':{'type':'nested'," + + "'properties':{" + + "'name':{'type':'text'," + + "'fields':{'keyword':{'type': 'keyword'}}}," + + "'age':{'type':'integer'}" + + "}" + + "}}}", ObjectNode.class); + + Map<String, String> result = getMappingAsMap(mapping); + + // Checking the multi-field mapping + assertThat(result.get("title"), is("text")); + assertThat(result.get("title.keyword"), is("keyword")); + + // Checking the nested mapping + assertThat(result.get("author"), is("nested")); + assertThat(result.get("author.name"), is("text")); + assertThat(result.get("author.name.keyword"), is("keyword")); + assertThat(result.get("author.age"), is("integer")); + } + + private static Map<String, String> getMappingAsMap(ObjectNode mapping) { + // ImmutableMap.Builder makes sure that we don't add the same key twice + // (would throw exception otherwise) + ImmutableMap.Builder<String, String> builder = ImmutableMap.builder(); + ElasticsearchJson.visitMappingProperties(mapping, builder::put); + + return builder.build(); + } } diff --git a/elasticsearch/src/test/java/org/apache/calcite/adapter/elasticsearch/EmbeddedElasticsearchPolicy.java b/elasticsearch/src/test/java/org/apache/calcite/adapter/elasticsearch/EmbeddedElasticsearchPolicy.java index cc1b8526ff..92e690fa4b 100644 --- a/elasticsearch/src/test/java/org/apache/calcite/adapter/elasticsearch/EmbeddedElasticsearchPolicy.java +++ b/elasticsearch/src/test/java/org/apache/calcite/adapter/elasticsearch/EmbeddedElasticsearchPolicy.java @@ -178,8 +178,17 @@ class EmbeddedElasticsearchPolicy { final int index = key.indexOf('.'); if (index > -1) { String prefix = key.substring(0, index); - String suffix = key.substring(index + 1, key.length()); - applyMapping(parent.withObject("/" + prefix).withObject("/properties"), suffix, type); + String suffix = key.substring(index + 1); + + if ("nested".equals(parent.get(prefix).get("type").asText())) { + // Nested field mapping + applyMapping(parent.withObject("/" + prefix).withObject("/properties"), + suffix, type); + } else { + // Multi-field mapping + applyMapping(parent.withObject("/" + prefix).withObject("/fields"), + suffix, type); + } } else { parent.withObject("/" + key).put("type", type); } diff --git a/elasticsearch/src/test/java/org/apache/calcite/adapter/elasticsearch/EmbeddedElasticsearchPolicyTest.java b/elasticsearch/src/test/java/org/apache/calcite/adapter/elasticsearch/EmbeddedElasticsearchPolicyTest.java new file mode 100644 index 0000000000..51e191d1ec --- /dev/null +++ b/elasticsearch/src/test/java/org/apache/calcite/adapter/elasticsearch/EmbeddedElasticsearchPolicyTest.java @@ -0,0 +1,139 @@ +/* + * 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.calcite.adapter.elasticsearch; + +import org.apache.http.HttpEntity; +import org.apache.http.util.EntityUtils; + +import com.fasterxml.jackson.databind.JsonNode; +import com.fasterxml.jackson.databind.ObjectMapper; +import com.google.common.collect.ImmutableMap; + +import org.elasticsearch.client.Request; +import org.elasticsearch.client.Response; +import org.junit.jupiter.api.Test; +import org.junit.jupiter.api.parallel.ResourceAccessMode; +import org.junit.jupiter.api.parallel.ResourceLock; + +import java.io.IOException; +import java.util.Map; + +import static org.hamcrest.CoreMatchers.is; +import static org.hamcrest.MatcherAssert.assertThat; + +/** + * Tests for {@link EmbeddedElasticsearchPolicy}. + */ +@ResourceLock(value = "elasticsearch-scrolls", mode = ResourceAccessMode.READ) +public class EmbeddedElasticsearchPolicyTest { + + private static final EmbeddedElasticsearchPolicy NODE = + EmbeddedElasticsearchPolicy.create(); + + /** Test case for + * <a href="https://issues.apache.org/jira/browse/CALCITE-6498";>[CALCITE-6498] + * Elasticsearch multi-field mappings do not work</a>. */ + @Test void testCreateIndexWithSimpleFieldMappings() throws Exception { + final Map<String, String> mapping = + ImmutableMap.of("a", "keyword", "b", "text", "c", "long"); + final String simpleMappingIndex = "index_simple_mapping"; + + NODE.createIndex(simpleMappingIndex, mapping); + + final JsonNode properties = getMappings(simpleMappingIndex); + + assertThat(properties.path("a").path("type").asText(), is("keyword")); + assertThat(properties.path("b").path("type").asText(), is("text")); + assertThat(properties.path("c").path("type").asText(), is("long")); + } + + /** Test case for + * <a href="https://issues.apache.org/jira/browse/CALCITE-6498";>[CALCITE-6498] + * Elasticsearch multi-field mappings do not work</a>. */ + @Test void testCreateIndexWithNestedFieldMappings() throws Exception { + final Map<String, String> mapping = + ImmutableMap.of("a", "nested", "a.b", "text", "a.c", "long"); + final String index = "index_nested_field_mappings"; + + NODE.createIndex(index, mapping); + + final JsonNode properties = getMappings(index); + + assertThat(properties.path("a").path("type").asText(), is("nested")); + assertThat(properties.path("a") + .path("properties") + .path("b").path("type").asText(), is("text")); + assertThat(properties.path("a") + .path("properties") + .path("c").path("type").asText(), is("long")); + } + + /** Test case for + * <a href="https://issues.apache.org/jira/browse/CALCITE-6498";>[CALCITE-6498] + * Elasticsearch multi-field mappings do not work</a>. */ + @Test void testCreateIndexWithMultiFieldMappings() throws Exception { + final Map<String, String> mapping = + ImmutableMap.of("a", "text", "a.keyword", "keyword"); + final String index = "index_multi_field_mappings"; + + NODE.createIndex(index, mapping); + + final JsonNode properties = getMappings(index); + + assertThat(properties.path("a").path("type").asText(), is("text")); + assertThat(properties.path("a") + .path("fields") + .path("keyword").path("type").asText(), + is("keyword")); + } + + /** Test case for + * <a href="https://issues.apache.org/jira/browse/CALCITE-6498";>[CALCITE-6498] + * Elasticsearch multi-field mappings do not work</a>. */ + @Test void testCreateIndexWithNestedFieldMappingsAndMultiFieldMappings() + throws Exception { + final Map<String, String> mapping = + ImmutableMap.of("a", "nested", "a.b", "text", "a.b.keyword", "keyword"); + final String index = "index_nested_and_multi_field_mappings"; + + NODE.createIndex(index, mapping); + + final JsonNode properties = getMappings(index); + + assertThat(properties.path("a").path("type").asText(), is("nested")); + assertThat(properties.path("a") + .path("properties") + .path("b").path("type").asText(), is("text")); + assertThat(properties.path("a") + .path("properties") + .path("b").path("fields") + .path("keyword") + .path("type").asText(), is("keyword")); + } + + private static JsonNode getMappings(String index) throws IOException { + Response indexMappingsResponse = NODE.restClient() + .performRequest(new Request("GET", "/" + index + "/_mapping")); + HttpEntity entity = indexMappingsResponse.getEntity(); + String responseBody = EntityUtils.toString(entity); + JsonNode responseJson = new ObjectMapper().readTree(responseBody); + + // It's more readable to assert on a JsonNode than on a map, where you need + // to cast a lot + return responseJson.path(index).path("mappings").path("properties"); + } +} 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 9dd34be78d..c79ad24459 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 @@ -59,8 +59,14 @@ class Projection2Test { @BeforeAll public static void setupInstance() throws Exception { final Map<String, String> mappings = - ImmutableMap.of("a", "long", - "b.a", "long", "b.b", "long", "b.c.a", "keyword"); + ImmutableMap.<String, String>builder() + .put("a", "long") + .put("b", "nested") + .put("b.a", "long") + .put("b.b", "long") + .put("b.c", "nested") + .put("b.c.a", "keyword") + .build(); NODE.createIndex(NAME, mappings);
