From 83bcc5c2181acdc3658d84abc049858b344e8662 Mon Sep 17 00:00:00 2001 From: Norman Jordan Date: Thu, 31 Oct 2024 13:41:40 -0700 Subject: [PATCH] Added some more tests Signed-off-by: Norman Jordan --- .../org/opensearch/sql/analysis/Analyzer.java | 2 +- .../org/opensearch/sql/ast/tree/FillNull.java | 1 + docs/category.json | 1 + docs/user/ppl/cmd/fillnull.rst | 18 ++-- .../org/opensearch/sql/ppl/ExplainIT.java | 3 +- .../opensearch/sql/ppl/FillNullCommandIT.java | 84 +++++++++++++++++++ .../ppl/explain_fillnull_push.json | 16 +--- ppl/src/main/antlr/OpenSearchPPLParser.g4 | 2 +- 8 files changed, 102 insertions(+), 25 deletions(-) diff --git a/core/src/main/java/org/opensearch/sql/analysis/Analyzer.java b/core/src/main/java/org/opensearch/sql/analysis/Analyzer.java index 2671daa6d3..71db736f78 100644 --- a/core/src/main/java/org/opensearch/sql/analysis/Analyzer.java +++ b/core/src/main/java/org/opensearch/sql/analysis/Analyzer.java @@ -559,7 +559,7 @@ public LogicalPlan visitAD(AD node, AnalysisContext context) { return new LogicalAD(child, options); } - /** Build {@link LogicalAD} for fillnull command. */ + /** Build {@link LogicalEval} for fillnull command. */ @Override public LogicalPlan visitFillNull(final FillNull node, final AnalysisContext context) { LogicalPlan child = node.getChild().get(0).accept(this, context); diff --git a/core/src/main/java/org/opensearch/sql/ast/tree/FillNull.java b/core/src/main/java/org/opensearch/sql/ast/tree/FillNull.java index 15ff2cac38..e1e56229b4 100644 --- a/core/src/main/java/org/opensearch/sql/ast/tree/FillNull.java +++ b/core/src/main/java/org/opensearch/sql/ast/tree/FillNull.java @@ -16,6 +16,7 @@ import org.opensearch.sql.ast.expression.Field; import org.opensearch.sql.ast.expression.UnresolvedExpression; +/** AST node represent FillNull operation. */ @RequiredArgsConstructor @AllArgsConstructor public class FillNull extends UnresolvedPlan { diff --git a/docs/category.json b/docs/category.json index e90c674a2e..c30bdc4882 100644 --- a/docs/category.json +++ b/docs/category.json @@ -14,6 +14,7 @@ "user/ppl/cmd/information_schema.rst", "user/ppl/cmd/eval.rst", "user/ppl/cmd/fields.rst", + "user/ppl/cmd/fillnull.rst", "user/ppl/cmd/grok.rst", "user/ppl/cmd/head.rst", "user/ppl/cmd/parse.rst", diff --git a/docs/user/ppl/cmd/fillnull.rst b/docs/user/ppl/cmd/fillnull.rst index 7dd8ea80c7..e6e022b30e 100644 --- a/docs/user/ppl/cmd/fillnull.rst +++ b/docs/user/ppl/cmd/fillnull.rst @@ -33,16 +33,16 @@ The example show to replace null values for email and host with "". PPL query:: - os> source=accounts | fields email, host | fillnull with '' email, host ; + os> source=accounts | fields email, employer | fillnull with '' in email ; fetched rows / total rows = 4/4 - +-----------------------+------------+ - | email | host | - |-----------------------+------------| - | amberduke@pyrami.com | pyrami.com | - | hattiebond@netagy.com | netagy.com | - | | | - | daleadams@boink.com | boink.com | - +-----------------------+------------+ + +-----------------------+----------+ + | email | employer | + |-----------------------+----------| + | amberduke@pyrami.com | Pyrami | + | hattiebond@netagy.com | Netagy | + | | Quility | + | daleadams@boink.com | null | + +-----------------------+----------+ Example 2: Replace null values for multiple fields with different values ======================================================================== diff --git a/integ-test/src/test/java/org/opensearch/sql/ppl/ExplainIT.java b/integ-test/src/test/java/org/opensearch/sql/ppl/ExplainIT.java index ee5e83db7f..b9c7f89ba0 100644 --- a/integ-test/src/test/java/org/opensearch/sql/ppl/ExplainIT.java +++ b/integ-test/src/test/java/org/opensearch/sql/ppl/ExplainIT.java @@ -97,8 +97,7 @@ public void testFillNullPushDownExplain() throws Exception { expected, explainQueryToString( "source=opensearch-sql_test_index_account" - + "| fields age, balance " - + "| fillnull with -1 in age,balance")); + + " | fillnull with -1 in age,balance | fields age, balance")); } String loadFromFile(String filename) throws Exception { diff --git a/integ-test/src/test/java/org/opensearch/sql/ppl/FillNullCommandIT.java b/integ-test/src/test/java/org/opensearch/sql/ppl/FillNullCommandIT.java index 5a1604ec05..d88d31c997 100644 --- a/integ-test/src/test/java/org/opensearch/sql/ppl/FillNullCommandIT.java +++ b/integ-test/src/test/java/org/opensearch/sql/ppl/FillNullCommandIT.java @@ -127,4 +127,88 @@ public void testFillNullVariousValuesTwoFields() throws IOException { rows(-1, 10.98), rows(-1, 7.87)); } + + @Test + public void testFillNullWithOtherField() throws IOException { + JSONObject result = + executeQuery( + String.format( + "source=%s | fillnull using num0 = num1 | fields str2, num0", TEST_INDEX_CALCS)); + verifyDataRows( + result, + rows("one", 12.3), + rows("two", -12.3), + rows("three", 15.7), + rows(null, -15.7), + rows("five", 3.5), + rows("six", -3.5), + rows(null, 0), + rows("eight", 11.38), + rows("nine", 10), + rows("ten", 12.4), + rows("eleven", 10.32), + rows("twelve", 2.47), + rows(null, 12.05), + rows("fourteen", 10.37), + rows("fifteen", 7.1), + rows("sixteen", 16.81), + rows(null, 7.12)); + } + + @Test + public void testFillNullWithFunctionOnOtherField() throws IOException { + JSONObject result = + executeQuery( + String.format( + "source=%s | fillnull with ceil(num1) in num0 | fields str2, num0", + TEST_INDEX_CALCS)); + verifyDataRows( + result, + rows("one", 12.3), + rows("two", -12.3), + rows("three", 15.7), + rows(null, -15.7), + rows("five", 3.5), + rows("six", -3.5), + rows(null, 0), + rows("eight", 12), + rows("nine", 10), + rows("ten", 13), + rows("eleven", 11), + rows("twelve", 3), + rows(null, 13), + rows("fourteen", 11), + rows("fifteen", 8), + rows("sixteen", 17), + rows(null, 8)); + } + + @Test + public void testFillNullWithFunctionMultipleCommands() throws IOException { + JSONObject result = + executeQuery( + String.format( + "source=%s | fillnull with num1 in num0 | fields str2, num0 | fillnull with" + + " 'unknown' in str2", + TEST_INDEX_CALCS)); + verifyDataRows( + result, + rows("one", 12.3), + rows("two", -12.3), + rows("three", 15.7), + rows("unknown", -15.7), + rows("five", 3.5), + rows("six", -3.5), + rows("unknown", 0), + rows("eight", 11.38), + rows("nine", 10), + rows("ten", 12.4), + rows("eleven", 10.32), + rows("twelve", 2.47), + rows("unknown", 12.05), + rows("fourteen", 10.37), + rows("fifteen", 7.1), + rows("sixteen", 16.81), + rows("unknown", 7.12)); + } } diff --git a/integ-test/src/test/resources/expectedOutput/ppl/explain_fillnull_push.json b/integ-test/src/test/resources/expectedOutput/ppl/explain_fillnull_push.json index e729e0dfbb..7e5e1c1c20 100644 --- a/integ-test/src/test/resources/expectedOutput/ppl/explain_fillnull_push.json +++ b/integ-test/src/test/resources/expectedOutput/ppl/explain_fillnull_push.json @@ -15,22 +15,14 @@ }, "children": [ { - "name": "ProjectOperator", + "name": "OpenSearchIndexScan", "description": { - "fields": "[age, balance]" + "request": "OpenSearchQueryRequest(indexName=opensearch-sql_test_index_account, sourceBuilder={\"from\":0,\"size\":10000,\"timeout\":\"1m\"}, needClean=true, searchDone=false, pitId=null, cursorKeepAlive=null, searchAfter=null, searchResponse=null)" }, - "children": [ - { - "name": "OpenSearchIndexScan", - "description": { - "request": "OpenSearchQueryRequest(indexName=opensearch-sql_test_index_account, sourceBuilder={\"from\":0,\"size\":10000,\"timeout\":\"1m\",\"_source\":{\"includes\":[\"age\",\"balance\"],\"excludes\":[]}}, needClean=true, searchDone=false, pitId=null, cursorKeepAlive=null, searchAfter=null, searchResponse=null)" - }, - "children": [] - } - ] + "children": [] } ] } ] } -} \ No newline at end of file +} diff --git a/ppl/src/main/antlr/OpenSearchPPLParser.g4 b/ppl/src/main/antlr/OpenSearchPPLParser.g4 index fa2d43e2a1..5eed9db26a 100644 --- a/ppl/src/main/antlr/OpenSearchPPLParser.g4 +++ b/ppl/src/main/antlr/OpenSearchPPLParser.g4 @@ -146,7 +146,7 @@ nullableField ; nullReplacement - : expression + : valueExpression ; kmeansCommand