From c05da53e8252191e441c82d33ea6bcbb5c0d8324 Mon Sep 17 00:00:00 2001 From: David Cui Date: Tue, 15 Oct 2019 13:43:58 -0700 Subject: [PATCH 01/15] updating release notes --- opendistro-elasticsearch-sql.release-notes | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/opendistro-elasticsearch-sql.release-notes b/opendistro-elasticsearch-sql.release-notes index 9b9a45b545..ae597f6a49 100644 --- a/opendistro-elasticsearch-sql.release-notes +++ b/opendistro-elasticsearch-sql.release-notes @@ -1,3 +1,9 @@ +## 2019-10-15, Version 1.2.1 (Current) + +### Notable changes +* Feature [#202](https://github.com/opendistro-for-elasticsearch/sql/issues/202): Elasticsearch 7.2.1 compatibility + + ## 2019-07-23, Version 1.2.0 (Current) ### Notable changes From 32065a6d35932a24f2a0c8f3ffcf80e5136f1ad1 Mon Sep 17 00:00:00 2001 From: David Cui Date: Wed, 23 Oct 2019 13:30:52 -0700 Subject: [PATCH 02/15] pushing grammar changes for CAST --- src/main/antlr/OpenDistroSqlLexer.g4 | 1 + src/main/antlr/OpenDistroSqlParser.g4 | 37 ++++++++++++++++++++++++++- 2 files changed, 37 insertions(+), 1 deletion(-) diff --git a/src/main/antlr/OpenDistroSqlLexer.g4 b/src/main/antlr/OpenDistroSqlLexer.g4 index 3486b4af22..e41d359a48 100644 --- a/src/main/antlr/OpenDistroSqlLexer.g4 +++ b/src/main/antlr/OpenDistroSqlLexer.g4 @@ -49,6 +49,7 @@ ASC: 'ASC'; BETWEEN: 'BETWEEN'; BY: 'BY'; CASE: 'CASE'; +CAST: 'CAST'; CROSS: 'CROSS'; DELETE: 'DELETE'; DESC: 'DESC'; diff --git a/src/main/antlr/OpenDistroSqlParser.g4 b/src/main/antlr/OpenDistroSqlParser.g4 index 3676166164..cfd0a68e4f 100644 --- a/src/main/antlr/OpenDistroSqlParser.g4 +++ b/src/main/antlr/OpenDistroSqlParser.g4 @@ -231,6 +231,7 @@ uid simpleId : ID | DOT_ID // note: the current scope by adding DOT_ID to simpleId is large, move DOT_ID upwards tablename if needed + | charsetNameBase | STRING_LITERAL | keywordsCanBeId | functionNameBase @@ -241,6 +242,12 @@ dottedId | '.' uid ; +charsetName + : BINARY + | charsetNameBase + | STRING_LITERAL + | CHARSET_REVERSE_QUOTE_STRING + ; // Literals @@ -297,7 +304,10 @@ functionCall ; specificFunction - : CASE expression caseFuncAlternative+ + : CONVERT '(' expression separator=',' convertedDataType ')' #dataTypeFunctionCall + | CONVERT '(' expression USING charsetName ')' #dataTypeFunctionCall + | CAST '(' expression AS convertedDataType ')' #dataTypeFunctionCall + | CASE expression caseFuncAlternative+ (ELSE elseArg=functionArg)? END #caseFunctionCall | CASE caseFuncAlternative+ (ELSE elseArg=functionArg)? END #caseFunctionCall @@ -308,6 +318,22 @@ caseFuncAlternative THEN consequent=functionArg ; +convertedDataType + : typeName=(BINARY| NCHAR) lengthOneDimension? + | typeName=CHAR lengthOneDimension? ((CHARACTER SET | CHARSET) charsetName)? + | typeName=(DATE | DATETIME | TIME) + | typeName=DECIMAL lengthTwoDimension? + | (SIGNED | UNSIGNED) INTEGER? + ; + +lengthOneDimension + : '(' decimalLiteral ')' + ; + +lengthTwoDimension + : '(' decimalLiteral ',' decimalLiteral ')' + ; + aggregateWindowedFunction : (AVG | MAX | MIN | SUM) '(' aggregator=(ALL | DISTINCT)? functionArg ')' @@ -391,6 +417,15 @@ mathOperator // Simple id sets // (that keyword, which can be id) +charsetNameBase + : ARMSCII8 | ASCII | BIG5 | CP1250 | CP1251 | CP1256 | CP1257 + | CP850 | CP852 | CP866 | CP932 | DEC8 | EUCJPMS | EUCKR + | GB2312 | GBK | GEOSTD8 | GREEK | HEBREW | HP8 | KEYBCS2 + | KOI8R | KOI8U | LATIN1 | LATIN2 | LATIN5 | LATIN7 | MACCE + | MACROMAN | SJIS | SWE7 | TIS620 | UCS2 | UJIS | UTF16 + | UTF16LE | UTF32 | UTF8 | UTF8MB3 | UTF8MB4 + ; + keywordsCanBeId : FULL | FIELD | D | T | TS // OD SQL and ODBC special From ea1135d5726f67f13141d96b6b329a366334ebdb Mon Sep 17 00:00:00 2001 From: David Cui Date: Thu, 24 Oct 2019 12:58:11 -0700 Subject: [PATCH 03/15] added supported fields for cast --- src/main/antlr/OpenDistroSqlLexer.g4 | 6 ++++++ src/main/antlr/OpenDistroSqlParser.g4 | 9 ++++++--- 2 files changed, 12 insertions(+), 3 deletions(-) diff --git a/src/main/antlr/OpenDistroSqlLexer.g4 b/src/main/antlr/OpenDistroSqlLexer.g4 index e41d359a48..08ab8b9720 100644 --- a/src/main/antlr/OpenDistroSqlLexer.g4 +++ b/src/main/antlr/OpenDistroSqlLexer.g4 @@ -51,23 +51,28 @@ BY: 'BY'; CASE: 'CASE'; CAST: 'CAST'; CROSS: 'CROSS'; +DATETIME: 'DATETIME'; DELETE: 'DELETE'; DESC: 'DESC'; DESCRIBE: 'DESCRIBE'; DISTINCT: 'DISTINCT'; +DOUBLE: 'DOUBLE'; ELSE: 'ELSE'; EXISTS: 'EXISTS'; FALSE: 'FALSE'; +FLOAT: 'FLOAT'; FROM: 'FROM'; GROUP: 'GROUP'; HAVING: 'HAVING'; IN: 'IN'; INNER: 'INNER'; +INT: 'INT'; IS: 'IS'; JOIN: 'JOIN'; LEFT: 'LEFT'; LIKE: 'LIKE'; LIMIT: 'LIMIT'; +LONG: 'LONG'; MATCH: 'MATCH'; NATURAL: 'NATURAL'; NOT: 'NOT'; @@ -80,6 +85,7 @@ REGEXP: 'REGEXP'; RIGHT: 'RIGHT'; SELECT: 'SELECT'; SHOW: 'SHOW'; +STRING: 'STRING'; THEN: 'THEN'; TRUE: 'TRUE'; UNION: 'UNION'; diff --git a/src/main/antlr/OpenDistroSqlParser.g4 b/src/main/antlr/OpenDistroSqlParser.g4 index cfd0a68e4f..f24247e40b 100644 --- a/src/main/antlr/OpenDistroSqlParser.g4 +++ b/src/main/antlr/OpenDistroSqlParser.g4 @@ -321,9 +321,12 @@ caseFuncAlternative convertedDataType : typeName=(BINARY| NCHAR) lengthOneDimension? | typeName=CHAR lengthOneDimension? ((CHARACTER SET | CHARSET) charsetName)? - | typeName=(DATE | DATETIME | TIME) - | typeName=DECIMAL lengthTwoDimension? - | (SIGNED | UNSIGNED) INTEGER? + | typeName=DATETIME + | typeName=INT + | typeName=DOUBLE + | typeName=LONG + | typeName=FLOAT + | typeName=STRING ; lengthOneDimension From d85c37b7e29eed8584d0ac4d8cf65c8f9abaa248 Mon Sep 17 00:00:00 2001 From: David Cui Date: Thu, 24 Oct 2019 16:16:37 -0700 Subject: [PATCH 04/15] adding fix for cast() result set --- .../sql/query/DefaultQueryAction.java | 6 +++++- .../sql/esintgtest/SQLFunctionsIT.java | 9 +++++++++ 2 files changed, 14 insertions(+), 1 deletion(-) diff --git a/src/main/java/com/amazon/opendistroforelasticsearch/sql/query/DefaultQueryAction.java b/src/main/java/com/amazon/opendistroforelasticsearch/sql/query/DefaultQueryAction.java index 1a2e666916..f662f4a8c0 100644 --- a/src/main/java/com/amazon/opendistroforelasticsearch/sql/query/DefaultQueryAction.java +++ b/src/main/java/com/amazon/opendistroforelasticsearch/sql/query/DefaultQueryAction.java @@ -18,6 +18,7 @@ import com.alibaba.druid.sql.ast.SQLExpr; import com.alibaba.druid.sql.ast.expr.SQLBinaryOpExpr; import com.alibaba.druid.sql.ast.expr.SQLBinaryOperator; +import com.alibaba.druid.sql.ast.expr.SQLCastExpr; import com.amazon.opendistroforelasticsearch.sql.domain.Field; import com.amazon.opendistroforelasticsearch.sql.domain.KVValue; import com.amazon.opendistroforelasticsearch.sql.domain.MethodField; @@ -95,7 +96,7 @@ public SqlElasticSearchRequestBuilder explain() throws SqlParseException { setIndicesAndTypes(); setFields(select.getFields()); - + // if cast, add casting field to include list, should do the trick bb setWhere(select.getWhere()); setSorts(select.getOrderBys()); setLimit(select.getOffset(), select.getRowCount()); @@ -152,6 +153,9 @@ public void setFields(List fields) throws SqlParseException { MethodField method = (MethodField) field; if (method.getName().toLowerCase().equals("script")) { handleScriptField(method); + if (method.getExpression() instanceof SQLCastExpr) { + includeFields.add(method.getParams().get(0).toString()); + } } else if (method.getName().equalsIgnoreCase("include")) { for (KVValue kvValue : method.getParams()) { includeFields.add(kvValue.value.toString()); diff --git a/src/test/java/com/amazon/opendistroforelasticsearch/sql/esintgtest/SQLFunctionsIT.java b/src/test/java/com/amazon/opendistroforelasticsearch/sql/esintgtest/SQLFunctionsIT.java index cabd01dd3f..6c04ca8ced 100644 --- a/src/test/java/com/amazon/opendistroforelasticsearch/sql/esintgtest/SQLFunctionsIT.java +++ b/src/test/java/com/amazon/opendistroforelasticsearch/sql/esintgtest/SQLFunctionsIT.java @@ -159,6 +159,15 @@ public void caseChangeWithAggregationTest() throws IOException { hitAny("/aggregations/UPPER_1/buckets", kvString("/key", equalTo("AMBER")))); } + @Test + public void castFieldWithoutAliasTest() throws IOException { + String query = "SELECT CAST(age AS DOUBLE) FROM " + TestsConstants.TEST_INDEX_ACCOUNT + " /account limit 5"; + + JSONObject response = executeQuery(query); + JSONArray hits = getHits(response); + System.out.println(hits.toString()); + } + @Test public void concat_ws_field_and_string() throws Exception { //here is a bug,csv field with spa From da88759ba5311f1533cf25d25ecd5da8879cba13 Mon Sep 17 00:00:00 2001 From: David Cui Date: Fri, 25 Oct 2019 10:26:19 -0700 Subject: [PATCH 05/15] added IT tests and removed unnecessary parts of grammar file --- src/main/antlr/OpenDistroSqlParser.g4 | 12 +----- .../sql/esintgtest/SQLFunctionsIT.java | 40 ++++++++++++++++--- 2 files changed, 36 insertions(+), 16 deletions(-) diff --git a/src/main/antlr/OpenDistroSqlParser.g4 b/src/main/antlr/OpenDistroSqlParser.g4 index f24247e40b..6c6f4169e3 100644 --- a/src/main/antlr/OpenDistroSqlParser.g4 +++ b/src/main/antlr/OpenDistroSqlParser.g4 @@ -319,9 +319,7 @@ caseFuncAlternative ; convertedDataType - : typeName=(BINARY| NCHAR) lengthOneDimension? - | typeName=CHAR lengthOneDimension? ((CHARACTER SET | CHARSET) charsetName)? - | typeName=DATETIME + : typeName=DATETIME | typeName=INT | typeName=DOUBLE | typeName=LONG @@ -329,14 +327,6 @@ convertedDataType | typeName=STRING ; -lengthOneDimension - : '(' decimalLiteral ')' - ; - -lengthTwoDimension - : '(' decimalLiteral ',' decimalLiteral ')' - ; - aggregateWindowedFunction : (AVG | MAX | MIN | SUM) '(' aggregator=(ALL | DISTINCT)? functionArg ')' diff --git a/src/test/java/com/amazon/opendistroforelasticsearch/sql/esintgtest/SQLFunctionsIT.java b/src/test/java/com/amazon/opendistroforelasticsearch/sql/esintgtest/SQLFunctionsIT.java index 6c04ca8ced..b0477221be 100644 --- a/src/test/java/com/amazon/opendistroforelasticsearch/sql/esintgtest/SQLFunctionsIT.java +++ b/src/test/java/com/amazon/opendistroforelasticsearch/sql/esintgtest/SQLFunctionsIT.java @@ -50,6 +50,7 @@ import static org.hamcrest.Matchers.hasEntry; import static org.hamcrest.Matchers.hasItems; import static org.hamcrest.Matchers.hasValue; +import static org.hamcrest.Matchers.instanceOf; import static org.hamcrest.Matchers.is; import static org.hamcrest.Matchers.isEmptyOrNullString; import static org.hamcrest.Matchers.not; @@ -160,12 +161,41 @@ public void caseChangeWithAggregationTest() throws IOException { } @Test - public void castFieldWithoutAliasTest() throws IOException { - String query = "SELECT CAST(age AS DOUBLE) FROM " + TestsConstants.TEST_INDEX_ACCOUNT + " /account limit 5"; + public void castIntFieldToDoubleWithoutAliasTest() throws IOException { + String query = "SELECT CAST(age AS DOUBLE) FROM " + TestsConstants.TEST_INDEX_ACCOUNT + " /account limit 1"; - JSONObject response = executeQuery(query); - JSONArray hits = getHits(response); - System.out.println(hits.toString()); + final SearchHit[] hits = query(query).getHits(); + assertTrue(hits[0].getFields().containsKey("cast_age")); + assertTrue(hits[0].getFields().get("cast_age").getValue() instanceof Double); + } + + @Test + public void castIntFieldToDoubleWithAliasTest() throws IOException { + String query = "SELECT CAST(age AS DOUBLE) AS test_alias " + + "FROM " + TestsConstants.TEST_INDEX_ACCOUNT + " /account limit 1"; + + final SearchHit[] hits = query(query).getHits(); + assertTrue(hits[0].getFields().containsKey("test_alias")); + assertTrue(hits[0].getFields().get("test_alias").getValue() instanceof Double); + } + + @Test + public void castIntFieldToStringWithoutAliasTest() throws IOException { + String query = "SELECT CAST(balance AS STRING) FROM " + TestsConstants.TEST_INDEX_ACCOUNT + " /account limit 1"; + + final SearchHit[] hits = query(query).getHits(); + assertTrue(hits[0].getFields().containsKey("cast_balance")); + assertTrue(hits[0].getFields().get("cast_balance").getValue() instanceof String); + } + + @Test + public void castIntFieldToStringWithAliasTest() throws IOException { + String query = "SELECT CAST(balance AS STRING) AS cast_string_alias " + + "FROM " + TestsConstants.TEST_INDEX_ACCOUNT + " /account limit 1"; + + final SearchHit[] hits = query(query).getHits(); + assertTrue(hits[0].getFields().containsKey("cast_string_alias")); + assertTrue(hits[0].getFields().get("cast_string_alias").getValue() instanceof String); } @Test From 841b2c59843cae19bc1ea4ddd31726fb10ec7715 Mon Sep 17 00:00:00 2001 From: David Cui Date: Fri, 25 Oct 2019 10:49:46 -0700 Subject: [PATCH 06/15] undoing release note change in this branch and removing more unnecessary grammar --- opendistro-elasticsearch-sql.release-notes | 6 ------ src/main/antlr/OpenDistroSqlParser.g4 | 11 ----------- 2 files changed, 17 deletions(-) diff --git a/opendistro-elasticsearch-sql.release-notes b/opendistro-elasticsearch-sql.release-notes index ae597f6a49..9b9a45b545 100644 --- a/opendistro-elasticsearch-sql.release-notes +++ b/opendistro-elasticsearch-sql.release-notes @@ -1,9 +1,3 @@ -## 2019-10-15, Version 1.2.1 (Current) - -### Notable changes -* Feature [#202](https://github.com/opendistro-for-elasticsearch/sql/issues/202): Elasticsearch 7.2.1 compatibility - - ## 2019-07-23, Version 1.2.0 (Current) ### Notable changes diff --git a/src/main/antlr/OpenDistroSqlParser.g4 b/src/main/antlr/OpenDistroSqlParser.g4 index 6c6f4169e3..7b74bee935 100644 --- a/src/main/antlr/OpenDistroSqlParser.g4 +++ b/src/main/antlr/OpenDistroSqlParser.g4 @@ -231,7 +231,6 @@ uid simpleId : ID | DOT_ID // note: the current scope by adding DOT_ID to simpleId is large, move DOT_ID upwards tablename if needed - | charsetNameBase | STRING_LITERAL | keywordsCanBeId | functionNameBase @@ -244,7 +243,6 @@ dottedId charsetName : BINARY - | charsetNameBase | STRING_LITERAL | CHARSET_REVERSE_QUOTE_STRING ; @@ -410,15 +408,6 @@ mathOperator // Simple id sets // (that keyword, which can be id) -charsetNameBase - : ARMSCII8 | ASCII | BIG5 | CP1250 | CP1251 | CP1256 | CP1257 - | CP850 | CP852 | CP866 | CP932 | DEC8 | EUCJPMS | EUCKR - | GB2312 | GBK | GEOSTD8 | GREEK | HEBREW | HP8 | KEYBCS2 - | KOI8R | KOI8U | LATIN1 | LATIN2 | LATIN5 | LATIN7 | MACCE - | MACROMAN | SJIS | SWE7 | TIS620 | UCS2 | UJIS | UTF16 - | UTF16LE | UTF32 | UTF8 | UTF8MB3 | UTF8MB4 - ; - keywordsCanBeId : FULL | FIELD | D | T | TS // OD SQL and ODBC special From 698ab349c7021aa63e4a30ac56bf678187382212 Mon Sep 17 00:00:00 2001 From: David Cui Date: Fri, 25 Oct 2019 10:55:37 -0700 Subject: [PATCH 07/15] removed more unnecessary lines and comments --- src/main/antlr/OpenDistroSqlParser.g4 | 10 +--------- .../sql/query/DefaultQueryAction.java | 1 - 2 files changed, 1 insertion(+), 10 deletions(-) diff --git a/src/main/antlr/OpenDistroSqlParser.g4 b/src/main/antlr/OpenDistroSqlParser.g4 index 7b74bee935..ec25c00262 100644 --- a/src/main/antlr/OpenDistroSqlParser.g4 +++ b/src/main/antlr/OpenDistroSqlParser.g4 @@ -241,12 +241,6 @@ dottedId | '.' uid ; -charsetName - : BINARY - | STRING_LITERAL - | CHARSET_REVERSE_QUOTE_STRING - ; - // Literals decimalLiteral @@ -302,9 +296,7 @@ functionCall ; specificFunction - : CONVERT '(' expression separator=',' convertedDataType ')' #dataTypeFunctionCall - | CONVERT '(' expression USING charsetName ')' #dataTypeFunctionCall - | CAST '(' expression AS convertedDataType ')' #dataTypeFunctionCall + : CAST '(' expression AS convertedDataType ')' #dataTypeFunctionCall | CASE expression caseFuncAlternative+ (ELSE elseArg=functionArg)? END #caseFunctionCall | CASE caseFuncAlternative+ diff --git a/src/main/java/com/amazon/opendistroforelasticsearch/sql/query/DefaultQueryAction.java b/src/main/java/com/amazon/opendistroforelasticsearch/sql/query/DefaultQueryAction.java index f662f4a8c0..32c1894cdd 100644 --- a/src/main/java/com/amazon/opendistroforelasticsearch/sql/query/DefaultQueryAction.java +++ b/src/main/java/com/amazon/opendistroforelasticsearch/sql/query/DefaultQueryAction.java @@ -96,7 +96,6 @@ public SqlElasticSearchRequestBuilder explain() throws SqlParseException { setIndicesAndTypes(); setFields(select.getFields()); - // if cast, add casting field to include list, should do the trick bb setWhere(select.getWhere()); setSorts(select.getOrderBys()); setLimit(select.getOffset(), select.getRowCount()); From 2e27903f72f4b3a01cd70252a4b3175dd1ee0b7c Mon Sep 17 00:00:00 2001 From: David Cui Date: Mon, 28 Oct 2019 11:00:08 -0700 Subject: [PATCH 08/15] committing changes to fetch upstream --- .../sql/executor/format/SelectResultSet.java | 7 +++++- .../sql/utils/SQLFunctions.java | 24 ++++++++++++++++++- .../sql/esintgtest/SQLFunctionsIT.java | 24 +++++++++++++++++++ 3 files changed, 53 insertions(+), 2 deletions(-) diff --git a/src/main/java/com/amazon/opendistroforelasticsearch/sql/executor/format/SelectResultSet.java b/src/main/java/com/amazon/opendistroforelasticsearch/sql/executor/format/SelectResultSet.java index 5d708aa1f8..da158ba22d 100644 --- a/src/main/java/com/amazon/opendistroforelasticsearch/sql/executor/format/SelectResultSet.java +++ b/src/main/java/com/amazon/opendistroforelasticsearch/sql/executor/format/SelectResultSet.java @@ -15,6 +15,7 @@ package com.amazon.opendistroforelasticsearch.sql.executor.format; +import com.alibaba.druid.sql.ast.expr.SQLCastExpr; import com.amazon.opendistroforelasticsearch.sql.domain.Field; import com.amazon.opendistroforelasticsearch.sql.domain.JoinSelect; import com.amazon.opendistroforelasticsearch.sql.domain.MethodField; @@ -315,7 +316,11 @@ private Schema.Type fetchMethodReturnType(Field field) { // TODO: return type information is disconnected from the function definitions in SQLFunctions. // Refactor SQLFunctions to have functions self-explanatory (types, scripts) and pluggable // (similar to Strategy pattern) - + MethodField methodField = (MethodField) field; + if (field.getExpression() instanceof SQLCastExpr) { + return SQLFunctions.getCastFunctionReturnType( + ((SQLCastExpr) field.getExpression()).getDataType().getName()); + } return SQLFunctions.getScriptFunctionReturnType( ((ScriptMethodField) field).getFunctionName()); } diff --git a/src/main/java/com/amazon/opendistroforelasticsearch/sql/utils/SQLFunctions.java b/src/main/java/com/amazon/opendistroforelasticsearch/sql/utils/SQLFunctions.java index fd8f90632e..5a5f02e0c0 100644 --- a/src/main/java/com/amazon/opendistroforelasticsearch/sql/utils/SQLFunctions.java +++ b/src/main/java/com/amazon/opendistroforelasticsearch/sql/utils/SQLFunctions.java @@ -65,7 +65,7 @@ public class SQLFunctions { "day_of_week", "hour_of_day", "minute_of_day", "minute_of_hour", "second_of_minute" ); - private static final Set utilityFunctions = Sets.newHashSet("field", "assign"); + private static final Set utilityFunctions = Sets.newHashSet("field", "assign", "cast"); public static final Set builtInFunctions = Stream.of( numberOperators, @@ -271,6 +271,8 @@ public Tuple function(String methodName, List paramers, case "assign": functionStr = assign((SQLExpr) paramers.get(0).value); break; + case "cast": + System.out.println("in SQLFunctions.java, successfully know that we're doing some casting"); default: } @@ -614,4 +616,24 @@ public static Schema.Type getScriptFunctionReturnType(String functionName) { "The following method is not supported in Schema: %s", functionName)); } + + public static Schema.Type getCastFunctionReturnType(String castType) { + if (castType.toUpperCase().equals("FLOAT")) { + return Schema.Type.FLOAT; + } else if (castType.toUpperCase().equals("DOUBLE")) { + return Schema.Type.DOUBLE; + } else if (castType.toUpperCase().equals("INT")) { + return Schema.Type.INTEGER; + } else if (castType.toUpperCase().equals("STRING")) { + return Schema.Type.TEXT; + } else if (castType.toUpperCase().equals("DATETIME")) { + return Schema.Type.DATE; + } else if (castType.toUpperCase().equals("LONG")) { + return Schema.Type.LONG; + } + + throw new UnsupportedOperationException( + StringUtils.format("The following type is not supported by cast(): %s", castType) + ); + } } diff --git a/src/test/java/com/amazon/opendistroforelasticsearch/sql/esintgtest/SQLFunctionsIT.java b/src/test/java/com/amazon/opendistroforelasticsearch/sql/esintgtest/SQLFunctionsIT.java index b0477221be..4befad264c 100644 --- a/src/test/java/com/amazon/opendistroforelasticsearch/sql/esintgtest/SQLFunctionsIT.java +++ b/src/test/java/com/amazon/opendistroforelasticsearch/sql/esintgtest/SQLFunctionsIT.java @@ -198,6 +198,26 @@ public void castIntFieldToStringWithAliasTest() throws IOException { assertTrue(hits[0].getFields().get("cast_string_alias").getValue() instanceof String); } + @Ignore + @Test + public void castIntFieldToFloatWithoutAliasTest() throws IOException { + String query = "SELECT CAST(balance AS FLOAT) FROM " + TestsConstants.TEST_INDEX_ACCOUNT + " /account limit 1"; + + final SearchHit[] hits = query(query).getHits(); + System.out.println(hits[0].getFields().toString()); + assertTrue(hits[0].getFields().containsKey("cast_balance")); + assertTrue(hits[0].getFields().get("cast_balance").getValue() instanceof Double); + } + + @Test + public void castIntFieldToFloatWithoutAliasJdbcFormatTest() throws IOException { + JSONObject response = executeJdbcRequest( + "SELECT CAST(balance AS FLOAT) FROM " + TestsConstants.TEST_INDEX_ACCOUNT + "/account limit 1"); + + System.out.println(response.toString()); +// assertTrue(response.getJSONArray("schema").get(1).equals("float")); + } + @Test public void concat_ws_field_and_string() throws Exception { //here is a bug,csv field with spa @@ -341,4 +361,8 @@ private SearchHits query(String query) throws IOException { rsp); return SearchResponse.fromXContent(parser).getHits(); } + + private JSONObject executeJdbcRequest(String query) { + return new JSONObject(executeQuery(query, "jdbc")); + } } From ecd43a867c2fa4b4a9e0f4e07eaa71e9cc595d1e Mon Sep 17 00:00:00 2001 From: David Cui Date: Mon, 28 Oct 2019 11:32:36 -0700 Subject: [PATCH 09/15] added additional ITs for float conversion in jdbc format --- .../sql/esintgtest/SQLFunctionsIT.java | 18 ++++++++++++++---- 1 file changed, 14 insertions(+), 4 deletions(-) diff --git a/src/test/java/com/amazon/opendistroforelasticsearch/sql/esintgtest/SQLFunctionsIT.java b/src/test/java/com/amazon/opendistroforelasticsearch/sql/esintgtest/SQLFunctionsIT.java index 4befad264c..837aa87f6d 100644 --- a/src/test/java/com/amazon/opendistroforelasticsearch/sql/esintgtest/SQLFunctionsIT.java +++ b/src/test/java/com/amazon/opendistroforelasticsearch/sql/esintgtest/SQLFunctionsIT.java @@ -210,12 +210,22 @@ public void castIntFieldToFloatWithoutAliasTest() throws IOException { } @Test - public void castIntFieldToFloatWithoutAliasJdbcFormatTest() throws IOException { + public void castIntFieldToFloatWithoutAliasJdbcFormatTest() { JSONObject response = executeJdbcRequest( - "SELECT CAST(balance AS FLOAT) FROM " + TestsConstants.TEST_INDEX_ACCOUNT + "/account limit 1"); + "SELECT CAST(balance AS FLOAT) FROM " + TestsConstants.TEST_INDEX_ACCOUNT + " limit 1"); - System.out.println(response.toString()); -// assertTrue(response.getJSONArray("schema").get(1).equals("float")); + String float_type_cast = "{\"name\":\"cast_balance\",\"type\":\"float\"}"; + assertEquals(response.getJSONArray("schema").get(0).toString(), float_type_cast); + } + + @Test + public void castIntFieldToFloatWithAliasJdbcFormatTest() { + JSONObject response = executeJdbcRequest( + "SELECT CAST(balance AS FLOAT) AS jdbc_float_alias " + + "FROM " + TestsConstants.TEST_INDEX_ACCOUNT + " limit 1"); + + String float_type_cast = "{\"name\":\"jdbc_float_alias\",\"type\":\"float\"}"; + assertEquals(response.getJSONArray("schema").get(0).toString(), float_type_cast); } @Test From adbade085c76671d39b683331205e59d84b5f303 Mon Sep 17 00:00:00 2001 From: David Cui Date: Mon, 28 Oct 2019 11:38:57 -0700 Subject: [PATCH 10/15] removed some unnecessary code --- .../opendistroforelasticsearch/sql/utils/SQLFunctions.java | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/src/main/java/com/amazon/opendistroforelasticsearch/sql/utils/SQLFunctions.java b/src/main/java/com/amazon/opendistroforelasticsearch/sql/utils/SQLFunctions.java index 5a5f02e0c0..7d4cc51c1a 100644 --- a/src/main/java/com/amazon/opendistroforelasticsearch/sql/utils/SQLFunctions.java +++ b/src/main/java/com/amazon/opendistroforelasticsearch/sql/utils/SQLFunctions.java @@ -65,7 +65,7 @@ public class SQLFunctions { "day_of_week", "hour_of_day", "minute_of_day", "minute_of_hour", "second_of_minute" ); - private static final Set utilityFunctions = Sets.newHashSet("field", "assign", "cast"); + private static final Set utilityFunctions = Sets.newHashSet("field", "assign"); public static final Set builtInFunctions = Stream.of( numberOperators, @@ -271,8 +271,6 @@ public Tuple function(String methodName, List paramers, case "assign": functionStr = assign((SQLExpr) paramers.get(0).value); break; - case "cast": - System.out.println("in SQLFunctions.java, successfully know that we're doing some casting"); default: } From 3eb3d2e09e785b082b2044243b88dccafcb8e04e Mon Sep 17 00:00:00 2001 From: David Cui Date: Wed, 30 Oct 2019 17:22:25 -0700 Subject: [PATCH 11/15] added support for ORDER BY and GROUP BY in jdbc format, and more tests --- .../sql/executor/format/SelectResultSet.java | 7 +++ .../sql/query/DefaultQueryAction.java | 15 ++++- .../sql/utils/SQLFunctions.java | 33 +++++----- .../sql/esintgtest/SQLFunctionsIT.java | 62 +++++++++++++++---- 4 files changed, 87 insertions(+), 30 deletions(-) diff --git a/src/main/java/com/amazon/opendistroforelasticsearch/sql/executor/format/SelectResultSet.java b/src/main/java/com/amazon/opendistroforelasticsearch/sql/executor/format/SelectResultSet.java index da158ba22d..ebfc25efda 100644 --- a/src/main/java/com/amazon/opendistroforelasticsearch/sql/executor/format/SelectResultSet.java +++ b/src/main/java/com/amazon/opendistroforelasticsearch/sql/executor/format/SelectResultSet.java @@ -125,6 +125,9 @@ private void loadFromEsState(Query query) { // Assumption is all indices share the same mapping which is validated in TermFieldRewriter. Map> indexMappings = mappings.values().iterator().next(); + // if index mappings size is 0 and the expression is a cast: that means that we are casting by alias + // if so, add the original field that was being looked at to the mapping (how?) + /* * There are three cases regarding type name to consider: * 1. If the correct type name was given, its typeMapping is retrieved @@ -246,9 +249,13 @@ private List fetchFields(Query query) { List groupByFields = select.getGroupBys().isEmpty() ? new ArrayList<>() : select.getGroupBys().get(0); + for (Field selectField : select.getFields()) { if (selectField instanceof MethodField && !selectField.isScriptField()) { groupByFields.add(selectField); + } else if (selectField.isScriptField() + && selectField.getAlias().equals(groupByFields.get(0).getName())) { + return select.getFields(); } } return groupByFields; diff --git a/src/main/java/com/amazon/opendistroforelasticsearch/sql/query/DefaultQueryAction.java b/src/main/java/com/amazon/opendistroforelasticsearch/sql/query/DefaultQueryAction.java index 32c1894cdd..90030d081d 100644 --- a/src/main/java/com/amazon/opendistroforelasticsearch/sql/query/DefaultQueryAction.java +++ b/src/main/java/com/amazon/opendistroforelasticsearch/sql/query/DefaultQueryAction.java @@ -268,8 +268,15 @@ private String getNullOrderString(SQLBinaryOpExpr expr) { private ScriptSortType getScriptSortType(Order order) { ScriptSortType scriptSortType; - ScriptMethodField smf = (ScriptMethodField) order.getSortField(); - Schema.Type scriptFunctionReturnType = SQLFunctions.getScriptFunctionReturnType(smf.getFunctionName()); + Schema.Type scriptFunctionReturnType; + if (order.getSortField().getExpression() instanceof SQLCastExpr) { + scriptFunctionReturnType = SQLFunctions.getCastFunctionReturnType( + ((SQLCastExpr) order.getSortField().getExpression()).getDataType().getName()); + } else { + ScriptMethodField smf = (ScriptMethodField) order.getSortField(); + scriptFunctionReturnType = SQLFunctions.getScriptFunctionReturnType(smf.getFunctionName()); + } + // as of now script function return type returns only text and double switch (scriptFunctionReturnType) { @@ -278,9 +285,11 @@ private ScriptSortType getScriptSortType(Order order) { break; case DOUBLE: + case FLOAT: + case INTEGER: + case LONG: scriptSortType = ScriptSortType.NUMBER; break; - default: throw new RuntimeException("unknown"); } diff --git a/src/main/java/com/amazon/opendistroforelasticsearch/sql/utils/SQLFunctions.java b/src/main/java/com/amazon/opendistroforelasticsearch/sql/utils/SQLFunctions.java index 7d4cc51c1a..a335586190 100644 --- a/src/main/java/com/amazon/opendistroforelasticsearch/sql/utils/SQLFunctions.java +++ b/src/main/java/com/amazon/opendistroforelasticsearch/sql/utils/SQLFunctions.java @@ -616,22 +616,23 @@ public static Schema.Type getScriptFunctionReturnType(String functionName) { } public static Schema.Type getCastFunctionReturnType(String castType) { - if (castType.toUpperCase().equals("FLOAT")) { - return Schema.Type.FLOAT; - } else if (castType.toUpperCase().equals("DOUBLE")) { - return Schema.Type.DOUBLE; - } else if (castType.toUpperCase().equals("INT")) { - return Schema.Type.INTEGER; - } else if (castType.toUpperCase().equals("STRING")) { - return Schema.Type.TEXT; - } else if (castType.toUpperCase().equals("DATETIME")) { - return Schema.Type.DATE; - } else if (castType.toUpperCase().equals("LONG")) { - return Schema.Type.LONG; + switch (castType) { + case "FLOAT": + return Schema.Type.FLOAT; + case "DOUBLE": + return Schema.Type.DOUBLE; + case "INT": + return Schema.Type.INTEGER; + case "STRING": + return Schema.Type.TEXT; + case "DATETIME": + return Schema.Type.DATE; + case "LONG": + return Schema.Type.LONG; + default: + throw new UnsupportedOperationException( + StringUtils.format("The following type is not supported by cast(): %s", castType) + ); } - - throw new UnsupportedOperationException( - StringUtils.format("The following type is not supported by cast(): %s", castType) - ); } } diff --git a/src/test/java/com/amazon/opendistroforelasticsearch/sql/esintgtest/SQLFunctionsIT.java b/src/test/java/com/amazon/opendistroforelasticsearch/sql/esintgtest/SQLFunctionsIT.java index 837aa87f6d..872f3794ff 100644 --- a/src/test/java/com/amazon/opendistroforelasticsearch/sql/esintgtest/SQLFunctionsIT.java +++ b/src/test/java/com/amazon/opendistroforelasticsearch/sql/esintgtest/SQLFunctionsIT.java @@ -38,6 +38,7 @@ import java.util.stream.IntStream; import static com.amazon.opendistroforelasticsearch.sql.esintgtest.TestsConstants.TEST_INDEX_ACCOUNT; +import static com.amazon.opendistroforelasticsearch.sql.util.MatcherUtils.hit; import static com.amazon.opendistroforelasticsearch.sql.util.MatcherUtils.hitAny; import static com.amazon.opendistroforelasticsearch.sql.util.MatcherUtils.kvDouble; import static com.amazon.opendistroforelasticsearch.sql.util.MatcherUtils.kvString; @@ -198,17 +199,6 @@ public void castIntFieldToStringWithAliasTest() throws IOException { assertTrue(hits[0].getFields().get("cast_string_alias").getValue() instanceof String); } - @Ignore - @Test - public void castIntFieldToFloatWithoutAliasTest() throws IOException { - String query = "SELECT CAST(balance AS FLOAT) FROM " + TestsConstants.TEST_INDEX_ACCOUNT + " /account limit 1"; - - final SearchHit[] hits = query(query).getHits(); - System.out.println(hits[0].getFields().toString()); - assertTrue(hits[0].getFields().containsKey("cast_balance")); - assertTrue(hits[0].getFields().get("cast_balance").getValue() instanceof Double); - } - @Test public void castIntFieldToFloatWithoutAliasJdbcFormatTest() { JSONObject response = executeJdbcRequest( @@ -228,6 +218,56 @@ public void castIntFieldToFloatWithAliasJdbcFormatTest() { assertEquals(response.getJSONArray("schema").get(0).toString(), float_type_cast); } + @Test + public void castIntFieldToDoubleWithoutAliasOrderByTest() throws IOException { + String query = "SELECT CAST(age AS DOUBLE) FROM " + TestsConstants.TEST_INDEX_ACCOUNT + " /account " + + "ORDER BY age limit 1"; + + final SearchHit[] hits = query(query).getHits(); + assertTrue(hits[0].getFields().containsKey("cast_age")); + assertTrue(hits[0].getFields().get("cast_age").getValue() instanceof Double); + } + + @Test + public void castIntFieldToDoubleWithAliasOrderByTest() throws IOException { + String query = "SELECT CAST(age AS DOUBLE) AS alias FROM " + TestsConstants.TEST_INDEX_ACCOUNT + " /account " + + "ORDER BY alias limit 1"; + + final SearchHit[] hits = query(query).getHits(); + assertTrue(hits[0].getFields().containsKey("alias")); + assertTrue(hits[0].getFields().get("alias").getValue() instanceof Double); + } + + @Test + public void castIntFieldToFloatWithoutAliasJdbcFormatGroupByTest() { + JSONObject response = executeJdbcRequest( + "SELECT CAST(balance AS FLOAT) AS jdbc_float_alias " + + "FROM " + TestsConstants.TEST_INDEX_ACCOUNT + " GROUP BY balance LIMIT 5"); + + String float_type_cast = "{\"name\":\"balance\",\"type\":\"long\"}"; + assertEquals(response.getJSONArray("schema").get(0).toString(), float_type_cast); + } + + @Test + public void castIntFieldToFloatWithAliasJdbcFormatGroupByTest() { + JSONObject response = executeJdbcRequest( + "SELECT CAST(balance AS FLOAT) AS jdbc_float_alias " + + "FROM " + TestsConstants.TEST_INDEX_ACCOUNT + " GROUP BY jdbc_float_alias LIMIT 5"); + + String float_type_cast = "{\"name\":\"jdbc_float_alias\",\"type\":\"float\"}"; + assertEquals(response.getJSONArray("schema").get(0).toString(), float_type_cast); + } + + @Test + public void castIntFieldToDoubleWithAliasJdbcFormatGroupByTest() { + JSONObject response = executeJdbcRequest( + "SELECT CAST(balance AS DOUBLE) AS jdbc_double_alias " + + "FROM " + TestsConstants.TEST_INDEX_ACCOUNT + " GROUP BY jdbc_double_alias LIMIT 5"); + + String float_type_cast = "{\"name\":\"jdbc_double_alias\",\"type\":\"double\"}"; + assertEquals(response.getJSONArray("schema").get(0).toString(), float_type_cast); + } + @Test public void concat_ws_field_and_string() throws Exception { //here is a bug,csv field with spa From cfed26406df1d370b66b122837795c17e1ab1f37 Mon Sep 17 00:00:00 2001 From: David Cui Date: Wed, 30 Oct 2019 17:24:31 -0700 Subject: [PATCH 12/15] removing unused line --- .../sql/executor/format/SelectResultSet.java | 1 - 1 file changed, 1 deletion(-) diff --git a/src/main/java/com/amazon/opendistroforelasticsearch/sql/executor/format/SelectResultSet.java b/src/main/java/com/amazon/opendistroforelasticsearch/sql/executor/format/SelectResultSet.java index ebfc25efda..954840f602 100644 --- a/src/main/java/com/amazon/opendistroforelasticsearch/sql/executor/format/SelectResultSet.java +++ b/src/main/java/com/amazon/opendistroforelasticsearch/sql/executor/format/SelectResultSet.java @@ -323,7 +323,6 @@ private Schema.Type fetchMethodReturnType(Field field) { // TODO: return type information is disconnected from the function definitions in SQLFunctions. // Refactor SQLFunctions to have functions self-explanatory (types, scripts) and pluggable // (similar to Strategy pattern) - MethodField methodField = (MethodField) field; if (field.getExpression() instanceof SQLCastExpr) { return SQLFunctions.getCastFunctionReturnType( ((SQLCastExpr) field.getExpression()).getDataType().getName()); From 059e065d785db708fbf82d16e003f46385f8db64 Mon Sep 17 00:00:00 2001 From: David Cui Date: Thu, 31 Oct 2019 10:46:45 -0700 Subject: [PATCH 13/15] addressed comments to add usage of IsMapContaining() and extracted assert statements to private method --- .../sql/esintgtest/SQLFunctionsIT.java | 77 +++++++++++-------- 1 file changed, 47 insertions(+), 30 deletions(-) diff --git a/src/test/java/com/amazon/opendistroforelasticsearch/sql/esintgtest/SQLFunctionsIT.java b/src/test/java/com/amazon/opendistroforelasticsearch/sql/esintgtest/SQLFunctionsIT.java index 872f3794ff..78dee4d7d5 100644 --- a/src/test/java/com/amazon/opendistroforelasticsearch/sql/esintgtest/SQLFunctionsIT.java +++ b/src/test/java/com/amazon/opendistroforelasticsearch/sql/esintgtest/SQLFunctionsIT.java @@ -26,6 +26,8 @@ import org.elasticsearch.search.SearchHit; import org.elasticsearch.search.SearchHits; import org.hamcrest.Matcher; +import org.hamcrest.Matchers; +import org.hamcrest.collection.IsMapContaining; import org.json.JSONArray; import org.json.JSONObject; import org.junit.Assert; @@ -34,6 +36,7 @@ import java.io.IOException; import java.util.ArrayList; +import java.util.Date; import java.util.List; import java.util.stream.IntStream; @@ -163,46 +166,38 @@ public void caseChangeWithAggregationTest() throws IOException { @Test public void castIntFieldToDoubleWithoutAliasTest() throws IOException { - String query = "SELECT CAST(age AS DOUBLE) FROM " + TestsConstants.TEST_INDEX_ACCOUNT + " /account limit 1"; + String query = "SELECT CAST(age AS DOUBLE) FROM " + TestsConstants.TEST_INDEX_ACCOUNT + " LIMIT 5"; - final SearchHit[] hits = query(query).getHits(); - assertTrue(hits[0].getFields().containsKey("cast_age")); - assertTrue(hits[0].getFields().get("cast_age").getValue() instanceof Double); + checkSuccessfulFieldCast(query, "cast_age", "DOUBLE"); } @Test public void castIntFieldToDoubleWithAliasTest() throws IOException { - String query = "SELECT CAST(age AS DOUBLE) AS test_alias " + - "FROM " + TestsConstants.TEST_INDEX_ACCOUNT + " /account limit 1"; + String query = "SELECT CAST(age AS DOUBLE) AS test_alias FROM " + TestsConstants.TEST_INDEX_ACCOUNT + + " LIMIT 5"; - final SearchHit[] hits = query(query).getHits(); - assertTrue(hits[0].getFields().containsKey("test_alias")); - assertTrue(hits[0].getFields().get("test_alias").getValue() instanceof Double); + checkSuccessfulFieldCast(query, "test_alias", "DOUBLE"); } @Test public void castIntFieldToStringWithoutAliasTest() throws IOException { - String query = "SELECT CAST(balance AS STRING) FROM " + TestsConstants.TEST_INDEX_ACCOUNT + " /account limit 1"; + String query = "SELECT CAST(balance AS STRING) FROM " + TestsConstants.TEST_INDEX_ACCOUNT + " LIMIT 5"; - final SearchHit[] hits = query(query).getHits(); - assertTrue(hits[0].getFields().containsKey("cast_balance")); - assertTrue(hits[0].getFields().get("cast_balance").getValue() instanceof String); + checkSuccessfulFieldCast(query, "cast_balance", "STRING"); } @Test public void castIntFieldToStringWithAliasTest() throws IOException { - String query = "SELECT CAST(balance AS STRING) AS cast_string_alias " + - "FROM " + TestsConstants.TEST_INDEX_ACCOUNT + " /account limit 1"; + String query = "SELECT CAST(balance AS STRING) AS cast_string_alias FROM " + TestsConstants.TEST_INDEX_ACCOUNT + + " LIMIT 5"; - final SearchHit[] hits = query(query).getHits(); - assertTrue(hits[0].getFields().containsKey("cast_string_alias")); - assertTrue(hits[0].getFields().get("cast_string_alias").getValue() instanceof String); + checkSuccessfulFieldCast(query, "cast_string_alias", "STRING"); } @Test public void castIntFieldToFloatWithoutAliasJdbcFormatTest() { JSONObject response = executeJdbcRequest( - "SELECT CAST(balance AS FLOAT) FROM " + TestsConstants.TEST_INDEX_ACCOUNT + " limit 1"); + "SELECT CAST(balance AS FLOAT) FROM " + TestsConstants.TEST_INDEX_ACCOUNT + " LIMIT 5"); String float_type_cast = "{\"name\":\"cast_balance\",\"type\":\"float\"}"; assertEquals(response.getJSONArray("schema").get(0).toString(), float_type_cast); @@ -212,7 +207,7 @@ public void castIntFieldToFloatWithoutAliasJdbcFormatTest() { public void castIntFieldToFloatWithAliasJdbcFormatTest() { JSONObject response = executeJdbcRequest( "SELECT CAST(balance AS FLOAT) AS jdbc_float_alias " + - "FROM " + TestsConstants.TEST_INDEX_ACCOUNT + " limit 1"); + "FROM " + TestsConstants.TEST_INDEX_ACCOUNT + " limit 5"); String float_type_cast = "{\"name\":\"jdbc_float_alias\",\"type\":\"float\"}"; assertEquals(response.getJSONArray("schema").get(0).toString(), float_type_cast); @@ -220,22 +215,17 @@ public void castIntFieldToFloatWithAliasJdbcFormatTest() { @Test public void castIntFieldToDoubleWithoutAliasOrderByTest() throws IOException { - String query = "SELECT CAST(age AS DOUBLE) FROM " + TestsConstants.TEST_INDEX_ACCOUNT + " /account " + - "ORDER BY age limit 1"; + String query = "SELECT CAST(age AS DOUBLE) FROM " + TestsConstants.TEST_INDEX_ACCOUNT + " ORDER BY age LIMIT 5"; - final SearchHit[] hits = query(query).getHits(); - assertTrue(hits[0].getFields().containsKey("cast_age")); - assertTrue(hits[0].getFields().get("cast_age").getValue() instanceof Double); + checkSuccessfulFieldCast(query, "cast_age", "DOUBLE"); } @Test public void castIntFieldToDoubleWithAliasOrderByTest() throws IOException { - String query = "SELECT CAST(age AS DOUBLE) AS alias FROM " + TestsConstants.TEST_INDEX_ACCOUNT + " /account " + - "ORDER BY alias limit 1"; + String query = "SELECT CAST(age AS DOUBLE) AS alias FROM " + TestsConstants.TEST_INDEX_ACCOUNT + + " ORDER BY alias LIMIT 5"; - final SearchHit[] hits = query(query).getHits(); - assertTrue(hits[0].getFields().containsKey("alias")); - assertTrue(hits[0].getFields().get("alias").getValue() instanceof Double); + checkSuccessfulFieldCast(query, "alias", "DOUBLE"); } @Test @@ -412,6 +402,33 @@ private SearchHits query(String query) throws IOException { return SearchResponse.fromXContent(parser).getHits(); } + private void checkSuccessfulFieldCast(String query, String field, String castType) throws IOException { + SearchHit[] hits = query(query).getHits(); + for (int i = 0; i < hits.length; ++i) { + Assert.assertThat(hits[i].getFields(), IsMapContaining.hasKey(field)); + switch (castType) { + case "FLOAT": + assertTrue(hits[i].getFields().get(field).getValue() instanceof Float); + break; + case "DOUBLE": + assertTrue(hits[i].getFields().get(field).getValue() instanceof Double); + break; + case "INT": + assertTrue(hits[i].getFields().get(field).getValue() instanceof Integer); + break; + case "STRING": + assertTrue(hits[i].getFields().get(field).getValue() instanceof String); + break; + case "DATETIME": + assertTrue(hits[i].getFields().get(field).getValue() instanceof Date); + break; + case "LONG": + assertTrue(hits[i].getFields().get(field).getValue() instanceof Long); + break; + } + } + } + private JSONObject executeJdbcRequest(String query) { return new JSONObject(executeQuery(query, "jdbc")); } From 87fa70afe7a470a7ea2dc5507f93a2627d06c4e3 Mon Sep 17 00:00:00 2001 From: David Cui Date: Thu, 31 Oct 2019 14:56:48 -0700 Subject: [PATCH 14/15] added value checks to test methods --- .../sql/esintgtest/SQLFunctionsIT.java | 109 +++++++++++++----- 1 file changed, 80 insertions(+), 29 deletions(-) diff --git a/src/test/java/com/amazon/opendistroforelasticsearch/sql/esintgtest/SQLFunctionsIT.java b/src/test/java/com/amazon/opendistroforelasticsearch/sql/esintgtest/SQLFunctionsIT.java index 78dee4d7d5..246137de40 100644 --- a/src/test/java/com/amazon/opendistroforelasticsearch/sql/esintgtest/SQLFunctionsIT.java +++ b/src/test/java/com/amazon/opendistroforelasticsearch/sql/esintgtest/SQLFunctionsIT.java @@ -16,7 +16,6 @@ package com.amazon.opendistroforelasticsearch.sql.esintgtest; -import com.amazon.opendistroforelasticsearch.sql.util.MatcherUtils; import org.elasticsearch.action.search.SearchResponse; import org.elasticsearch.common.xcontent.LoggingDeprecationHandler; import org.elasticsearch.common.xcontent.NamedXContentRegistry; @@ -25,23 +24,17 @@ import org.elasticsearch.common.xcontent.XContentType; import org.elasticsearch.search.SearchHit; import org.elasticsearch.search.SearchHits; -import org.hamcrest.Matcher; -import org.hamcrest.Matchers; import org.hamcrest.collection.IsMapContaining; -import org.json.JSONArray; import org.json.JSONObject; import org.junit.Assert; import org.junit.Ignore; import org.junit.Test; import java.io.IOException; -import java.util.ArrayList; import java.util.Date; -import java.util.List; import java.util.stream.IntStream; import static com.amazon.opendistroforelasticsearch.sql.esintgtest.TestsConstants.TEST_INDEX_ACCOUNT; -import static com.amazon.opendistroforelasticsearch.sql.util.MatcherUtils.hit; import static com.amazon.opendistroforelasticsearch.sql.util.MatcherUtils.hitAny; import static com.amazon.opendistroforelasticsearch.sql.util.MatcherUtils.kvDouble; import static com.amazon.opendistroforelasticsearch.sql.util.MatcherUtils.kvString; @@ -54,7 +47,6 @@ import static org.hamcrest.Matchers.hasEntry; import static org.hamcrest.Matchers.hasItems; import static org.hamcrest.Matchers.hasValue; -import static org.hamcrest.Matchers.instanceOf; import static org.hamcrest.Matchers.is; import static org.hamcrest.Matchers.isEmptyOrNullString; import static org.hamcrest.Matchers.not; @@ -166,96 +158,156 @@ public void caseChangeWithAggregationTest() throws IOException { @Test public void castIntFieldToDoubleWithoutAliasTest() throws IOException { - String query = "SELECT CAST(age AS DOUBLE) FROM " + TestsConstants.TEST_INDEX_ACCOUNT + " LIMIT 5"; + String query = "SELECT CAST(age AS DOUBLE) FROM " + TestsConstants.TEST_INDEX_ACCOUNT + + " ORDER BY age DESC LIMIT 5"; - checkSuccessfulFieldCast(query, "cast_age", "DOUBLE"); + SearchHit[] hits = query(query).getHits(); + checkSuccessfulFieldCast(hits, "cast_age", "DOUBLE"); + for (int i = 0; i < hits.length; ++i) { + Assert.assertThat(hits[i].getFields().get("cast_age").getValue(), is(40.0)); + } } @Test public void castIntFieldToDoubleWithAliasTest() throws IOException { String query = "SELECT CAST(age AS DOUBLE) AS test_alias FROM " + TestsConstants.TEST_INDEX_ACCOUNT + - " LIMIT 5"; + " ORDER BY age LIMIT 5"; - checkSuccessfulFieldCast(query, "test_alias", "DOUBLE"); + SearchHit[] hits = query(query).getHits(); + checkSuccessfulFieldCast(hits, "test_alias", "DOUBLE"); + for (int i = 0; i < hits.length; ++i) { + Assert.assertThat(hits[i].getFields().get("test_alias").getValue(), is(20.0)); + } } @Test public void castIntFieldToStringWithoutAliasTest() throws IOException { - String query = "SELECT CAST(balance AS STRING) FROM " + TestsConstants.TEST_INDEX_ACCOUNT + " LIMIT 5"; + String query = "SELECT CAST(balance AS STRING) FROM " + TestsConstants.TEST_INDEX_ACCOUNT + + " ORDER BY balance LIMIT 1"; - checkSuccessfulFieldCast(query, "cast_balance", "STRING"); + SearchHit[] hits = query(query).getHits(); + checkSuccessfulFieldCast(hits, "cast_balance", "STRING"); + for (int i = 0; i < hits.length; ++i) { + Assert.assertThat(hits[i].getFields().get("cast_balance").getValue(), is("1011")); + } } @Test public void castIntFieldToStringWithAliasTest() throws IOException { String query = "SELECT CAST(balance AS STRING) AS cast_string_alias FROM " + TestsConstants.TEST_INDEX_ACCOUNT + - " LIMIT 5"; + " ORDER BY cast_string_alias DESC LIMIT 1"; - checkSuccessfulFieldCast(query, "cast_string_alias", "STRING"); + SearchHit[] hits = query(query).getHits(); + checkSuccessfulFieldCast(hits, "cast_string_alias", "STRING"); + for (int i = 0; i < hits.length; ++i) { + Assert.assertThat(hits[i].getFields().get("cast_string_alias").getValue(), is("9838")); + } } @Test public void castIntFieldToFloatWithoutAliasJdbcFormatTest() { JSONObject response = executeJdbcRequest( - "SELECT CAST(balance AS FLOAT) FROM " + TestsConstants.TEST_INDEX_ACCOUNT + " LIMIT 5"); + "SELECT CAST(balance AS FLOAT) FROM " + TestsConstants.TEST_INDEX_ACCOUNT + + " ORDER BY balance DESC LIMIT 1"); String float_type_cast = "{\"name\":\"cast_balance\",\"type\":\"float\"}"; assertEquals(response.getJSONArray("schema").get(0).toString(), float_type_cast); + Assert.assertThat( + response.getJSONArray("datarows") + .getJSONArray(0).getFloat(0), + equalTo(49989.0F)); } @Test public void castIntFieldToFloatWithAliasJdbcFormatTest() { JSONObject response = executeJdbcRequest( "SELECT CAST(balance AS FLOAT) AS jdbc_float_alias " + - "FROM " + TestsConstants.TEST_INDEX_ACCOUNT + " limit 5"); + "FROM " + TestsConstants.TEST_INDEX_ACCOUNT + " ORDER BY jdbc_float_alias LIMIT 1"); String float_type_cast = "{\"name\":\"jdbc_float_alias\",\"type\":\"float\"}"; assertEquals(response.getJSONArray("schema").get(0).toString(), float_type_cast); + Assert.assertThat( + response.getJSONArray("datarows") + .getJSONArray(0).getFloat(0), + equalTo(1011.0F)); + } @Test public void castIntFieldToDoubleWithoutAliasOrderByTest() throws IOException { - String query = "SELECT CAST(age AS DOUBLE) FROM " + TestsConstants.TEST_INDEX_ACCOUNT + " ORDER BY age LIMIT 5"; + String query = "SELECT CAST(age AS DOUBLE) FROM " + TestsConstants.TEST_INDEX_ACCOUNT + " ORDER BY age LIMIT 1"; - checkSuccessfulFieldCast(query, "cast_age", "DOUBLE"); + SearchHit[] hits = query(query).getHits(); + checkSuccessfulFieldCast(hits, "cast_age", "DOUBLE"); + for (int i = 0; i < hits.length; ++i) { + Assert.assertThat(hits[i].getFields().get("cast_age").getValue(), is(20.0)); + } } @Test public void castIntFieldToDoubleWithAliasOrderByTest() throws IOException { String query = "SELECT CAST(age AS DOUBLE) AS alias FROM " + TestsConstants.TEST_INDEX_ACCOUNT + - " ORDER BY alias LIMIT 5"; + " ORDER BY alias DESC LIMIT 1"; - checkSuccessfulFieldCast(query, "alias", "DOUBLE"); + SearchHit[] hits = query(query).getHits(); + checkSuccessfulFieldCast(hits, "alias", "DOUBLE"); + for (int i = 0; i < hits.length; ++i) { + Assert.assertThat(hits[i].getFields().get("alias").getValue(), is(40.0)); + } } @Test public void castIntFieldToFloatWithoutAliasJdbcFormatGroupByTest() { JSONObject response = executeJdbcRequest( - "SELECT CAST(balance AS FLOAT) AS jdbc_float_alias " + - "FROM " + TestsConstants.TEST_INDEX_ACCOUNT + " GROUP BY balance LIMIT 5"); + "SELECT CAST(balance AS FLOAT) FROM " + + TestsConstants.TEST_INDEX_ACCOUNT + " GROUP BY balance DESC LIMIT 5"); String float_type_cast = "{\"name\":\"balance\",\"type\":\"long\"}"; assertEquals(response.getJSONArray("schema").get(0).toString(), float_type_cast); + Float[] expectedOutput = new Float[] {22026.0F, 23285.0F, 36038.0F, 39063.0F, 45493.0F}; + + for (int i = 0; i < response.getJSONArray("datarows").length(); ++i) { + Assert.assertThat( + response.getJSONArray("datarows") + .getJSONArray(i).getFloat(0), + equalTo(expectedOutput[i])); + } } @Test public void castIntFieldToFloatWithAliasJdbcFormatGroupByTest() { JSONObject response = executeJdbcRequest( "SELECT CAST(balance AS FLOAT) AS jdbc_float_alias " + - "FROM " + TestsConstants.TEST_INDEX_ACCOUNT + " GROUP BY jdbc_float_alias LIMIT 5"); + "FROM " + TestsConstants.TEST_INDEX_ACCOUNT + " GROUP BY jdbc_float_alias ASC LIMIT 5"); String float_type_cast = "{\"name\":\"jdbc_float_alias\",\"type\":\"float\"}"; assertEquals(response.getJSONArray("schema").get(0).toString(), float_type_cast); + Float[] expectedOutput = new Float[] {22026.0F, 23285.0F, 36038.0F, 39063.0F, 45493.0F}; + + for (int i = 0; i < response.getJSONArray("datarows").length(); ++i) { + Assert.assertThat( + response.getJSONArray("datarows") + .getJSONArray(i).getFloat(0), + equalTo(expectedOutput[i])); + } } @Test public void castIntFieldToDoubleWithAliasJdbcFormatGroupByTest() { JSONObject response = executeJdbcRequest( - "SELECT CAST(balance AS DOUBLE) AS jdbc_double_alias " + - "FROM " + TestsConstants.TEST_INDEX_ACCOUNT + " GROUP BY jdbc_double_alias LIMIT 5"); + "SELECT CAST(age AS DOUBLE) AS jdbc_double_alias " + + "FROM " + TestsConstants.TEST_INDEX_ACCOUNT + " GROUP BY jdbc_double_alias DESC LIMIT 5"); String float_type_cast = "{\"name\":\"jdbc_double_alias\",\"type\":\"double\"}"; assertEquals(response.getJSONArray("schema").get(0).toString(), float_type_cast); + Double[] expectedOutput = new Double[] {31.0, 39.0, 26.0, 32.0, 35.0}; + + for (int i = 0; i < response.getJSONArray("datarows").length(); ++i) { + Assert.assertThat( + response.getJSONArray("datarows") + .getJSONArray(i).getDouble(0), + equalTo(expectedOutput[i])); + } } @Test @@ -402,8 +454,7 @@ private SearchHits query(String query) throws IOException { return SearchResponse.fromXContent(parser).getHits(); } - private void checkSuccessfulFieldCast(String query, String field, String castType) throws IOException { - SearchHit[] hits = query(query).getHits(); + private void checkSuccessfulFieldCast(SearchHit[] hits, String field, String castType) { for (int i = 0; i < hits.length; ++i) { Assert.assertThat(hits[i].getFields(), IsMapContaining.hasKey(field)); switch (castType) { From bfa1fda2c6781722894b4562fb0e37760fb59f19 Mon Sep 17 00:00:00 2001 From: David Cui Date: Thu, 31 Oct 2019 16:49:06 -0700 Subject: [PATCH 15/15] added uppercase() to support case-sensitivity for the casted field (e.g float vs FLOAT) --- .../opendistroforelasticsearch/sql/utils/SQLFunctions.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/main/java/com/amazon/opendistroforelasticsearch/sql/utils/SQLFunctions.java b/src/main/java/com/amazon/opendistroforelasticsearch/sql/utils/SQLFunctions.java index a335586190..560aa9f993 100644 --- a/src/main/java/com/amazon/opendistroforelasticsearch/sql/utils/SQLFunctions.java +++ b/src/main/java/com/amazon/opendistroforelasticsearch/sql/utils/SQLFunctions.java @@ -616,7 +616,7 @@ public static Schema.Type getScriptFunctionReturnType(String functionName) { } public static Schema.Type getCastFunctionReturnType(String castType) { - switch (castType) { + switch (StringUtils.toUpper(castType)) { case "FLOAT": return Schema.Type.FLOAT; case "DOUBLE":