Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Rework on OpenSearchDataType: parse, store and use mapping information #180

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
29 commits
Select commit Hold shift + click to select a range
f335101
First PoC.
Yury-Fridlyand Nov 16, 2022
797d2a8
Replace old `typeMapping` by new one in `OpenSearchExprValueFactory`,…
Yury-Fridlyand Nov 17, 2022
4a76716
Minor fix.
Yury-Fridlyand Nov 17, 2022
e4d89de
Rework on `OpenSearchDataType`.
Yury-Fridlyand Dec 1, 2022
16d7ce0
Further rework on `OpenSearchDataType`:
Yury-Fridlyand Dec 20, 2022
ecf60de
Fix failing unit tests.
Yury-Fridlyand Jan 4, 2023
f699c15
Test fixes and coverage.
Yury-Fridlyand Jan 5, 2023
c538ac7
Fix code style.
Yury-Fridlyand Jan 6, 2023
4a6fb9f
Minor fixes.
Yury-Fridlyand Jan 6, 2023
de08526
Fix for code coverage in `OpenSearchDataType.of(Type)`.
Yury-Fridlyand Jan 6, 2023
53c4fdd
Fix reordering columns.
Yury-Fridlyand Jan 6, 2023
2b9f253
Fix doctests.
Yury-Fridlyand Jan 6, 2023
43a0a98
Typo fix.
Yury-Fridlyand Jan 6, 2023
1f7b151
Rework on 53c4fdd. New fix has code coverage and all test passed on it.
Yury-Fridlyand Jan 6, 2023
90bae1f
Typo fix for previous commit.
Yury-Fridlyand Jan 6, 2023
735e9f7
Revert wrong test changes made in 1f7b1516.
Yury-Fridlyand Jan 6, 2023
46f20ee
Remove `TextKeywordValue`.
Yury-Fridlyand Jan 11, 2023
ae11c22
Update comments.
Yury-Fridlyand Jan 11, 2023
84cbc67
Add changes according to the PR review.
Yury-Fridlyand Jan 14, 2023
902929e
Changes according to the PR feedback.
Yury-Fridlyand Jan 27, 2023
733a262
A fix to avoid breaking changes.
Yury-Fridlyand Jan 27, 2023
d2f4ed5
Move `convertTextToKeyword` from `ScriptUtils` to `OpenSearchTextType…
Yury-Fridlyand Jan 27, 2023
8fa98df
Update UT for `typeof` function.
Yury-Fridlyand Jan 27, 2023
6e96dae
Minor commenting.
Yury-Fridlyand Jan 27, 2023
e32798c
Typo fix for `d2f4ed5e`.
Yury-Fridlyand Jan 28, 2023
a41aa28
Make all instances of `OpenSearchDataType` and of derived types singl…
Yury-Fridlyand Jan 28, 2023
b1ffaa3
Address PR feedback.
Yury-Fridlyand Jan 31, 2023
7f614ac
Fix doc and integration tests.
Yury-Fridlyand Jan 31, 2023
4c7e293
Fix a unit test.
Yury-Fridlyand Jan 31, 2023
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@ public int compareTo(ExprValue other) {
throw new IllegalStateException(
String.format("[BUG] Unreachable, Comparing with NULL or MISSING is undefined"));
}
if ((this.isNumber() && other.isNumber()) || this.type() == other.type()) {
if ((this.isNumber() && other.isNumber()) || 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 @@ -81,9 +81,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 @@ -41,7 +41,7 @@ public Pair<FunctionSignature, FunctionBuilder> resolve(
arguments -> 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 @@ -47,7 +47,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 @@ -58,8 +58,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 @@ -3125,5 +3125,5 @@ 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)
acarbonetto marked this conversation as resolved.
Show resolved Hide resolved
public class OpenSearchBinaryType extends OpenSearchDataType {
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What is the difference of OpenSearchBinaryType and OpenSearchGeoPointType? only class name is different?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The mapping type is different (line 21 in both files)


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

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

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