Skip to content

Commit

Permalink
Rework on OpenSearchDataType: parse, store and use mapping informat…
Browse files Browse the repository at this point in the history
…ion (#180)

Rework on `OpenSearchDataType`:
* Add data types for those classes are not defined in `ExprCoreType`.
* Address #180 (comment)
* Remove `TextKeywordValue`.
* Add changes according to the PR review. #180
* Update `IndexMapping::parseMapping` function.
* Add `OpenSearchDataType::resolve` function.
* Add new constructor for `OpenSearchTextType`.
* Make `fields` and `properties` in `OpenSearchDataType` readonly. Update tests and mapping parser.
* Move `getFields` from `OpenSearchDataType` to `OpenSearchTextType`. Update tests.
* Rewrite `traverseAndFlatten` according to #180 (comment)
* Minor comment fix.
* A fix to avoid breaking changes.
* `typeName` and `legacyTypeName` to return different type names.
* Change `typeof` function and corresponding tests.
* Move `convertTextToKeyword` from `ScriptUtils` to `OpenSearchTextType`. Update tests.
* Update UT for `typeof` function.
* Make all instances of `OpenSearchDataType` and of derived types singletones as much as possible.
* Make string representations of all `ExprType`s uppercase.
* Remove functions from `IndexMapping` used in tests only.

Signed-off-by: Yury-Fridlyand <yuryf@bitquilltech.com>
Signed-off-by: Yury-Fridlyand <yury.fridlyand@improving.com>
  • Loading branch information
Yury-Fridlyand committed Feb 1, 2023
1 parent cfe6802 commit c9d106a
Show file tree
Hide file tree
Showing 60 changed files with 1,412 additions and 717 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@ public int compareTo(ExprValue other) {
}
if ((this.isNumber() && other.isNumber())
|| (this.isDateTime() && other.isDateTime())
|| this.type() == other.type()) {
|| this.type().equals(other.type())) {
return compare(other);
} else {
throw new ExpressionEvaluationException(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -80,9 +80,9 @@ public enum ExprCoreType implements ExprType {
*/
private static final Map<ExprCoreType, String> LEGACY_TYPE_NAME_MAPPING =
new ImmutableMap.Builder<ExprCoreType, String>()
.put(STRUCT, "object")
.put(ARRAY, "nested")
.put(STRING, "keyword")
.put(STRUCT, "OBJECT")
.put(ARRAY, "NESTED")
.put(STRING, "KEYWORD")
.build();

private static final Set<ExprType> NUMBER_TYPES =
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,7 @@ public Pair<FunctionSignature, FunctionBuilder> resolve(
new FunctionExpression(BuiltinFunctionName.TYPEOF.getName(), arguments) {
@Override
public ExprValue valueOf(Environment<Expression, ExprValue> valueEnv) {
return new ExprStringValue(getArguments().get(0).type().toString());
return new ExprStringValue(getArguments().get(0).type().legacyTypeName());
}

@Override
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -76,9 +76,9 @@ public void getParent() {

@Test
void legacyName() {
assertEquals("keyword", STRING.legacyTypeName());
assertEquals("nested", ARRAY.legacyTypeName());
assertEquals("object", STRUCT.legacyTypeName());
assertEquals("KEYWORD", STRING.legacyTypeName());
assertEquals("NESTED", ARRAY.legacyTypeName());
assertEquals("OBJECT", STRUCT.legacyTypeName());
assertEquals("integer", INTEGER.legacyTypeName().toLowerCase());
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,7 @@ public class SystemFunctionsTest {
void typeof() {
assertEquals(STRING, DSL.typeof(DSL.literal(1)).type());

assertEquals("ARRAY", typeofGetValue(new ExprCollectionValue(List.of())));
assertEquals("NESTED", typeofGetValue(new ExprCollectionValue(List.of())));
assertEquals("BOOLEAN", typeofGetValue(ExprBooleanValue.of(false)));
assertEquals("BYTE", typeofGetValue(new ExprByteValue(0)));
assertEquals("DATE", typeofGetValue(new ExprDateValue(LocalDate.now())));
Expand All @@ -56,8 +56,8 @@ void typeof() {
assertEquals("INTERVAL", typeofGetValue(new ExprIntervalValue(Duration.ofDays(0))));
assertEquals("LONG", typeofGetValue(new ExprLongValue(0)));
assertEquals("SHORT", typeofGetValue(new ExprShortValue(0)));
assertEquals("STRING", typeofGetValue(new ExprStringValue("")));
assertEquals("STRUCT", typeofGetValue(new ExprTupleValue(new LinkedHashMap<>())));
assertEquals("KEYWORD", typeofGetValue(new ExprStringValue("")));
assertEquals("OBJECT", typeofGetValue(new ExprTupleValue(new LinkedHashMap<>())));
assertEquals("TIME", typeofGetValue(new ExprTimeValue(LocalTime.now())));
assertEquals("TIMESTAMP", typeofGetValue(new ExprTimestampValue(Instant.now())));
assertEquals("UNDEFINED", typeofGetValue(ExprNullValue.of()));
Expand Down
2 changes: 1 addition & 1 deletion docs/user/dql/functions.rst
Original file line number Diff line number Diff line change
Expand Up @@ -3805,7 +3805,7 @@ Example::
+----------------+---------------+-----------------+------------------+
| typeof(date) | typeof(int) | typeof(now()) | typeof(column) |
|----------------+---------------+-----------------+------------------|
| DATE | INTEGER | DATETIME | STRUCT |
| DATE | INTEGER | DATETIME | OBJECT |
+----------------+---------------+-----------------+------------------+


2 changes: 1 addition & 1 deletion docs/user/ppl/functions/system.rst
Original file line number Diff line number Diff line change
Expand Up @@ -27,5 +27,5 @@ Example::
+----------------+---------------+-----------------+------------------+
| typeof(date) | typeof(int) | typeof(now()) | typeof(column) |
|----------------+---------------+-----------------+------------------|
| DATE | INTEGER | DATETIME | STRUCT |
| DATE | INTEGER | DATETIME | OBJECT |
+----------------+---------------+-----------------+------------------+
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,7 @@ public void typeof_sql_types() throws IOException {
TEST_INDEX_DATATYPE_NUMERIC));
// TODO: test null in PPL
verifyDataRows(response,
rows("STRING", "DOUBLE", "INTEGER", "LONG", "INTERVAL"));
rows("KEYWORD", "DOUBLE", "INTEGER", "LONG", "INTERVAL"));

response = executeQuery(String.format("source=%s | eval "
+ "`timestamp` = typeof(CAST('1961-04-12 09:07:00' AS TIMESTAMP)),"
Expand Down Expand Up @@ -68,7 +68,7 @@ public void typeof_opensearch_types() throws IOException {
+ " | fields `text`, `date`, `boolean`, `object`, `keyword`, `ip`, `binary`, `geo_point`",
TEST_INDEX_DATATYPE_NONNUMERIC));
verifyDataRows(response,
rows("OPENSEARCH_TEXT", "TIMESTAMP", "BOOLEAN", "STRUCT", "STRING",
"OPENSEARCH_IP", "OPENSEARCH_BINARY", "OPENSEARCH_GEO_POINT"));
rows("TEXT", "TIMESTAMP", "BOOLEAN", "OBJECT", "KEYWORD",
"IP", "BINARY", "GEO_POINT"));
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@ public void typeof_sql_types() {
JSONObject response = executeJdbcRequest("SELECT typeof('pewpew'), typeof(NULL), typeof(1.0),"
+ "typeof(12345), typeof(1234567891011), typeof(INTERVAL 2 DAY);");
verifyDataRows(response,
rows("STRING", "UNDEFINED", "DOUBLE", "INTEGER", "LONG", "INTERVAL"));
rows("KEYWORD", "UNDEFINED", "DOUBLE", "INTEGER", "LONG", "INTERVAL"));

response = executeJdbcRequest("SELECT"
+ " typeof(CAST('1961-04-12 09:07:00' AS TIMESTAMP)),"
Expand All @@ -54,7 +54,7 @@ public void typeof_opensearch_types() {
//+ ", typeof(nested_value)"
+ " from %s;", TEST_INDEX_DATATYPE_NONNUMERIC));
verifyDataRows(response,
rows("OPENSEARCH_TEXT", "TIMESTAMP", "BOOLEAN", "STRUCT", "STRING",
"OPENSEARCH_IP", "OPENSEARCH_BINARY", "OPENSEARCH_GEO_POINT"));
rows("TEXT", "TIMESTAMP", "BOOLEAN", "OBJECT", "KEYWORD",
"IP", "BINARY", "GEO_POINT"));
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,33 @@
/*
* Copyright OpenSearch Contributors
* SPDX-License-Identifier: Apache-2.0
*/


package org.opensearch.sql.opensearch.data.type;

import static org.opensearch.sql.data.type.ExprCoreType.UNKNOWN;

import lombok.EqualsAndHashCode;
import lombok.Getter;

/**
* The type of a binary value. See
* <a href="https://opensearch.org/docs/latest/opensearch/supported-field-types/binary/">doc</a>
*/
@EqualsAndHashCode(callSuper = false)
public class OpenSearchBinaryType extends OpenSearchDataType {

@Getter
private static final OpenSearchBinaryType instance = new OpenSearchBinaryType();

private OpenSearchBinaryType() {
super(MappingType.Binary);
exprCoreType = UNKNOWN;
}

@Override
protected OpenSearchDataType cloneEmpty() {
return instance;
}
}
Loading

0 comments on commit c9d106a

Please sign in to comment.