From f335101253b52395b3cccf161cc79b34f6dac80b Mon Sep 17 00:00:00 2001 From: Yury-Fridlyand Date: Wed, 16 Nov 2022 13:30:08 -0800 Subject: [PATCH 1/3] First PoC. Signed-off-by: Yury-Fridlyand --- .../sql/data/model/ExprTimeValue.java | 15 +++ .../sql/data/model/ExprTimestampValue.java | 2 +- .../value/OpenSearchExprValueFactory.java | 110 ++++++++++++++---- .../sql/opensearch/mapping/IndexMapping.java | 14 +++ .../sql/opensearch/mapping/MappingEntry.java | 66 +++++++++++ .../OpenSearchDescribeIndexRequest.java | 18 +++ .../opensearch/storage/OpenSearchIndex.java | 3 +- 7 files changed, 204 insertions(+), 24 deletions(-) create mode 100644 opensearch/src/main/java/org/opensearch/sql/opensearch/mapping/MappingEntry.java diff --git a/core/src/main/java/org/opensearch/sql/data/model/ExprTimeValue.java b/core/src/main/java/org/opensearch/sql/data/model/ExprTimeValue.java index 6cc4021d2e..40d66fda7a 100644 --- a/core/src/main/java/org/opensearch/sql/data/model/ExprTimeValue.java +++ b/core/src/main/java/org/opensearch/sql/data/model/ExprTimeValue.java @@ -57,6 +57,21 @@ public LocalTime timeValue() { return time; } + @Override + public LocalDate dateValue() { + return LocalDate.now(); + } + + @Override + public LocalDateTime datetimeValue() { + return LocalDateTime.of(dateValue(), timeValue()); + } + + @Override + public Instant timestampValue() { + return ZonedDateTime.of(dateValue(), timeValue(), ExprTimestampValue.ZONE).toInstant(); + } + @Override public String toString() { return String.format("TIME '%s'", value()); diff --git a/core/src/main/java/org/opensearch/sql/data/model/ExprTimestampValue.java b/core/src/main/java/org/opensearch/sql/data/model/ExprTimestampValue.java index 219a4c2663..a7ae605a7f 100644 --- a/core/src/main/java/org/opensearch/sql/data/model/ExprTimestampValue.java +++ b/core/src/main/java/org/opensearch/sql/data/model/ExprTimestampValue.java @@ -30,7 +30,7 @@ public class ExprTimestampValue extends AbstractExprValue { /** * todo. only support UTC now. */ - private static final ZoneId ZONE = ZoneId.of("UTC"); + public static final ZoneId ZONE = ZoneId.of("UTC"); private final Instant timestamp; diff --git a/opensearch/src/main/java/org/opensearch/sql/opensearch/data/value/OpenSearchExprValueFactory.java b/opensearch/src/main/java/org/opensearch/sql/opensearch/data/value/OpenSearchExprValueFactory.java index 2536121e91..76f01a4920 100644 --- a/opensearch/src/main/java/org/opensearch/sql/opensearch/data/value/OpenSearchExprValueFactory.java +++ b/opensearch/src/main/java/org/opensearch/sql/opensearch/data/value/OpenSearchExprValueFactory.java @@ -30,16 +30,22 @@ import com.fasterxml.jackson.core.JsonProcessingException; import com.fasterxml.jackson.databind.ObjectMapper; import com.google.common.collect.ImmutableMap; +import java.time.DateTimeException; import java.time.Instant; +import java.time.LocalDate; +import java.time.LocalDateTime; +import java.time.LocalTime; import java.time.format.DateTimeParseException; +import java.time.temporal.TemporalAccessor; import java.util.ArrayList; import java.util.LinkedHashMap; import java.util.List; import java.util.Map; import java.util.Optional; -import java.util.function.Function; +import java.util.function.BiFunction; import lombok.Getter; import lombok.Setter; +import org.apache.logging.log4j.LogManager; import org.opensearch.common.time.DateFormatters; import org.opensearch.sql.data.model.ExprBooleanValue; import org.opensearch.sql.data.model.ExprByteValue; @@ -61,6 +67,7 @@ import org.opensearch.sql.opensearch.data.utils.Content; import org.opensearch.sql.opensearch.data.utils.ObjectContent; import org.opensearch.sql.opensearch.data.utils.OpenSearchJsonContent; +import org.opensearch.sql.opensearch.mapping.MappingEntry; import org.opensearch.sql.opensearch.response.agg.OpenSearchAggregationResponseParser; /** @@ -73,6 +80,9 @@ public class OpenSearchExprValueFactory { @Setter private Map typeMapping; + + private Map typeMapping2; + @Getter @Setter private OpenSearchAggregationResponseParser parser; @@ -81,26 +91,26 @@ public class OpenSearchExprValueFactory { private static final ObjectMapper OBJECT_MAPPER = new ObjectMapper(); - private final Map> typeActionMap = - new ImmutableMap.Builder>() - .put(INTEGER, c -> new ExprIntegerValue(c.intValue())) - .put(LONG, c -> new ExprLongValue(c.longValue())) - .put(SHORT, c -> new ExprShortValue(c.shortValue())) - .put(BYTE, c -> new ExprByteValue(c.byteValue())) - .put(FLOAT, c -> new ExprFloatValue(c.floatValue())) - .put(DOUBLE, c -> new ExprDoubleValue(c.doubleValue())) - .put(STRING, c -> new ExprStringValue(c.stringValue())) - .put(BOOLEAN, c -> ExprBooleanValue.of(c.booleanValue())) + private final Map> typeActionMap = + new ImmutableMap.Builder>() + .put(INTEGER, (c, m) -> new ExprIntegerValue(c.intValue())) + .put(LONG, (c, m) -> new ExprLongValue(c.longValue())) + .put(SHORT, (c, m) -> new ExprShortValue(c.shortValue())) + .put(BYTE, (c, m) -> new ExprByteValue(c.byteValue())) + .put(FLOAT, (c, m) -> new ExprFloatValue(c.floatValue())) + .put(DOUBLE, (c, m) -> new ExprDoubleValue(c.doubleValue())) + .put(STRING, (c, m) -> new ExprStringValue(c.stringValue())) + .put(BOOLEAN, (c, m) -> ExprBooleanValue.of(c.booleanValue())) .put(TIMESTAMP, this::parseTimestamp) - .put(DATE, c -> new ExprDateValue(parseTimestamp(c).dateValue().toString())) - .put(TIME, c -> new ExprTimeValue(parseTimestamp(c).timeValue().toString())) - .put(DATETIME, c -> new ExprDatetimeValue(parseTimestamp(c).datetimeValue())) - .put(OPENSEARCH_TEXT, c -> new OpenSearchExprTextValue(c.stringValue())) - .put(OPENSEARCH_TEXT_KEYWORD, c -> new OpenSearchExprTextKeywordValue(c.stringValue())) - .put(OPENSEARCH_IP, c -> new OpenSearchExprIpValue(c.stringValue())) - .put(OPENSEARCH_GEO_POINT, c -> new OpenSearchExprGeoPointValue(c.geoValue().getLeft(), + .put(DATE, (c, m) -> new ExprDateValue(parseTimestamp(c, m).dateValue().toString())) + .put(TIME, (c, m) -> new ExprTimeValue(parseTimestamp(c, m).timeValue().toString())) + .put(DATETIME, (c, m) -> new ExprDatetimeValue(parseTimestamp(c, m).datetimeValue())) + .put(OPENSEARCH_TEXT, (c, m) -> new OpenSearchExprTextValue(c.stringValue())) + .put(OPENSEARCH_TEXT_KEYWORD, (c, m) -> new OpenSearchExprTextKeywordValue(c.stringValue())) + .put(OPENSEARCH_IP, (c, m) -> new OpenSearchExprIpValue(c.stringValue())) + .put(OPENSEARCH_GEO_POINT, (c, m) -> new OpenSearchExprGeoPointValue(c.geoValue().getLeft(), c.geoValue().getRight())) - .put(OPENSEARCH_BINARY, c -> new OpenSearchExprBinaryValue(c.stringValue())) + .put(OPENSEARCH_BINARY, (c, m) -> new OpenSearchExprBinaryValue(c.stringValue())) .build(); /** @@ -111,6 +121,12 @@ public OpenSearchExprValueFactory( this.typeMapping = typeMapping; } + public OpenSearchExprValueFactory(Map typeMapping, + Map typeMapping2) { + this.typeMapping = typeMapping; + this.typeMapping2 = typeMapping2; + } + /** * The struct construction has the following assumption. 1. The field has OpenSearch Object * data type. https://www.elastic.co/guide/en/elasticsearch/reference/current/object.html 2. The @@ -151,7 +167,7 @@ private ExprValue parse(Content content, String field, Optional fieldT return parseArray(content, field); } else { if (typeActionMap.containsKey(type)) { - return typeActionMap.get(type).apply(content); + return typeActionMap.get(type).apply(content, typeMapping2.getOrDefault(field, null)); } else { throw new IllegalStateException( String.format( @@ -188,11 +204,61 @@ private ExprValue constructTimestamp(String value) { } } - private ExprValue parseTimestamp(Content value) { + // returns java.time.format.Parsed + private TemporalAccessor parseTimestampString(String value, MappingEntry mapping) { + if (mapping == null) { + return null; + } + for (var formatter : mapping.getRegularFormatters()) { + try { + return formatter.parse(value); + } catch (Exception ignored) { + // nothing to do, try another format + } + } + for (var formatter : mapping.getNamedFormatters()) { + try { + return formatter.parse(value); + } catch (Exception ignored) { + // nothing to do, try another format + } + } + return null; + } + + private ExprValue parseTimestamp(Content value, MappingEntry mapping) { if (value.isNumber()) { return new ExprTimestampValue(Instant.ofEpochMilli(value.longValue())); } else if (value.isString()) { - return constructTimestamp(value.stringValue()); + TemporalAccessor parsed = parseTimestampString(value.stringValue(), mapping); + if (parsed == null) { // failed to parse or no formats given + return constructTimestamp(value.stringValue()); + } + try { + return new ExprTimestampValue(Instant.from(parsed)); + } catch (DateTimeException ignored) { + // nothing to do, try another type + } + // TODO return not ExprTimestampValue + try { + return new ExprTimestampValue(new ExprDateValue(LocalDate.from(parsed)).timestampValue()); + } catch (DateTimeException ignored) { + // nothing to do, try another type + } + try { + return new ExprTimestampValue(new ExprDatetimeValue(LocalDateTime.from(parsed)).timestampValue()); + } catch (DateTimeException ignored) { + // nothing to do, try another type + } + try { + return new ExprTimestampValue(new ExprTimeValue(LocalTime.from(parsed)).timestampValue()); + } catch (DateTimeException ignored) { + // nothing to do, try another type + } + // TODO throw exception + LogManager.getLogger(OpenSearchExprValueFactory.class).error( + String.format("Can't recognize parsed value: %s, %s", parsed, parsed.getClass())); + return new ExprStringValue(value.stringValue()); } else { return new ExprTimestampValue((Instant) value.objectValue()); } diff --git a/opensearch/src/main/java/org/opensearch/sql/opensearch/mapping/IndexMapping.java b/opensearch/src/main/java/org/opensearch/sql/opensearch/mapping/IndexMapping.java index 64bfcc9972..f659ed99ec 100644 --- a/opensearch/src/main/java/org/opensearch/sql/opensearch/mapping/IndexMapping.java +++ b/opensearch/src/main/java/org/opensearch/sql/opensearch/mapping/IndexMapping.java @@ -26,11 +26,14 @@ public class IndexMapping { /** Field mappings from field name to field type in OpenSearch date type system. */ private final Map fieldMappings; + public Map mapping2; + public IndexMapping(Map fieldMappings) { this.fieldMappings = fieldMappings; } public IndexMapping(MappingMetadata metaData) { + this.mapping2 = flat2(metaData.getSourceAsMap()); this.fieldMappings = flatMappings(metaData.getSourceAsMap()); } @@ -65,6 +68,17 @@ public Map getAllFieldTypes(Function transform) { .collect(Collectors.toMap(Map.Entry::getKey, e -> transform.apply(e.getValue()))); } + @SuppressWarnings("unchecked") + private Map flat2(Map indexMapping) { + return ((Map)indexMapping.get("properties")).entrySet().stream() + .collect(Collectors.toMap(e -> e.getKey(), e -> { + Map mapping = (Map) e.getValue(); + return new MappingEntry((String) mapping.getOrDefault("type", "object"), + (String) mapping.getOrDefault("format", null), null); + })); + } + + @SuppressWarnings("unchecked") private Map flatMappings(Map indexMapping) { ImmutableMap.Builder builder = new ImmutableMap.Builder<>(); diff --git a/opensearch/src/main/java/org/opensearch/sql/opensearch/mapping/MappingEntry.java b/opensearch/src/main/java/org/opensearch/sql/opensearch/mapping/MappingEntry.java new file mode 100644 index 0000000000..0b9e662bcc --- /dev/null +++ b/opensearch/src/main/java/org/opensearch/sql/opensearch/mapping/MappingEntry.java @@ -0,0 +1,66 @@ +/* + * Copyright OpenSearch Contributors + * SPDX-License-Identifier: Apache-2.0 + */ + + +package org.opensearch.sql.opensearch.mapping; + +import java.time.format.DateTimeFormatter; +import java.util.Arrays; +import java.util.List; +import java.util.Objects; +import java.util.stream.Collectors; +import lombok.AllArgsConstructor; +import lombok.Getter; +import lombok.Setter; +import org.opensearch.common.time.DateFormatter; +import org.opensearch.sql.data.type.ExprType; + +@AllArgsConstructor +public class MappingEntry { + + @Getter + private String fieldType; + + @Getter + private String formats; + + @Getter + @Setter + private ExprType dataType; + + public MappingEntry(String fieldType) { + this(fieldType, null, null); + } + + public List getFormatList() { + if (formats == null || formats.isEmpty()) { + return List.of(); + } + return Arrays.stream(formats.split("\\|\\|")).map(String::trim).collect(Collectors.toList()); + } + + public List getNamedFormatters() { + return getFormatList().stream().filter(f -> { + try { + DateTimeFormatter.ofPattern(f); + return false; + } catch (Exception e) { + return true; + } + }) + .map(DateFormatter::forPattern).collect(Collectors.toList()); + } + + public List getRegularFormatters() { + return getFormatList().stream().map(f -> { + try { + return DateTimeFormatter.ofPattern(f); + } catch (Exception e) { + return null; + } + }) + .filter(Objects::nonNull).collect(Collectors.toList()); + } +} diff --git a/opensearch/src/main/java/org/opensearch/sql/opensearch/request/system/OpenSearchDescribeIndexRequest.java b/opensearch/src/main/java/org/opensearch/sql/opensearch/request/system/OpenSearchDescribeIndexRequest.java index f321497099..56c9436059 100644 --- a/opensearch/src/main/java/org/opensearch/sql/opensearch/request/system/OpenSearchDescribeIndexRequest.java +++ b/opensearch/src/main/java/org/opensearch/sql/opensearch/request/system/OpenSearchDescribeIndexRequest.java @@ -17,6 +17,9 @@ import java.util.List; import java.util.Map; import java.util.stream.Collectors; + +import org.apache.commons.lang3.tuple.Triple; +import org.opensearch.common.collect.Tuple; import org.opensearch.sql.data.model.ExprTupleValue; import org.opensearch.sql.data.model.ExprValue; import org.opensearch.sql.data.type.ExprCoreType; @@ -24,6 +27,7 @@ import org.opensearch.sql.opensearch.client.OpenSearchClient; import org.opensearch.sql.opensearch.data.type.OpenSearchDataType; import org.opensearch.sql.opensearch.mapping.IndexMapping; +import org.opensearch.sql.opensearch.mapping.MappingEntry; import org.opensearch.sql.opensearch.request.OpenSearchRequest; /** @@ -121,6 +125,20 @@ public Map getFieldTypes() { return fieldTypes; } + // TODO possible collision if two indices have fields with same names + public Map getFieldTypes2() { + Map indexMappings = client.getIndexMappings(indexName.getIndexNames()); + Map fieldTypes = new HashMap<>(); + + for (IndexMapping indexMapping : indexMappings.values()) { + indexMapping.mapping2.forEach((key, value) -> + value.setDataType(transformESTypeToExprType(value.getFieldType()))); + fieldTypes + .putAll(indexMapping.mapping2); + } + return fieldTypes; + } + /** * Get the minimum of the max result windows of the indices. * diff --git a/opensearch/src/main/java/org/opensearch/sql/opensearch/storage/OpenSearchIndex.java b/opensearch/src/main/java/org/opensearch/sql/opensearch/storage/OpenSearchIndex.java index 9ebdc12ba2..a1c62ab0bf 100644 --- a/opensearch/src/main/java/org/opensearch/sql/opensearch/storage/OpenSearchIndex.java +++ b/opensearch/src/main/java/org/opensearch/sql/opensearch/storage/OpenSearchIndex.java @@ -104,7 +104,8 @@ public Integer getMaxResultWindow() { @Override public PhysicalPlan implement(LogicalPlan plan) { OpenSearchIndexScan indexScan = new OpenSearchIndexScan(client, settings, indexName, - getMaxResultWindow(), new OpenSearchExprValueFactory(getFieldTypes())); + getMaxResultWindow(), new OpenSearchExprValueFactory(getFieldTypes(), + new OpenSearchDescribeIndexRequest(client, indexName).getFieldTypes2())); /* * Visit logical plan with index scan as context so logical operators visited, such as From 797d2a86718ccd6449ef295df516d3fdcd1e3bec Mon Sep 17 00:00:00 2001 From: Yury-Fridlyand Date: Wed, 16 Nov 2022 20:45:58 -0800 Subject: [PATCH 2/3] Replace old `typeMapping` by new one in `OpenSearchExprValueFactory`, update tests. Signed-off-by: Yury-Fridlyand --- .../value/OpenSearchExprValueFactory.java | 19 ++----- .../sql/opensearch/mapping/IndexMapping.java | 4 +- .../sql/opensearch/mapping/MappingEntry.java | 9 +++ .../OpenSearchDescribeIndexRequest.java | 5 +- .../opensearch/storage/OpenSearchIndex.java | 13 ++++- .../storage/script/core/ExpressionScript.java | 8 +-- .../value/OpenSearchExprValueFactoryTest.java | 55 ++++++++++--------- .../storage/OpenSearchIndexScanTest.java | 4 +- 8 files changed, 64 insertions(+), 53 deletions(-) diff --git a/opensearch/src/main/java/org/opensearch/sql/opensearch/data/value/OpenSearchExprValueFactory.java b/opensearch/src/main/java/org/opensearch/sql/opensearch/data/value/OpenSearchExprValueFactory.java index 76f01a4920..662fede0ed 100644 --- a/opensearch/src/main/java/org/opensearch/sql/opensearch/data/value/OpenSearchExprValueFactory.java +++ b/opensearch/src/main/java/org/opensearch/sql/opensearch/data/value/OpenSearchExprValueFactory.java @@ -77,11 +77,7 @@ public class OpenSearchExprValueFactory { /** * The Mapping of Field and ExprType. */ - @Setter - private Map typeMapping; - - - private Map typeMapping2; + private Map typeMapping; @Getter @Setter @@ -116,15 +112,8 @@ public class OpenSearchExprValueFactory { /** * Constructor of OpenSearchExprValueFactory. */ - public OpenSearchExprValueFactory( - Map typeMapping) { - this.typeMapping = typeMapping; - } - - public OpenSearchExprValueFactory(Map typeMapping, - Map typeMapping2) { + public OpenSearchExprValueFactory(Map typeMapping) { this.typeMapping = typeMapping; - this.typeMapping2 = typeMapping2; } /** @@ -167,7 +156,7 @@ private ExprValue parse(Content content, String field, Optional fieldT return parseArray(content, field); } else { if (typeActionMap.containsKey(type)) { - return typeActionMap.get(type).apply(content, typeMapping2.getOrDefault(field, null)); + return typeActionMap.get(type).apply(content, typeMapping.getOrDefault(field, null)); } else { throw new IllegalStateException( String.format( @@ -181,7 +170,7 @@ private ExprValue parse(Content content, String field, Optional fieldT * but has empty value. For example, {"empty_field": []}. */ private Optional type(String field) { - return Optional.ofNullable(typeMapping.get(field)); + return Optional.ofNullable(typeMapping.get(field).getDataType()); } /** diff --git a/opensearch/src/main/java/org/opensearch/sql/opensearch/mapping/IndexMapping.java b/opensearch/src/main/java/org/opensearch/sql/opensearch/mapping/IndexMapping.java index f659ed99ec..110e6cfa57 100644 --- a/opensearch/src/main/java/org/opensearch/sql/opensearch/mapping/IndexMapping.java +++ b/opensearch/src/main/java/org/opensearch/sql/opensearch/mapping/IndexMapping.java @@ -68,9 +68,11 @@ public Map getAllFieldTypes(Function transform) { .collect(Collectors.toMap(Map.Entry::getKey, e -> transform.apply(e.getValue()))); } + // TODO nested, consider recursive call @SuppressWarnings("unchecked") private Map flat2(Map indexMapping) { - return ((Map)indexMapping.get("properties")).entrySet().stream() + return ((Map)indexMapping.getOrDefault("properties", emptyMap())) + .entrySet().stream() .collect(Collectors.toMap(e -> e.getKey(), e -> { Map mapping = (Map) e.getValue(); return new MappingEntry((String) mapping.getOrDefault("type", "object"), diff --git a/opensearch/src/main/java/org/opensearch/sql/opensearch/mapping/MappingEntry.java b/opensearch/src/main/java/org/opensearch/sql/opensearch/mapping/MappingEntry.java index 0b9e662bcc..e2561e81ee 100644 --- a/opensearch/src/main/java/org/opensearch/sql/opensearch/mapping/MappingEntry.java +++ b/opensearch/src/main/java/org/opensearch/sql/opensearch/mapping/MappingEntry.java @@ -20,12 +20,21 @@ @AllArgsConstructor public class MappingEntry { + /** + * Data type stored in index mapping. + */ @Getter private String fieldType; + /** + * Date formats stored in index mapping. + */ @Getter private String formats; + /** + * ExprType calculated for given `fieldType`. + */ @Getter @Setter private ExprType dataType; diff --git a/opensearch/src/main/java/org/opensearch/sql/opensearch/request/system/OpenSearchDescribeIndexRequest.java b/opensearch/src/main/java/org/opensearch/sql/opensearch/request/system/OpenSearchDescribeIndexRequest.java index 56c9436059..da9ae32e4c 100644 --- a/opensearch/src/main/java/org/opensearch/sql/opensearch/request/system/OpenSearchDescribeIndexRequest.java +++ b/opensearch/src/main/java/org/opensearch/sql/opensearch/request/system/OpenSearchDescribeIndexRequest.java @@ -17,9 +17,6 @@ import java.util.List; import java.util.Map; import java.util.stream.Collectors; - -import org.apache.commons.lang3.tuple.Triple; -import org.opensearch.common.collect.Tuple; import org.opensearch.sql.data.model.ExprTupleValue; import org.opensearch.sql.data.model.ExprValue; import org.opensearch.sql.data.type.ExprCoreType; @@ -126,7 +123,7 @@ public Map getFieldTypes() { } // TODO possible collision if two indices have fields with same names - public Map getFieldTypes2() { + public Map getFieldMappings() { Map indexMappings = client.getIndexMappings(indexName.getIndexNames()); Map fieldTypes = new HashMap<>(); diff --git a/opensearch/src/main/java/org/opensearch/sql/opensearch/storage/OpenSearchIndex.java b/opensearch/src/main/java/org/opensearch/sql/opensearch/storage/OpenSearchIndex.java index a1c62ab0bf..23e92653a2 100644 --- a/opensearch/src/main/java/org/opensearch/sql/opensearch/storage/OpenSearchIndex.java +++ b/opensearch/src/main/java/org/opensearch/sql/opensearch/storage/OpenSearchIndex.java @@ -19,6 +19,7 @@ import org.opensearch.sql.data.type.ExprType; import org.opensearch.sql.opensearch.client.OpenSearchClient; import org.opensearch.sql.opensearch.data.value.OpenSearchExprValueFactory; +import org.opensearch.sql.opensearch.mapping.MappingEntry; import org.opensearch.sql.opensearch.planner.logical.OpenSearchLogicalIndexAgg; import org.opensearch.sql.opensearch.planner.logical.OpenSearchLogicalIndexScan; import org.opensearch.sql.opensearch.planner.logical.OpenSearchLogicalPlanOptimizerFactory; @@ -60,6 +61,8 @@ public class OpenSearchIndex implements Table { */ private Map cachedFieldTypes = null; + private Map cachedFieldMappings = null; + /** * The cached max result window setting of index. */ @@ -87,6 +90,13 @@ public Map getFieldTypes() { return cachedFieldTypes; } + public Map getFieldMappings() { + if (cachedFieldMappings == null) { + cachedFieldMappings = new OpenSearchDescribeIndexRequest(client, indexName).getFieldMappings(); + } + return cachedFieldMappings; + } + /** * Get the max result window setting of the table. */ @@ -104,8 +114,7 @@ public Integer getMaxResultWindow() { @Override public PhysicalPlan implement(LogicalPlan plan) { OpenSearchIndexScan indexScan = new OpenSearchIndexScan(client, settings, indexName, - getMaxResultWindow(), new OpenSearchExprValueFactory(getFieldTypes(), - new OpenSearchDescribeIndexRequest(client, indexName).getFieldTypes2())); + getMaxResultWindow(), new OpenSearchExprValueFactory(getFieldMappings())); /* * Visit logical plan with index scan as context so logical operators visited, such as diff --git a/opensearch/src/main/java/org/opensearch/sql/opensearch/storage/script/core/ExpressionScript.java b/opensearch/src/main/java/org/opensearch/sql/opensearch/storage/script/core/ExpressionScript.java index 9399c38e46..e8d9f89969 100644 --- a/opensearch/src/main/java/org/opensearch/sql/opensearch/storage/script/core/ExpressionScript.java +++ b/opensearch/src/main/java/org/opensearch/sql/opensearch/storage/script/core/ExpressionScript.java @@ -29,6 +29,7 @@ import org.opensearch.sql.expression.env.Environment; import org.opensearch.sql.expression.parse.ParseExpression; import org.opensearch.sql.opensearch.data.value.OpenSearchExprValueFactory; +import org.opensearch.sql.opensearch.mapping.MappingEntry; import org.opensearch.sql.opensearch.storage.script.ScriptUtils; /** @@ -105,10 +106,9 @@ public Object visitParse(ParseExpression node, Set context) } private OpenSearchExprValueFactory buildValueFactory(Set fields) { - Map typeEnv = fields.stream() - .collect(toMap( - ReferenceExpression::getAttr, - ReferenceExpression::type)); + Map typeEnv = fields.stream().collect(toMap( + e -> e.getAttr(), + e -> new MappingEntry(null, null, e.type()))); return new OpenSearchExprValueFactory(typeEnv); } diff --git a/opensearch/src/test/java/org/opensearch/sql/opensearch/data/value/OpenSearchExprValueFactoryTest.java b/opensearch/src/test/java/org/opensearch/sql/opensearch/data/value/OpenSearchExprValueFactoryTest.java index 8d5552d6a8..57f69c009a 100644 --- a/opensearch/src/test/java/org/opensearch/sql/opensearch/data/value/OpenSearchExprValueFactoryTest.java +++ b/opensearch/src/test/java/org/opensearch/sql/opensearch/data/value/OpenSearchExprValueFactoryTest.java @@ -56,35 +56,37 @@ import org.opensearch.sql.data.model.ExprValue; import org.opensearch.sql.data.type.ExprType; import org.opensearch.sql.opensearch.data.utils.OpenSearchJsonContent; +import org.opensearch.sql.opensearch.mapping.MappingEntry; class OpenSearchExprValueFactoryTest { - private static final Map MAPPING = - new ImmutableMap.Builder() - .put("byteV", BYTE) - .put("shortV", SHORT) - .put("intV", INTEGER) - .put("longV", LONG) - .put("floatV", FLOAT) - .put("doubleV", DOUBLE) - .put("stringV", STRING) - .put("dateV", DATE) - .put("datetimeV", DATETIME) - .put("timeV", TIME) - .put("timestampV", TIMESTAMP) - .put("boolV", BOOLEAN) - .put("structV", STRUCT) - .put("structV.id", INTEGER) - .put("structV.state", STRING) - .put("arrayV", ARRAY) - .put("arrayV.info", STRING) - .put("arrayV.author", STRING) - .put("textV", OPENSEARCH_TEXT) - .put("textKeywordV", OPENSEARCH_TEXT_KEYWORD) - .put("ipV", OPENSEARCH_IP) - .put("geoV", OPENSEARCH_GEO_POINT) - .put("binaryV", OPENSEARCH_BINARY) + private static final Map MAPPING = + new ImmutableMap.Builder() + .put("byteV", new MappingEntry(null, null, BYTE)) + .put("shortV", new MappingEntry(null, null, SHORT)) + .put("intV", new MappingEntry(null, null, INTEGER)) + .put("longV", new MappingEntry(null, null, LONG)) + .put("floatV", new MappingEntry(null, null, FLOAT)) + .put("doubleV", new MappingEntry(null, null, DOUBLE)) + .put("stringV", new MappingEntry(null, null, STRING)) + .put("dateV", new MappingEntry(null, null, DATE)) + .put("datetimeV", new MappingEntry(null, null, DATETIME)) + .put("timeV", new MappingEntry(null, null, TIME)) + .put("timestampV", new MappingEntry(null, null, TIMESTAMP)) + .put("boolV", new MappingEntry(null, null, BOOLEAN)) + .put("structV", new MappingEntry(null, null, STRUCT)) + .put("structV.id", new MappingEntry(null, null, INTEGER)) + .put("structV.state", new MappingEntry(null, null, STRING)) + .put("arrayV", new MappingEntry(null, null, ARRAY)) + .put("arrayV.info", new MappingEntry(null, null, STRING)) + .put("arrayV.author", new MappingEntry(null, null, STRING)) + .put("textV", new MappingEntry(null, null, OPENSEARCH_TEXT)) + .put("textKeywordV", new MappingEntry(null, null, OPENSEARCH_TEXT_KEYWORD)) + .put("ipV", new MappingEntry(null, null, OPENSEARCH_IP)) + .put("geoV", new MappingEntry(null, null, OPENSEARCH_GEO_POINT)) + .put("binaryV", new MappingEntry(null, null, OPENSEARCH_BINARY)) .build(); + private OpenSearchExprValueFactory exprValueFactory = new OpenSearchExprValueFactory(MAPPING); @@ -364,7 +366,8 @@ public void noTypeFoundForMapping() { @Test public void constructUnsupportedTypeThrowException() { OpenSearchExprValueFactory exprValueFactory = - new OpenSearchExprValueFactory(ImmutableMap.of("type", new TestType())); + new OpenSearchExprValueFactory(ImmutableMap.of("type", + new MappingEntry(null, null, new TestType()))); IllegalStateException exception = assertThrows(IllegalStateException.class, () -> exprValueFactory.construct("{\"type\":1}")); assertEquals("Unsupported type: TEST_TYPE for value: 1.", exception.getMessage()); diff --git a/opensearch/src/test/java/org/opensearch/sql/opensearch/storage/OpenSearchIndexScanTest.java b/opensearch/src/test/java/org/opensearch/sql/opensearch/storage/OpenSearchIndexScanTest.java index 9a606750a3..cb7e609b63 100644 --- a/opensearch/src/test/java/org/opensearch/sql/opensearch/storage/OpenSearchIndexScanTest.java +++ b/opensearch/src/test/java/org/opensearch/sql/opensearch/storage/OpenSearchIndexScanTest.java @@ -41,6 +41,7 @@ import org.opensearch.sql.exception.SemanticCheckException; import org.opensearch.sql.opensearch.client.OpenSearchClient; import org.opensearch.sql.opensearch.data.value.OpenSearchExprValueFactory; +import org.opensearch.sql.opensearch.mapping.MappingEntry; import org.opensearch.sql.opensearch.request.OpenSearchQueryRequest; import org.opensearch.sql.opensearch.request.OpenSearchRequest; import org.opensearch.sql.opensearch.response.OpenSearchResponse; @@ -55,7 +56,8 @@ class OpenSearchIndexScanTest { private Settings settings; private OpenSearchExprValueFactory exprValueFactory = new OpenSearchExprValueFactory( - ImmutableMap.of("name", STRING, "department", STRING)); + Map.of("name", new MappingEntry(null, null, STRING), + "department", new MappingEntry(null, null, STRING))); @BeforeEach void setup() { From 4a76716cec5b321a6b487fa159dd3694ac91a12e Mon Sep 17 00:00:00 2001 From: Yury-Fridlyand Date: Wed, 16 Nov 2022 21:31:00 -0800 Subject: [PATCH 3/3] Minor fix. Signed-off-by: Yury-Fridlyand --- .../data/value/OpenSearchExprValueFactory.java | 1 + .../opensearch/request/OpenSearchRequestBuilder.java | 3 ++- .../script/aggregation/AggregationQueryBuilder.java | 11 ++++++----- 3 files changed, 9 insertions(+), 6 deletions(-) diff --git a/opensearch/src/main/java/org/opensearch/sql/opensearch/data/value/OpenSearchExprValueFactory.java b/opensearch/src/main/java/org/opensearch/sql/opensearch/data/value/OpenSearchExprValueFactory.java index 662fede0ed..3deaad64de 100644 --- a/opensearch/src/main/java/org/opensearch/sql/opensearch/data/value/OpenSearchExprValueFactory.java +++ b/opensearch/src/main/java/org/opensearch/sql/opensearch/data/value/OpenSearchExprValueFactory.java @@ -77,6 +77,7 @@ public class OpenSearchExprValueFactory { /** * The Mapping of Field and ExprType. */ + @Setter private Map typeMapping; @Getter diff --git a/opensearch/src/main/java/org/opensearch/sql/opensearch/request/OpenSearchRequestBuilder.java b/opensearch/src/main/java/org/opensearch/sql/opensearch/request/OpenSearchRequestBuilder.java index c26413c622..201ec97176 100644 --- a/opensearch/src/main/java/org/opensearch/sql/opensearch/request/OpenSearchRequestBuilder.java +++ b/opensearch/src/main/java/org/opensearch/sql/opensearch/request/OpenSearchRequestBuilder.java @@ -32,6 +32,7 @@ import org.opensearch.sql.exception.SemanticCheckException; import org.opensearch.sql.expression.ReferenceExpression; import org.opensearch.sql.opensearch.data.value.OpenSearchExprValueFactory; +import org.opensearch.sql.opensearch.mapping.MappingEntry; import org.opensearch.sql.opensearch.response.agg.OpenSearchAggregationResponseParser; /** @@ -213,7 +214,7 @@ public void pushDownProjects(Set projects) { sourceBuilder.fetchSource(projectsSet.toArray(new String[0]), new String[0]); } - public void pushTypeMapping(Map typeMapping) { + public void pushTypeMapping(Map typeMapping) { exprValueFactory.setTypeMapping(typeMapping); } diff --git a/opensearch/src/main/java/org/opensearch/sql/opensearch/storage/script/aggregation/AggregationQueryBuilder.java b/opensearch/src/main/java/org/opensearch/sql/opensearch/storage/script/aggregation/AggregationQueryBuilder.java index ae3239eea0..030e841089 100644 --- a/opensearch/src/main/java/org/opensearch/sql/opensearch/storage/script/aggregation/AggregationQueryBuilder.java +++ b/opensearch/src/main/java/org/opensearch/sql/opensearch/storage/script/aggregation/AggregationQueryBuilder.java @@ -30,6 +30,7 @@ import org.opensearch.sql.expression.NamedExpression; import org.opensearch.sql.expression.ReferenceExpression; import org.opensearch.sql.expression.aggregation.NamedAggregator; +import org.opensearch.sql.opensearch.mapping.MappingEntry; import org.opensearch.sql.opensearch.response.agg.CompositeAggregationParser; import org.opensearch.sql.opensearch.response.agg.MetricParser; import org.opensearch.sql.opensearch.response.agg.NoBucketAggregationParser; @@ -104,14 +105,14 @@ public AggregationQueryBuilder( } /** - * Build ElasticsearchExprValueFactory. + * Build mapping for OpenSearchExprValueFactory. */ - public Map buildTypeMapping( + public Map buildTypeMapping( List namedAggregatorList, List groupByList) { - ImmutableMap.Builder builder = new ImmutableMap.Builder<>(); - namedAggregatorList.forEach(agg -> builder.put(agg.getName(), agg.type())); - groupByList.forEach(group -> builder.put(group.getNameOrAlias(), group.type())); + ImmutableMap.Builder builder = new ImmutableMap.Builder<>(); + namedAggregatorList.forEach(agg -> builder.put(agg.getName(), new MappingEntry(null, null, agg.type()))); + groupByList.forEach(group -> builder.put(group.getNameOrAlias(), new MappingEntry(null, null, group.type()))); return builder.build(); }