From 70152eb653c2ea2e09428d602af68e55e63af290 Mon Sep 17 00:00:00 2001 From: Kenrick Yap <14yapkc1@gmail.com> Date: Fri, 3 Jan 2025 10:43:05 -0800 Subject: [PATCH 01/21] added implementation Signed-off-by: Kenrick Yap <14yapkc1@gmail.com> --- .../org/opensearch/sql/expression/DSL.java | 4 ++ .../function/BuiltinFunctionName.java | 3 ++ .../function/BuiltinFunctionRepository.java | 2 + .../sql/expression/json/JsonFunctions.java | 46 +++++++++++++++++++ ppl/src/main/antlr/OpenSearchPPLLexer.g4 | 3 ++ ppl/src/main/antlr/OpenSearchPPLParser.g4 | 1 + 6 files changed, 59 insertions(+) create mode 100644 core/src/main/java/org/opensearch/sql/expression/json/JsonFunctions.java diff --git a/core/src/main/java/org/opensearch/sql/expression/DSL.java b/core/src/main/java/org/opensearch/sql/expression/DSL.java index 44ecc2bc867..fc97f35d117 100644 --- a/core/src/main/java/org/opensearch/sql/expression/DSL.java +++ b/core/src/main/java/org/opensearch/sql/expression/DSL.java @@ -683,6 +683,10 @@ public static FunctionExpression notLike(Expression... expressions) { return compile(FunctionProperties.None, BuiltinFunctionName.NOT_LIKE, expressions); } + public static FunctionExpression jsonValid(Expression... expressions){ + return compile(FunctionProperties.None, BuiltinFunctionName.JSON_VALID, expressions); + } + public static Aggregator avg(Expression... expressions) { return aggregate(BuiltinFunctionName.AVG, expressions); } diff --git a/core/src/main/java/org/opensearch/sql/expression/function/BuiltinFunctionName.java b/core/src/main/java/org/opensearch/sql/expression/function/BuiltinFunctionName.java index f8e9cf7c5f7..43fdbf2eb75 100644 --- a/core/src/main/java/org/opensearch/sql/expression/function/BuiltinFunctionName.java +++ b/core/src/main/java/org/opensearch/sql/expression/function/BuiltinFunctionName.java @@ -204,6 +204,9 @@ public enum BuiltinFunctionName { TRIM(FunctionName.of("trim")), UPPER(FunctionName.of("upper")), + /** Json Functions. */ + JSON_VALID(FunctionName.of("json_valid")), + /** NULL Test. */ IS_NULL(FunctionName.of("is null")), IS_NOT_NULL(FunctionName.of("is not null")), diff --git a/core/src/main/java/org/opensearch/sql/expression/function/BuiltinFunctionRepository.java b/core/src/main/java/org/opensearch/sql/expression/function/BuiltinFunctionRepository.java index 79ea58b8608..72d637fd2ba 100644 --- a/core/src/main/java/org/opensearch/sql/expression/function/BuiltinFunctionRepository.java +++ b/core/src/main/java/org/opensearch/sql/expression/function/BuiltinFunctionRepository.java @@ -28,6 +28,7 @@ import org.opensearch.sql.expression.datetime.DateTimeFunctions; import org.opensearch.sql.expression.datetime.IntervalClause; import org.opensearch.sql.expression.ip.IPFunctions; +import org.opensearch.sql.expression.json.JsonFunctions; import org.opensearch.sql.expression.operator.arthmetic.ArithmeticFunctions; import org.opensearch.sql.expression.operator.arthmetic.MathematicalFunctions; import org.opensearch.sql.expression.operator.convert.TypeCastOperators; @@ -83,6 +84,7 @@ public static synchronized BuiltinFunctionRepository getInstance() { SystemFunctions.register(instance); OpenSearchFunctions.register(instance); IPFunctions.register(instance); + JsonFunctions.register(instance); } return instance; } diff --git a/core/src/main/java/org/opensearch/sql/expression/json/JsonFunctions.java b/core/src/main/java/org/opensearch/sql/expression/json/JsonFunctions.java new file mode 100644 index 00000000000..e4ba7af4998 --- /dev/null +++ b/core/src/main/java/org/opensearch/sql/expression/json/JsonFunctions.java @@ -0,0 +1,46 @@ +package org.opensearch.sql.expression.json; + +import com.fasterxml.jackson.databind.ObjectMapper; +import lombok.experimental.UtilityClass; +import org.opensearch.sql.data.model.ExprValue; +import org.opensearch.sql.data.model.ExprValueUtils; +import org.opensearch.sql.exception.SemanticCheckException; +import org.opensearch.sql.expression.function.BuiltinFunctionName; +import org.opensearch.sql.expression.function.BuiltinFunctionRepository; +import org.opensearch.sql.expression.function.DefaultFunctionResolver; + +import static org.opensearch.sql.data.type.ExprCoreType.BOOLEAN; +import static org.opensearch.sql.data.type.ExprCoreType.STRING; +import static org.opensearch.sql.expression.function.FunctionDSL.define; +import static org.opensearch.sql.expression.function.FunctionDSL.impl; +import static org.opensearch.sql.expression.function.FunctionDSL.nullMissingHandling; + +@UtilityClass +public class JsonFunctions { + public void register(BuiltinFunctionRepository repository) { + + repository.register(jsonValid()); + } + + private DefaultFunctionResolver jsonValid() { + return define( + BuiltinFunctionName.JSON_VALID.getName(), + impl(nullMissingHandling(JsonFunctions::isValidJson), BOOLEAN, STRING)); + } + + /** + * Checks if given JSON string can be parsed as valid JSON. + * + * @param jsonExprValue JSON string (e.g. "198.51.100.14" or "2001:0db8::ff00:42:8329"). + * @return true if the string can be parsed as valid JSON, else false. + */ + private ExprValue isValidJson(ExprValue jsonExprValue) { + ObjectMapper objectMapper = new ObjectMapper(); + try { + objectMapper.readTree(jsonExprValue.stringValue()); + return ExprValueUtils.LITERAL_TRUE; + } catch (Exception e) { + return ExprValueUtils.LITERAL_FALSE; + } + } +} diff --git a/ppl/src/main/antlr/OpenSearchPPLLexer.g4 b/ppl/src/main/antlr/OpenSearchPPLLexer.g4 index 053ec530db4..5b6b9e41b89 100644 --- a/ppl/src/main/antlr/OpenSearchPPLLexer.g4 +++ b/ppl/src/main/antlr/OpenSearchPPLLexer.g4 @@ -332,6 +332,9 @@ ISNULL: 'ISNULL'; ISNOTNULL: 'ISNOTNULL'; CIDRMATCH: 'CIDRMATCH'; +// JSON FUNCTIONS +JSON_VALID: 'JSON_VALID'; + // FLOWCONTROL FUNCTIONS IFNULL: 'IFNULL'; NULLIF: 'NULLIF'; diff --git a/ppl/src/main/antlr/OpenSearchPPLParser.g4 b/ppl/src/main/antlr/OpenSearchPPLParser.g4 index 27f7e4014ba..999c5d9c87c 100644 --- a/ppl/src/main/antlr/OpenSearchPPLParser.g4 +++ b/ppl/src/main/antlr/OpenSearchPPLParser.g4 @@ -662,6 +662,7 @@ conditionFunctionName | ISNULL | ISNOTNULL | CIDRMATCH + | JSON_VALID ; // flow control function return non-boolean value From 76c3995ddd8765ce26672dd4948abf3f0270ba4b Mon Sep 17 00:00:00 2001 From: Kenrick Yap <14yapkc1@gmail.com> Date: Mon, 6 Jan 2025 15:14:25 -0800 Subject: [PATCH 02/21] added doctest, integ-tests, and unit tests Signed-off-by: Kenrick Yap <14yapkc1@gmail.com> --- .../expression/json/JsonFunctionsTest.java | 59 +++++++++++++++++ docs/category.json | 1 + docs/user/dql/metadata.rst | 3 +- docs/user/ppl/functions/json.rst | 34 ++++++++++ doctest/test_data/json_test.json | 5 ++ doctest/test_docs.py | 4 +- .../sql/legacy/SQLIntegTestCase.java | 8 ++- .../org/opensearch/sql/legacy/TestUtils.java | 5 ++ .../opensearch/sql/legacy/TestsConstants.java | 1 + .../opensearch/sql/ppl/JsonFunctionIT.java | 65 +++++++++++++++++++ .../json_test_index_mappping.json | 12 ++++ integ-test/src/test/resources/json_test.json | 10 +++ 12 files changed, 204 insertions(+), 3 deletions(-) create mode 100644 core/src/test/java/org/opensearch/sql/expression/json/JsonFunctionsTest.java create mode 100644 docs/user/ppl/functions/json.rst create mode 100644 doctest/test_data/json_test.json create mode 100644 integ-test/src/test/java/org/opensearch/sql/ppl/JsonFunctionIT.java create mode 100644 integ-test/src/test/resources/indexDefinitions/json_test_index_mappping.json create mode 100644 integ-test/src/test/resources/json_test.json diff --git a/core/src/test/java/org/opensearch/sql/expression/json/JsonFunctionsTest.java b/core/src/test/java/org/opensearch/sql/expression/json/JsonFunctionsTest.java new file mode 100644 index 00000000000..ee817dc71a8 --- /dev/null +++ b/core/src/test/java/org/opensearch/sql/expression/json/JsonFunctionsTest.java @@ -0,0 +1,59 @@ +/* + * Copyright OpenSearch Contributors + * SPDX-License-Identifier: Apache-2.0 + */ + +package org.opensearch.sql.expression.json; + +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.mockito.Mockito.when; +import static org.opensearch.sql.data.model.ExprValueUtils.LITERAL_FALSE; +import static org.opensearch.sql.data.model.ExprValueUtils.LITERAL_TRUE; +import static org.opensearch.sql.data.type.ExprCoreType.STRING; + +import org.junit.jupiter.api.Test; +import org.junit.jupiter.api.extension.ExtendWith; +import org.mockito.Mock; +import org.mockito.junit.jupiter.MockitoExtension; +import org.opensearch.sql.data.model.ExprValue; +import org.opensearch.sql.data.model.ExprValueUtils; +import org.opensearch.sql.expression.DSL; +import org.opensearch.sql.expression.Expression; +import org.opensearch.sql.expression.FunctionExpression; +import org.opensearch.sql.expression.env.Environment; + +@ExtendWith(MockitoExtension.class) +public class JsonFunctionsTest { + + private static final ExprValue JsonObject = ExprValueUtils.stringValue("{\"a\":\"1\",\"b\":\"2\"}"); + private static final ExprValue JsonArray = ExprValueUtils.stringValue("[1, 2, 3, 4]"); + private static final ExprValue JsonScalarString = ExprValueUtils.stringValue("\"abc\""); + private static final ExprValue JsonEmptyString = ExprValueUtils.stringValue(""); + private static final ExprValue JsonInvalidObject = ExprValueUtils.stringValue("{\"invalid\":\"json\", \"string\"}"); + private static final ExprValue JsonInvalidScalar = ExprValueUtils.stringValue("abc"); + + @Mock private Environment env; + + @Test + public void json_valid_invalid_json_string() { + assertEquals(LITERAL_FALSE, execute(JsonInvalidObject)); + assertEquals(LITERAL_FALSE, execute(JsonInvalidScalar)); + } + + @Test + public void json_valid_valid_json_string() { + assertEquals(LITERAL_TRUE, JsonObject); + assertEquals(LITERAL_TRUE, JsonArray); + assertEquals(LITERAL_TRUE, JsonScalarString); + assertEquals(LITERAL_TRUE, JsonEmptyString); + } + + private ExprValue execute(ExprValue jsonString) { + final String fieldName = "json_string"; + FunctionExpression exp = DSL.jsonValid(DSL.literal(jsonString)); + + when(DSL.ref(fieldName, STRING).valueOf(env)).thenReturn(jsonString); + + return exp.valueOf(env); + } +} diff --git a/docs/category.json b/docs/category.json index 32f56cfb467..efbb57d6e60 100644 --- a/docs/category.json +++ b/docs/category.json @@ -34,6 +34,7 @@ "user/ppl/functions/datetime.rst", "user/ppl/functions/expressions.rst", "user/ppl/functions/ip.rst", + "user/ppl/functions/json.rst", "user/ppl/functions/math.rst", "user/ppl/functions/relevance.rst", "user/ppl/functions/string.rst" diff --git a/docs/user/dql/metadata.rst b/docs/user/dql/metadata.rst index aba4eb0c75f..b059c0cded9 100644 --- a/docs/user/dql/metadata.rst +++ b/docs/user/dql/metadata.rst @@ -35,7 +35,7 @@ Example 1: Show All Indices Information SQL query:: os> SHOW TABLES LIKE '%' - fetched rows / total rows = 10/10 + fetched rows / total rows = 11/11 +----------------+-------------+-----------------+------------+---------+----------+------------+-----------+---------------------------+----------------+ | TABLE_CAT | TABLE_SCHEM | TABLE_NAME | TABLE_TYPE | REMARKS | TYPE_CAT | TYPE_SCHEM | TYPE_NAME | SELF_REFERENCING_COL_NAME | REF_GENERATION | |----------------+-------------+-----------------+------------+---------+----------+------------+-----------+---------------------------+----------------| @@ -44,6 +44,7 @@ SQL query:: | docTestCluster | null | accounts | BASE TABLE | null | null | null | null | null | null | | docTestCluster | null | apache | BASE TABLE | null | null | null | null | null | null | | docTestCluster | null | books | BASE TABLE | null | null | null | null | null | null | + | docTestCluster | null | json_test | BASE TABLE | null | null | null | null | null | null | | docTestCluster | null | nested | BASE TABLE | null | null | null | null | null | null | | docTestCluster | null | nyc_taxi | BASE TABLE | null | null | null | null | null | null | | docTestCluster | null | people | BASE TABLE | null | null | null | null | null | null | diff --git a/docs/user/ppl/functions/json.rst b/docs/user/ppl/functions/json.rst new file mode 100644 index 00000000000..8f986ace6b9 --- /dev/null +++ b/docs/user/ppl/functions/json.rst @@ -0,0 +1,34 @@ +==================== +IP Address Functions +==================== + +.. rubric:: Table of contents + +.. contents:: + :local: + :depth: 1 + +JSON_VALID +---------- + +Description +>>>>>>>>>>> + +Usage: `json_valid(json_string)` checks if `json_string` is a valid STRING string. + +Argument type: STRING + +Return type: BOOLEAN + +Example:: + + > source=json_test | where json_valid(json_string) | fields test_name, json_string + fetched rows / total rows = 4/4 + +--------------------+--------------------+ + | test_name | json_string | + |--------------------|--------------------| + | json object | {"a":"1","b":"2"} | + | json array | [1, 2, 3, 4] | + | json scalar string | [1, 2, 3, 4] | + | json empty string | [1, 2, 3, 4] | + +--------------------+--------------------+ diff --git a/doctest/test_data/json_test.json b/doctest/test_data/json_test.json new file mode 100644 index 00000000000..2da491675e5 --- /dev/null +++ b/doctest/test_data/json_test.json @@ -0,0 +1,5 @@ +{"test_name":"json object", "json_string":"{\"a\":\"1\",\"b\":\"2\"}"} +{"test_name":"json array", "json_string":"[1, 2, 3, 4]"} +{"test_name":"json scalar string", "json_string":"\"abc\""} +{"test_name":"json empty string","json_string":""} +{"test_name":"json invalid object", "json_string":"{\"invalid\":\"json\", \"string\"}"} diff --git a/doctest/test_docs.py b/doctest/test_docs.py index 1d46766c6d6..906bbd65b54 100644 --- a/doctest/test_docs.py +++ b/doctest/test_docs.py @@ -30,6 +30,7 @@ NESTED = "nested" DATASOURCES = ".ql-datasources" WEBLOGS = "weblogs" +JSON_TEST = "json_test" class DocTestConnection(OpenSearchConnection): @@ -123,6 +124,7 @@ def set_up_test_indices(test): load_file("nested_objects.json", index_name=NESTED) load_file("datasources.json", index_name=DATASOURCES) load_file("weblogs.json", index_name=WEBLOGS) + load_file("json_test.json", index_name=JSON_TEST) def load_file(filename, index_name): @@ -151,7 +153,7 @@ def set_up(test): def tear_down(test): # drop leftover tables after each test - test_data_client.indices.delete(index=[ACCOUNTS, EMPLOYEES, PEOPLE, ACCOUNT2, NYC_TAXI, BOOKS, APACHE, WILDCARD, NESTED, WEBLOGS], ignore_unavailable=True) + test_data_client.indices.delete(index=[ACCOUNTS, EMPLOYEES, PEOPLE, ACCOUNT2, NYC_TAXI, BOOKS, APACHE, WILDCARD, NESTED, WEBLOGS, JSON_TEST], ignore_unavailable=True) docsuite = partial(doctest.DocFileSuite, diff --git a/integ-test/src/test/java/org/opensearch/sql/legacy/SQLIntegTestCase.java b/integ-test/src/test/java/org/opensearch/sql/legacy/SQLIntegTestCase.java index 1728be74e6c..d4f72137364 100644 --- a/integ-test/src/test/java/org/opensearch/sql/legacy/SQLIntegTestCase.java +++ b/integ-test/src/test/java/org/opensearch/sql/legacy/SQLIntegTestCase.java @@ -22,6 +22,7 @@ import static org.opensearch.sql.legacy.TestUtils.getGameOfThronesIndexMapping; import static org.opensearch.sql.legacy.TestUtils.getGeopointIndexMapping; import static org.opensearch.sql.legacy.TestUtils.getJoinTypeIndexMapping; +import static org.opensearch.sql.legacy.TestUtils.getJsonTestIndexMapping; import static org.opensearch.sql.legacy.TestUtils.getLocationIndexMapping; import static org.opensearch.sql.legacy.TestUtils.getMappingFile; import static org.opensearch.sql.legacy.TestUtils.getNestedSimpleIndexMapping; @@ -745,7 +746,12 @@ public enum Index { TestsConstants.TEST_INDEX_GEOPOINT, "dates", getGeopointIndexMapping(), - "src/test/resources/geopoints.json"); + "src/test/resources/geopoints.json"), + JSON_TEST( + TestsConstants.TEST_INDEX_JSON_TEST, + "json", + getJsonTestIndexMapping(), + "src/test/resources/json_test.json"); private final String name; private final String type; diff --git a/integ-test/src/test/java/org/opensearch/sql/legacy/TestUtils.java b/integ-test/src/test/java/org/opensearch/sql/legacy/TestUtils.java index 195dda0cbdd..610ad1366a9 100644 --- a/integ-test/src/test/java/org/opensearch/sql/legacy/TestUtils.java +++ b/integ-test/src/test/java/org/opensearch/sql/legacy/TestUtils.java @@ -250,6 +250,11 @@ public static String getGeopointIndexMapping() { return getMappingFile(mappingFile); } + public static String getJsonTestIndexMapping() { + String mappingFile = "json_test_index_mapping.json"; + return getMappingFile(mappingFile); + } + public static void loadBulk(Client client, String jsonPath, String defaultIndex) throws Exception { System.out.println(String.format("Loading file %s into opensearch cluster", jsonPath)); diff --git a/integ-test/src/test/java/org/opensearch/sql/legacy/TestsConstants.java b/integ-test/src/test/java/org/opensearch/sql/legacy/TestsConstants.java index 1e336f544e9..387054ac7e5 100644 --- a/integ-test/src/test/java/org/opensearch/sql/legacy/TestsConstants.java +++ b/integ-test/src/test/java/org/opensearch/sql/legacy/TestsConstants.java @@ -58,6 +58,7 @@ public class TestsConstants { public static final String TEST_INDEX_MULTI_NESTED_TYPE = TEST_INDEX + "_multi_nested"; public static final String TEST_INDEX_NESTED_WITH_NULLS = TEST_INDEX + "_nested_with_nulls"; public static final String TEST_INDEX_GEOPOINT = TEST_INDEX + "_geopoint"; + public static final String TEST_INDEX_JSON_TEST = TEST_INDEX + "_json_test"; public static final String DATASOURCES = ".ql-datasources"; public static final String DATE_FORMAT = "yyyy-MM-dd'T'HH:mm:ss.SSS'Z'"; diff --git a/integ-test/src/test/java/org/opensearch/sql/ppl/JsonFunctionIT.java b/integ-test/src/test/java/org/opensearch/sql/ppl/JsonFunctionIT.java new file mode 100644 index 00000000000..62e7868b41e --- /dev/null +++ b/integ-test/src/test/java/org/opensearch/sql/ppl/JsonFunctionIT.java @@ -0,0 +1,65 @@ +/* + * Copyright OpenSearch Contributors + * SPDX-License-Identifier: Apache-2.0 + */ + +package org.opensearch.sql.ppl; + +import org.junit.jupiter.api.Test; + +import java.io.IOException; +import org.json.JSONObject; + +import javax.json.Json; + +import static org.opensearch.sql.legacy.TestsConstants.TEST_INDEX_JSON_TEST; +import static org.opensearch.sql.util.MatcherUtils.rows; +import static org.opensearch.sql.util.MatcherUtils.schema; +import static org.opensearch.sql.util.MatcherUtils.verifyDataRows; +import static org.opensearch.sql.util.MatcherUtils.verifySchema; + +public class JsonFunctionIT extends PPLIntegTestCase { + @Override + public void init() throws IOException { + loadIndex(Index.JSON_TEST); + } + + @Test + public void test_json_valid() throws IOException { + JSONObject result; + + result = + executeQuery( + String.format( + "source=%s | where json_valid(json_string) | fields test_name", + TEST_INDEX_JSON_TEST + ) + ); + verifySchema(result, schema("test_name", null, "string")); + verifyDataRows( + result, + rows("json object"), + rows("json array"), + rows("json scalar string"), + rows("json empty string") + ); + } + + @Test + public void test_not_json_valid() throws IOException { + JSONObject result; + + result = + executeQuery( + String.format( + "source=%s | where not json_valid(json_string) | fields test_name", + TEST_INDEX_JSON_TEST + ) + ); + verifySchema(result, schema("test_name", null, "string")); + verifyDataRows( + result, + rows("json invalid object") + ); + } +} diff --git a/integ-test/src/test/resources/indexDefinitions/json_test_index_mappping.json b/integ-test/src/test/resources/indexDefinitions/json_test_index_mappping.json new file mode 100644 index 00000000000..b825254b111 --- /dev/null +++ b/integ-test/src/test/resources/indexDefinitions/json_test_index_mappping.json @@ -0,0 +1,12 @@ +{ + "mappings": { + "properties": { + "test_name": { + "type": "text" + }, + "json_string": { + "type": "text" + } + } + } +} diff --git a/integ-test/src/test/resources/json_test.json b/integ-test/src/test/resources/json_test.json new file mode 100644 index 00000000000..e198eb7c430 --- /dev/null +++ b/integ-test/src/test/resources/json_test.json @@ -0,0 +1,10 @@ +{"index":{"_id":"1"}} +{"test_name":"json object", "json_string":"{\"a\":\"1\",\"b\":\"2\"}"} +{"index":{"_id":"2"}} +{"test_name":"json array", "json_string":"[1, 2, 3, 4]"} +{"index":{"_id":"3"}} +{"test_name":"json scalar string", "json_string":"\"abc\""} +{"index":{"_id":"4"}} +{"test_name":"json empty string","json_string":""} +{"index":{"_id":"5"}} +{"test_name":"json invalid object", "json_string":"{\"invalid\":\"json\", \"string\"}"} From ce2c551351eaf58f3055cd280f54a7bb3f0ac95a Mon Sep 17 00:00:00 2001 From: Kenrick Yap Date: Mon, 6 Jan 2025 15:27:23 -0800 Subject: [PATCH 03/21] addressed PR comments Signed-off-by: Kenrick Yap --- .../org/opensearch/sql/expression/json/JsonFunctions.java | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/core/src/main/java/org/opensearch/sql/expression/json/JsonFunctions.java b/core/src/main/java/org/opensearch/sql/expression/json/JsonFunctions.java index e4ba7af4998..f0745af4b7d 100644 --- a/core/src/main/java/org/opensearch/sql/expression/json/JsonFunctions.java +++ b/core/src/main/java/org/opensearch/sql/expression/json/JsonFunctions.java @@ -1,3 +1,8 @@ +/* + * Copyright OpenSearch Contributors + * SPDX-License-Identifier: Apache-2.0 + */ + package org.opensearch.sql.expression.json; import com.fasterxml.jackson.databind.ObjectMapper; From ad1bde30c80a9eaf5305af1bb22c8d776a2cefbe Mon Sep 17 00:00:00 2001 From: Kenrick Yap Date: Mon, 6 Jan 2025 17:03:25 -0800 Subject: [PATCH 04/21] fixed unit tests Signed-off-by: Kenrick Yap --- .../sql/expression/json/JsonFunctions.java | 1 - .../expression/json/JsonFunctionsTest.java | 20 +++++-------------- docs/user/ppl/functions/json.rst | 19 +++++++++--------- 3 files changed, 15 insertions(+), 25 deletions(-) diff --git a/core/src/main/java/org/opensearch/sql/expression/json/JsonFunctions.java b/core/src/main/java/org/opensearch/sql/expression/json/JsonFunctions.java index f0745af4b7d..7ef90cf73c7 100644 --- a/core/src/main/java/org/opensearch/sql/expression/json/JsonFunctions.java +++ b/core/src/main/java/org/opensearch/sql/expression/json/JsonFunctions.java @@ -23,7 +23,6 @@ @UtilityClass public class JsonFunctions { public void register(BuiltinFunctionRepository repository) { - repository.register(jsonValid()); } diff --git a/core/src/test/java/org/opensearch/sql/expression/json/JsonFunctionsTest.java b/core/src/test/java/org/opensearch/sql/expression/json/JsonFunctionsTest.java index ee817dc71a8..56b63631677 100644 --- a/core/src/test/java/org/opensearch/sql/expression/json/JsonFunctionsTest.java +++ b/core/src/test/java/org/opensearch/sql/expression/json/JsonFunctionsTest.java @@ -13,18 +13,14 @@ import org.junit.jupiter.api.Test; import org.junit.jupiter.api.extension.ExtendWith; -import org.mockito.Mock; import org.mockito.junit.jupiter.MockitoExtension; import org.opensearch.sql.data.model.ExprValue; import org.opensearch.sql.data.model.ExprValueUtils; import org.opensearch.sql.expression.DSL; -import org.opensearch.sql.expression.Expression; import org.opensearch.sql.expression.FunctionExpression; -import org.opensearch.sql.expression.env.Environment; @ExtendWith(MockitoExtension.class) public class JsonFunctionsTest { - private static final ExprValue JsonObject = ExprValueUtils.stringValue("{\"a\":\"1\",\"b\":\"2\"}"); private static final ExprValue JsonArray = ExprValueUtils.stringValue("[1, 2, 3, 4]"); private static final ExprValue JsonScalarString = ExprValueUtils.stringValue("\"abc\""); @@ -32,8 +28,6 @@ public class JsonFunctionsTest { private static final ExprValue JsonInvalidObject = ExprValueUtils.stringValue("{\"invalid\":\"json\", \"string\"}"); private static final ExprValue JsonInvalidScalar = ExprValueUtils.stringValue("abc"); - @Mock private Environment env; - @Test public void json_valid_invalid_json_string() { assertEquals(LITERAL_FALSE, execute(JsonInvalidObject)); @@ -42,18 +36,14 @@ public void json_valid_invalid_json_string() { @Test public void json_valid_valid_json_string() { - assertEquals(LITERAL_TRUE, JsonObject); - assertEquals(LITERAL_TRUE, JsonArray); - assertEquals(LITERAL_TRUE, JsonScalarString); - assertEquals(LITERAL_TRUE, JsonEmptyString); + assertEquals(LITERAL_TRUE, execute(JsonObject)); + assertEquals(LITERAL_TRUE, execute(JsonArray)); + assertEquals(LITERAL_TRUE, execute(JsonScalarString)); + assertEquals(LITERAL_TRUE, execute(JsonEmptyString)); } private ExprValue execute(ExprValue jsonString) { - final String fieldName = "json_string"; FunctionExpression exp = DSL.jsonValid(DSL.literal(jsonString)); - - when(DSL.ref(fieldName, STRING).valueOf(env)).thenReturn(jsonString); - - return exp.valueOf(env); + return exp.valueOf(); } } diff --git a/docs/user/ppl/functions/json.rst b/docs/user/ppl/functions/json.rst index 8f986ace6b9..bf5bd46b7a3 100644 --- a/docs/user/ppl/functions/json.rst +++ b/docs/user/ppl/functions/json.rst @@ -22,13 +22,14 @@ Return type: BOOLEAN Example:: - > source=json_test | where json_valid(json_string) | fields test_name, json_string + > source=json_test | eval is_valid = json_valid(json_string) | fields test_name, json_string, is_valid fetched rows / total rows = 4/4 - +--------------------+--------------------+ - | test_name | json_string | - |--------------------|--------------------| - | json object | {"a":"1","b":"2"} | - | json array | [1, 2, 3, 4] | - | json scalar string | [1, 2, 3, 4] | - | json empty string | [1, 2, 3, 4] | - +--------------------+--------------------+ + +---------------------+------------------------------+----------+ + | test_name | json_string | is_valid | + |---------------------|------------------------------|----------| + | json object | {"a":"1","b":"2"} | True | + | json array | [1, 2, 3, 4] | True | + | json scalar string | "abc" | True | + | json empty string | | True | + | json invalid object | {"invalid":"json", "string"} | True | + +---------------------+------------------------------+----------+ From ccf47a277b420ccf2ec9ab94d23c05c33b0408d1 Mon Sep 17 00:00:00 2001 From: Kenrick Yap Date: Tue, 7 Jan 2025 09:20:59 -0800 Subject: [PATCH 05/21] addressed pr comments Signed-off-by: Kenrick Yap --- .../org/opensearch/sql/expression/json/JsonFunctions.java | 3 ++- .../org/opensearch/sql/expression/json/JsonFunctionsTest.java | 4 ++-- .../resources/indexDefinitions/json_test_index_mappping.json | 2 +- 3 files changed, 5 insertions(+), 4 deletions(-) diff --git a/core/src/main/java/org/opensearch/sql/expression/json/JsonFunctions.java b/core/src/main/java/org/opensearch/sql/expression/json/JsonFunctions.java index 7ef90cf73c7..6cdc14807ed 100644 --- a/core/src/main/java/org/opensearch/sql/expression/json/JsonFunctions.java +++ b/core/src/main/java/org/opensearch/sql/expression/json/JsonFunctions.java @@ -5,6 +5,7 @@ package org.opensearch.sql.expression.json; +import com.fasterxml.jackson.core.JsonProcessingException; import com.fasterxml.jackson.databind.ObjectMapper; import lombok.experimental.UtilityClass; import org.opensearch.sql.data.model.ExprValue; @@ -43,7 +44,7 @@ private ExprValue isValidJson(ExprValue jsonExprValue) { try { objectMapper.readTree(jsonExprValue.stringValue()); return ExprValueUtils.LITERAL_TRUE; - } catch (Exception e) { + } catch (JsonProcessingException e) { return ExprValueUtils.LITERAL_FALSE; } } diff --git a/core/src/test/java/org/opensearch/sql/expression/json/JsonFunctionsTest.java b/core/src/test/java/org/opensearch/sql/expression/json/JsonFunctionsTest.java index 56b63631677..5f2f51bcb9f 100644 --- a/core/src/test/java/org/opensearch/sql/expression/json/JsonFunctionsTest.java +++ b/core/src/test/java/org/opensearch/sql/expression/json/JsonFunctionsTest.java @@ -29,13 +29,13 @@ public class JsonFunctionsTest { private static final ExprValue JsonInvalidScalar = ExprValueUtils.stringValue("abc"); @Test - public void json_valid_invalid_json_string() { + public void json_valid_returns_false() { assertEquals(LITERAL_FALSE, execute(JsonInvalidObject)); assertEquals(LITERAL_FALSE, execute(JsonInvalidScalar)); } @Test - public void json_valid_valid_json_string() { + public void json_valid_returns_true() { assertEquals(LITERAL_TRUE, execute(JsonObject)); assertEquals(LITERAL_TRUE, execute(JsonArray)); assertEquals(LITERAL_TRUE, execute(JsonScalarString)); diff --git a/integ-test/src/test/resources/indexDefinitions/json_test_index_mappping.json b/integ-test/src/test/resources/indexDefinitions/json_test_index_mappping.json index b825254b111..fb97836d5ec 100644 --- a/integ-test/src/test/resources/indexDefinitions/json_test_index_mappping.json +++ b/integ-test/src/test/resources/indexDefinitions/json_test_index_mappping.json @@ -2,7 +2,7 @@ "mappings": { "properties": { "test_name": { - "type": "text" + "type": "keyword" }, "json_string": { "type": "text" From acc76a027792e7f66d6a6dd2c3c8771080fb01ff Mon Sep 17 00:00:00 2001 From: Kenrick Yap Date: Tue, 7 Jan 2025 13:40:28 -0800 Subject: [PATCH 06/21] addressed PR comments Signed-off-by: Kenrick Yap --- .../sql/expression/json/JsonFunctions.java | 19 ++------------ .../org/opensearch/sql/utils/JsonUtils.java | 26 +++++++++++++++++++ .../datetime/DateTimeFunctionTest.java | 4 +-- .../sql/expression/datetime/ExtractTest.java | 3 +++ .../sql/expression/datetime/YearweekTest.java | 5 ++-- 5 files changed, 36 insertions(+), 21 deletions(-) create mode 100644 core/src/main/java/org/opensearch/sql/utils/JsonUtils.java diff --git a/core/src/main/java/org/opensearch/sql/expression/json/JsonFunctions.java b/core/src/main/java/org/opensearch/sql/expression/json/JsonFunctions.java index 6cdc14807ed..4e8a3bba695 100644 --- a/core/src/main/java/org/opensearch/sql/expression/json/JsonFunctions.java +++ b/core/src/main/java/org/opensearch/sql/expression/json/JsonFunctions.java @@ -14,6 +14,7 @@ import org.opensearch.sql.expression.function.BuiltinFunctionName; import org.opensearch.sql.expression.function.BuiltinFunctionRepository; import org.opensearch.sql.expression.function.DefaultFunctionResolver; +import org.opensearch.sql.utils.JsonUtils; import static org.opensearch.sql.data.type.ExprCoreType.BOOLEAN; import static org.opensearch.sql.data.type.ExprCoreType.STRING; @@ -30,22 +31,6 @@ public void register(BuiltinFunctionRepository repository) { private DefaultFunctionResolver jsonValid() { return define( BuiltinFunctionName.JSON_VALID.getName(), - impl(nullMissingHandling(JsonFunctions::isValidJson), BOOLEAN, STRING)); - } - - /** - * Checks if given JSON string can be parsed as valid JSON. - * - * @param jsonExprValue JSON string (e.g. "198.51.100.14" or "2001:0db8::ff00:42:8329"). - * @return true if the string can be parsed as valid JSON, else false. - */ - private ExprValue isValidJson(ExprValue jsonExprValue) { - ObjectMapper objectMapper = new ObjectMapper(); - try { - objectMapper.readTree(jsonExprValue.stringValue()); - return ExprValueUtils.LITERAL_TRUE; - } catch (JsonProcessingException e) { - return ExprValueUtils.LITERAL_FALSE; - } + impl(nullMissingHandling(JsonUtils::isValidJson), BOOLEAN, STRING)); } } diff --git a/core/src/main/java/org/opensearch/sql/utils/JsonUtils.java b/core/src/main/java/org/opensearch/sql/utils/JsonUtils.java new file mode 100644 index 00000000000..393f83256a5 --- /dev/null +++ b/core/src/main/java/org/opensearch/sql/utils/JsonUtils.java @@ -0,0 +1,26 @@ +package org.opensearch.sql.utils; + +import com.fasterxml.jackson.core.JsonProcessingException; +import com.fasterxml.jackson.databind.ObjectMapper; +import lombok.experimental.UtilityClass; +import org.opensearch.sql.data.model.ExprValue; +import org.opensearch.sql.data.model.ExprValueUtils; + +@UtilityClass +public class JsonUtils { + /** + * Checks if given JSON string can be parsed as valid JSON. + * + * @param jsonExprValue JSON string (e.g. "198.51.100.14" or "2001:0db8::ff00:42:8329"). + * @return true if the string can be parsed as valid JSON, else false. + */ + public static ExprValue isValidJson(ExprValue jsonExprValue) { + ObjectMapper objectMapper = new ObjectMapper(); + try { + objectMapper.readTree(jsonExprValue.stringValue()); + return ExprValueUtils.LITERAL_TRUE; + } catch (JsonProcessingException e) { + return ExprValueUtils.LITERAL_FALSE; + } + } +} diff --git a/core/src/test/java/org/opensearch/sql/expression/datetime/DateTimeFunctionTest.java b/core/src/test/java/org/opensearch/sql/expression/datetime/DateTimeFunctionTest.java index c820c97196f..dbc75c45ae9 100644 --- a/core/src/test/java/org/opensearch/sql/expression/datetime/DateTimeFunctionTest.java +++ b/core/src/test/java/org/opensearch/sql/expression/datetime/DateTimeFunctionTest.java @@ -27,6 +27,7 @@ import java.util.List; import java.util.stream.Stream; import lombok.AllArgsConstructor; +import org.junit.jupiter.api.Disabled; import org.junit.jupiter.api.Test; import org.junit.jupiter.params.ParameterizedTest; import org.junit.jupiter.params.provider.Arguments; @@ -1228,9 +1229,8 @@ public void testWeekFormats( expectedInteger); } - // subtracting 1 as a temporary fix for year 2024. - // Issue: https://github.com/opensearch-project/sql/issues/2477 @Test + @Disabled("Test is disabled because of issue https://github.com/opensearch-project/sql/issues/2477") public void testWeekOfYearWithTimeType() { assertAll( () -> diff --git a/core/src/test/java/org/opensearch/sql/expression/datetime/ExtractTest.java b/core/src/test/java/org/opensearch/sql/expression/datetime/ExtractTest.java index 02d50d0b597..fd87c144daa 100644 --- a/core/src/test/java/org/opensearch/sql/expression/datetime/ExtractTest.java +++ b/core/src/test/java/org/opensearch/sql/expression/datetime/ExtractTest.java @@ -11,6 +11,8 @@ import java.time.LocalDate; import java.util.stream.Stream; + +import org.junit.jupiter.api.Disabled; import org.junit.jupiter.api.Test; import org.junit.jupiter.params.ParameterizedTest; import org.junit.jupiter.params.provider.Arguments; @@ -91,6 +93,7 @@ private void datePartWithTimeArgQuery(String part, String time, long expected) { } @Test + @Disabled("Test is disabled because of issue https://github.com/opensearch-project/sql/issues/2477") public void testExtractDatePartWithTimeType() { datePartWithTimeArgQuery( "DAY", timeInput, LocalDate.now(functionProperties.getQueryStartClock()).getDayOfMonth()); diff --git a/core/src/test/java/org/opensearch/sql/expression/datetime/YearweekTest.java b/core/src/test/java/org/opensearch/sql/expression/datetime/YearweekTest.java index 47225ac601e..87e2f05d85b 100644 --- a/core/src/test/java/org/opensearch/sql/expression/datetime/YearweekTest.java +++ b/core/src/test/java/org/opensearch/sql/expression/datetime/YearweekTest.java @@ -14,6 +14,8 @@ import java.time.LocalDate; import java.util.stream.Stream; + +import org.junit.jupiter.api.Disabled; import org.junit.jupiter.api.Test; import org.junit.jupiter.params.ParameterizedTest; import org.junit.jupiter.params.provider.Arguments; @@ -97,9 +99,8 @@ public void testYearweekWithoutMode() { assertEquals(eval(expression), eval(expressionWithoutMode)); } - // subtracting 1 as a temporary fix for year 2024. - // Issue: https://github.com/opensearch-project/sql/issues/2477 @Test + @Disabled("Test is disabled because of issue https://github.com/opensearch-project/sql/issues/2477") public void testYearweekWithTimeType() { int week = LocalDate.now(functionProperties.getQueryStartClock()).get(ALIGNED_WEEK_OF_YEAR) - 1; int year = LocalDate.now(functionProperties.getQueryStartClock()).getYear(); From 519c6f21aaa8c45dd22d525c414f220db64a704d Mon Sep 17 00:00:00 2001 From: Kenrick Yap Date: Tue, 7 Jan 2025 13:40:47 -0800 Subject: [PATCH 07/21] removed unused dependencies Signed-off-by: Kenrick Yap --- .../org/opensearch/sql/expression/json/JsonFunctions.java | 5 ----- 1 file changed, 5 deletions(-) diff --git a/core/src/main/java/org/opensearch/sql/expression/json/JsonFunctions.java b/core/src/main/java/org/opensearch/sql/expression/json/JsonFunctions.java index 4e8a3bba695..dc7bde32c23 100644 --- a/core/src/main/java/org/opensearch/sql/expression/json/JsonFunctions.java +++ b/core/src/main/java/org/opensearch/sql/expression/json/JsonFunctions.java @@ -5,12 +5,7 @@ package org.opensearch.sql.expression.json; -import com.fasterxml.jackson.core.JsonProcessingException; -import com.fasterxml.jackson.databind.ObjectMapper; import lombok.experimental.UtilityClass; -import org.opensearch.sql.data.model.ExprValue; -import org.opensearch.sql.data.model.ExprValueUtils; -import org.opensearch.sql.exception.SemanticCheckException; import org.opensearch.sql.expression.function.BuiltinFunctionName; import org.opensearch.sql.expression.function.BuiltinFunctionRepository; import org.opensearch.sql.expression.function.DefaultFunctionResolver; From 2e319fea1e26bcb1b324413cc2ebc9983eb11453 Mon Sep 17 00:00:00 2001 From: Kenrick Yap Date: Tue, 7 Jan 2025 14:28:45 -0800 Subject: [PATCH 08/21] linting Signed-off-by: Kenrick Yap --- .../org/opensearch/sql/expression/DSL.java | 2 +- .../sql/expression/json/JsonFunctions.java | 28 +++--- .../org/opensearch/sql/utils/JsonUtils.java | 28 +++--- .../datetime/DateTimeFunctionTest.java | 3 +- .../sql/expression/datetime/ExtractTest.java | 4 +- .../sql/expression/datetime/YearweekTest.java | 4 +- .../expression/json/JsonFunctionsTest.java | 48 +++++----- .../opensearch/sql/ppl/JsonFunctionIT.java | 89 ++++++++----------- 8 files changed, 98 insertions(+), 108 deletions(-) diff --git a/core/src/main/java/org/opensearch/sql/expression/DSL.java b/core/src/main/java/org/opensearch/sql/expression/DSL.java index fc97f35d117..dc819c8163d 100644 --- a/core/src/main/java/org/opensearch/sql/expression/DSL.java +++ b/core/src/main/java/org/opensearch/sql/expression/DSL.java @@ -683,7 +683,7 @@ public static FunctionExpression notLike(Expression... expressions) { return compile(FunctionProperties.None, BuiltinFunctionName.NOT_LIKE, expressions); } - public static FunctionExpression jsonValid(Expression... expressions){ + public static FunctionExpression jsonValid(Expression... expressions) { return compile(FunctionProperties.None, BuiltinFunctionName.JSON_VALID, expressions); } diff --git a/core/src/main/java/org/opensearch/sql/expression/json/JsonFunctions.java b/core/src/main/java/org/opensearch/sql/expression/json/JsonFunctions.java index dc7bde32c23..49541e5d591 100644 --- a/core/src/main/java/org/opensearch/sql/expression/json/JsonFunctions.java +++ b/core/src/main/java/org/opensearch/sql/expression/json/JsonFunctions.java @@ -5,27 +5,27 @@ package org.opensearch.sql.expression.json; -import lombok.experimental.UtilityClass; -import org.opensearch.sql.expression.function.BuiltinFunctionName; -import org.opensearch.sql.expression.function.BuiltinFunctionRepository; -import org.opensearch.sql.expression.function.DefaultFunctionResolver; -import org.opensearch.sql.utils.JsonUtils; - import static org.opensearch.sql.data.type.ExprCoreType.BOOLEAN; import static org.opensearch.sql.data.type.ExprCoreType.STRING; import static org.opensearch.sql.expression.function.FunctionDSL.define; import static org.opensearch.sql.expression.function.FunctionDSL.impl; import static org.opensearch.sql.expression.function.FunctionDSL.nullMissingHandling; +import lombok.experimental.UtilityClass; +import org.opensearch.sql.expression.function.BuiltinFunctionName; +import org.opensearch.sql.expression.function.BuiltinFunctionRepository; +import org.opensearch.sql.expression.function.DefaultFunctionResolver; +import org.opensearch.sql.utils.JsonUtils; + @UtilityClass public class JsonFunctions { - public void register(BuiltinFunctionRepository repository) { - repository.register(jsonValid()); - } + public void register(BuiltinFunctionRepository repository) { + repository.register(jsonValid()); + } - private DefaultFunctionResolver jsonValid() { - return define( - BuiltinFunctionName.JSON_VALID.getName(), - impl(nullMissingHandling(JsonUtils::isValidJson), BOOLEAN, STRING)); - } + private DefaultFunctionResolver jsonValid() { + return define( + BuiltinFunctionName.JSON_VALID.getName(), + impl(nullMissingHandling(JsonUtils::isValidJson), BOOLEAN, STRING)); + } } diff --git a/core/src/main/java/org/opensearch/sql/utils/JsonUtils.java b/core/src/main/java/org/opensearch/sql/utils/JsonUtils.java index 393f83256a5..af02d26ef75 100644 --- a/core/src/main/java/org/opensearch/sql/utils/JsonUtils.java +++ b/core/src/main/java/org/opensearch/sql/utils/JsonUtils.java @@ -8,19 +8,19 @@ @UtilityClass public class JsonUtils { - /** - * Checks if given JSON string can be parsed as valid JSON. - * - * @param jsonExprValue JSON string (e.g. "198.51.100.14" or "2001:0db8::ff00:42:8329"). - * @return true if the string can be parsed as valid JSON, else false. - */ - public static ExprValue isValidJson(ExprValue jsonExprValue) { - ObjectMapper objectMapper = new ObjectMapper(); - try { - objectMapper.readTree(jsonExprValue.stringValue()); - return ExprValueUtils.LITERAL_TRUE; - } catch (JsonProcessingException e) { - return ExprValueUtils.LITERAL_FALSE; - } + /** + * Checks if given JSON string can be parsed as valid JSON. + * + * @param jsonExprValue JSON string (e.g. "198.51.100.14" or "2001:0db8::ff00:42:8329"). + * @return true if the string can be parsed as valid JSON, else false. + */ + public static ExprValue isValidJson(ExprValue jsonExprValue) { + ObjectMapper objectMapper = new ObjectMapper(); + try { + objectMapper.readTree(jsonExprValue.stringValue()); + return ExprValueUtils.LITERAL_TRUE; + } catch (JsonProcessingException e) { + return ExprValueUtils.LITERAL_FALSE; } + } } diff --git a/core/src/test/java/org/opensearch/sql/expression/datetime/DateTimeFunctionTest.java b/core/src/test/java/org/opensearch/sql/expression/datetime/DateTimeFunctionTest.java index dbc75c45ae9..042491b2fc5 100644 --- a/core/src/test/java/org/opensearch/sql/expression/datetime/DateTimeFunctionTest.java +++ b/core/src/test/java/org/opensearch/sql/expression/datetime/DateTimeFunctionTest.java @@ -1230,7 +1230,8 @@ public void testWeekFormats( } @Test - @Disabled("Test is disabled because of issue https://github.com/opensearch-project/sql/issues/2477") + @Disabled( + "Test is disabled because of issue https://github.com/opensearch-project/sql/issues/2477") public void testWeekOfYearWithTimeType() { assertAll( () -> diff --git a/core/src/test/java/org/opensearch/sql/expression/datetime/ExtractTest.java b/core/src/test/java/org/opensearch/sql/expression/datetime/ExtractTest.java index fd87c144daa..feb809af57f 100644 --- a/core/src/test/java/org/opensearch/sql/expression/datetime/ExtractTest.java +++ b/core/src/test/java/org/opensearch/sql/expression/datetime/ExtractTest.java @@ -11,7 +11,6 @@ import java.time.LocalDate; import java.util.stream.Stream; - import org.junit.jupiter.api.Disabled; import org.junit.jupiter.api.Test; import org.junit.jupiter.params.ParameterizedTest; @@ -93,7 +92,8 @@ private void datePartWithTimeArgQuery(String part, String time, long expected) { } @Test - @Disabled("Test is disabled because of issue https://github.com/opensearch-project/sql/issues/2477") + @Disabled( + "Test is disabled because of issue https://github.com/opensearch-project/sql/issues/2477") public void testExtractDatePartWithTimeType() { datePartWithTimeArgQuery( "DAY", timeInput, LocalDate.now(functionProperties.getQueryStartClock()).getDayOfMonth()); diff --git a/core/src/test/java/org/opensearch/sql/expression/datetime/YearweekTest.java b/core/src/test/java/org/opensearch/sql/expression/datetime/YearweekTest.java index 87e2f05d85b..9e7b8c04c15 100644 --- a/core/src/test/java/org/opensearch/sql/expression/datetime/YearweekTest.java +++ b/core/src/test/java/org/opensearch/sql/expression/datetime/YearweekTest.java @@ -14,7 +14,6 @@ import java.time.LocalDate; import java.util.stream.Stream; - import org.junit.jupiter.api.Disabled; import org.junit.jupiter.api.Test; import org.junit.jupiter.params.ParameterizedTest; @@ -100,7 +99,8 @@ public void testYearweekWithoutMode() { } @Test - @Disabled("Test is disabled because of issue https://github.com/opensearch-project/sql/issues/2477") + @Disabled( + "Test is disabled because of issue https://github.com/opensearch-project/sql/issues/2477") public void testYearweekWithTimeType() { int week = LocalDate.now(functionProperties.getQueryStartClock()).get(ALIGNED_WEEK_OF_YEAR) - 1; int year = LocalDate.now(functionProperties.getQueryStartClock()).getYear(); diff --git a/core/src/test/java/org/opensearch/sql/expression/json/JsonFunctionsTest.java b/core/src/test/java/org/opensearch/sql/expression/json/JsonFunctionsTest.java index 5f2f51bcb9f..2e8ece2817c 100644 --- a/core/src/test/java/org/opensearch/sql/expression/json/JsonFunctionsTest.java +++ b/core/src/test/java/org/opensearch/sql/expression/json/JsonFunctionsTest.java @@ -6,10 +6,8 @@ package org.opensearch.sql.expression.json; import static org.junit.jupiter.api.Assertions.assertEquals; -import static org.mockito.Mockito.when; import static org.opensearch.sql.data.model.ExprValueUtils.LITERAL_FALSE; import static org.opensearch.sql.data.model.ExprValueUtils.LITERAL_TRUE; -import static org.opensearch.sql.data.type.ExprCoreType.STRING; import org.junit.jupiter.api.Test; import org.junit.jupiter.api.extension.ExtendWith; @@ -21,29 +19,31 @@ @ExtendWith(MockitoExtension.class) public class JsonFunctionsTest { - private static final ExprValue JsonObject = ExprValueUtils.stringValue("{\"a\":\"1\",\"b\":\"2\"}"); - private static final ExprValue JsonArray = ExprValueUtils.stringValue("[1, 2, 3, 4]"); - private static final ExprValue JsonScalarString = ExprValueUtils.stringValue("\"abc\""); - private static final ExprValue JsonEmptyString = ExprValueUtils.stringValue(""); - private static final ExprValue JsonInvalidObject = ExprValueUtils.stringValue("{\"invalid\":\"json\", \"string\"}"); - private static final ExprValue JsonInvalidScalar = ExprValueUtils.stringValue("abc"); + private static final ExprValue JsonObject = + ExprValueUtils.stringValue("{\"a\":\"1\",\"b\":\"2\"}"); + private static final ExprValue JsonArray = ExprValueUtils.stringValue("[1, 2, 3, 4]"); + private static final ExprValue JsonScalarString = ExprValueUtils.stringValue("\"abc\""); + private static final ExprValue JsonEmptyString = ExprValueUtils.stringValue(""); + private static final ExprValue JsonInvalidObject = + ExprValueUtils.stringValue("{\"invalid\":\"json\", \"string\"}"); + private static final ExprValue JsonInvalidScalar = ExprValueUtils.stringValue("abc"); - @Test - public void json_valid_returns_false() { - assertEquals(LITERAL_FALSE, execute(JsonInvalidObject)); - assertEquals(LITERAL_FALSE, execute(JsonInvalidScalar)); - } + @Test + public void json_valid_returns_false() { + assertEquals(LITERAL_FALSE, execute(JsonInvalidObject)); + assertEquals(LITERAL_FALSE, execute(JsonInvalidScalar)); + } - @Test - public void json_valid_returns_true() { - assertEquals(LITERAL_TRUE, execute(JsonObject)); - assertEquals(LITERAL_TRUE, execute(JsonArray)); - assertEquals(LITERAL_TRUE, execute(JsonScalarString)); - assertEquals(LITERAL_TRUE, execute(JsonEmptyString)); - } + @Test + public void json_valid_returns_true() { + assertEquals(LITERAL_TRUE, execute(JsonObject)); + assertEquals(LITERAL_TRUE, execute(JsonArray)); + assertEquals(LITERAL_TRUE, execute(JsonScalarString)); + assertEquals(LITERAL_TRUE, execute(JsonEmptyString)); + } - private ExprValue execute(ExprValue jsonString) { - FunctionExpression exp = DSL.jsonValid(DSL.literal(jsonString)); - return exp.valueOf(); - } + private ExprValue execute(ExprValue jsonString) { + FunctionExpression exp = DSL.jsonValid(DSL.literal(jsonString)); + return exp.valueOf(); + } } diff --git a/integ-test/src/test/java/org/opensearch/sql/ppl/JsonFunctionIT.java b/integ-test/src/test/java/org/opensearch/sql/ppl/JsonFunctionIT.java index 62e7868b41e..f02750147d6 100644 --- a/integ-test/src/test/java/org/opensearch/sql/ppl/JsonFunctionIT.java +++ b/integ-test/src/test/java/org/opensearch/sql/ppl/JsonFunctionIT.java @@ -5,61 +5,50 @@ package org.opensearch.sql.ppl; -import org.junit.jupiter.api.Test; - -import java.io.IOException; -import org.json.JSONObject; - -import javax.json.Json; - import static org.opensearch.sql.legacy.TestsConstants.TEST_INDEX_JSON_TEST; import static org.opensearch.sql.util.MatcherUtils.rows; import static org.opensearch.sql.util.MatcherUtils.schema; import static org.opensearch.sql.util.MatcherUtils.verifyDataRows; import static org.opensearch.sql.util.MatcherUtils.verifySchema; -public class JsonFunctionIT extends PPLIntegTestCase { - @Override - public void init() throws IOException { - loadIndex(Index.JSON_TEST); - } - - @Test - public void test_json_valid() throws IOException { - JSONObject result; - - result = - executeQuery( - String.format( - "source=%s | where json_valid(json_string) | fields test_name", - TEST_INDEX_JSON_TEST - ) - ); - verifySchema(result, schema("test_name", null, "string")); - verifyDataRows( - result, - rows("json object"), - rows("json array"), - rows("json scalar string"), - rows("json empty string") - ); - } - - @Test - public void test_not_json_valid() throws IOException { - JSONObject result; +import java.io.IOException; +import org.json.JSONObject; +import org.junit.jupiter.api.Test; - result = - executeQuery( - String.format( - "source=%s | where not json_valid(json_string) | fields test_name", - TEST_INDEX_JSON_TEST - ) - ); - verifySchema(result, schema("test_name", null, "string")); - verifyDataRows( - result, - rows("json invalid object") - ); - } +public class JsonFunctionIT extends PPLIntegTestCase { + @Override + public void init() throws IOException { + loadIndex(Index.JSON_TEST); + } + + @Test + public void test_json_valid() throws IOException { + JSONObject result; + + result = + executeQuery( + String.format( + "source=%s | where json_valid(json_string) | fields test_name", + TEST_INDEX_JSON_TEST)); + verifySchema(result, schema("test_name", null, "string")); + verifyDataRows( + result, + rows("json object"), + rows("json array"), + rows("json scalar string"), + rows("json empty string")); + } + + @Test + public void test_not_json_valid() throws IOException { + JSONObject result; + + result = + executeQuery( + String.format( + "source=%s | where not json_valid(json_string) | fields test_name", + TEST_INDEX_JSON_TEST)); + verifySchema(result, schema("test_name", null, "string")); + verifyDataRows(result, rows("json invalid object")); + } } From ee0820df0b2edb2e44753822831305e493e8c43a Mon Sep 17 00:00:00 2001 From: Kenrick Yap Date: Wed, 8 Jan 2025 14:16:31 -0800 Subject: [PATCH 09/21] addressed pr comment and rolling back disabled test case Signed-off-by: Kenrick Yap --- core/src/main/java/org/opensearch/sql/utils/JsonUtils.java | 2 +- .../sql/expression/datetime/DateTimeFunctionTest.java | 4 ++-- .../org/opensearch/sql/expression/datetime/ExtractTest.java | 2 -- .../org/opensearch/sql/expression/datetime/YearweekTest.java | 4 ++-- 4 files changed, 5 insertions(+), 7 deletions(-) diff --git a/core/src/main/java/org/opensearch/sql/utils/JsonUtils.java b/core/src/main/java/org/opensearch/sql/utils/JsonUtils.java index af02d26ef75..d7f37b41970 100644 --- a/core/src/main/java/org/opensearch/sql/utils/JsonUtils.java +++ b/core/src/main/java/org/opensearch/sql/utils/JsonUtils.java @@ -11,7 +11,7 @@ public class JsonUtils { /** * Checks if given JSON string can be parsed as valid JSON. * - * @param jsonExprValue JSON string (e.g. "198.51.100.14" or "2001:0db8::ff00:42:8329"). + * @param jsonExprValue JSON string (e.g. "{\"hello\": \"world\"}"). * @return true if the string can be parsed as valid JSON, else false. */ public static ExprValue isValidJson(ExprValue jsonExprValue) { diff --git a/core/src/test/java/org/opensearch/sql/expression/datetime/DateTimeFunctionTest.java b/core/src/test/java/org/opensearch/sql/expression/datetime/DateTimeFunctionTest.java index 042491b2fc5..4b287319ba5 100644 --- a/core/src/test/java/org/opensearch/sql/expression/datetime/DateTimeFunctionTest.java +++ b/core/src/test/java/org/opensearch/sql/expression/datetime/DateTimeFunctionTest.java @@ -1229,9 +1229,9 @@ public void testWeekFormats( expectedInteger); } + // subtracting 1 as a temporary fix for year 2024. + // Issue: https://github.com/opensearch-project/sql/issues/2477 @Test - @Disabled( - "Test is disabled because of issue https://github.com/opensearch-project/sql/issues/2477") public void testWeekOfYearWithTimeType() { assertAll( () -> diff --git a/core/src/test/java/org/opensearch/sql/expression/datetime/ExtractTest.java b/core/src/test/java/org/opensearch/sql/expression/datetime/ExtractTest.java index feb809af57f..a9ae6274b1a 100644 --- a/core/src/test/java/org/opensearch/sql/expression/datetime/ExtractTest.java +++ b/core/src/test/java/org/opensearch/sql/expression/datetime/ExtractTest.java @@ -92,8 +92,6 @@ private void datePartWithTimeArgQuery(String part, String time, long expected) { } @Test - @Disabled( - "Test is disabled because of issue https://github.com/opensearch-project/sql/issues/2477") public void testExtractDatePartWithTimeType() { datePartWithTimeArgQuery( "DAY", timeInput, LocalDate.now(functionProperties.getQueryStartClock()).getDayOfMonth()); diff --git a/core/src/test/java/org/opensearch/sql/expression/datetime/YearweekTest.java b/core/src/test/java/org/opensearch/sql/expression/datetime/YearweekTest.java index 9e7b8c04c15..e273ee9ed40 100644 --- a/core/src/test/java/org/opensearch/sql/expression/datetime/YearweekTest.java +++ b/core/src/test/java/org/opensearch/sql/expression/datetime/YearweekTest.java @@ -98,9 +98,9 @@ public void testYearweekWithoutMode() { assertEquals(eval(expression), eval(expressionWithoutMode)); } + // subtracting 1 as a temporary fix for year 2024. + // Issue: https://github.com/opensearch-project/sql/issues/2477 @Test - @Disabled( - "Test is disabled because of issue https://github.com/opensearch-project/sql/issues/2477") public void testYearweekWithTimeType() { int week = LocalDate.now(functionProperties.getQueryStartClock()).get(ALIGNED_WEEK_OF_YEAR) - 1; int year = LocalDate.now(functionProperties.getQueryStartClock()).getYear(); From 3407d4a4d92473b6c2be0154c325b49b6f26751a Mon Sep 17 00:00:00 2001 From: Kenrick Yap Date: Thu, 9 Jan 2025 10:58:21 -0800 Subject: [PATCH 10/21] removed disabled import Signed-off-by: Kenrick Yap --- .../opensearch/sql/expression/datetime/DateTimeFunctionTest.java | 1 - .../java/org/opensearch/sql/expression/datetime/ExtractTest.java | 1 - .../org/opensearch/sql/expression/datetime/YearweekTest.java | 1 - 3 files changed, 3 deletions(-) diff --git a/core/src/test/java/org/opensearch/sql/expression/datetime/DateTimeFunctionTest.java b/core/src/test/java/org/opensearch/sql/expression/datetime/DateTimeFunctionTest.java index 4b287319ba5..c820c97196f 100644 --- a/core/src/test/java/org/opensearch/sql/expression/datetime/DateTimeFunctionTest.java +++ b/core/src/test/java/org/opensearch/sql/expression/datetime/DateTimeFunctionTest.java @@ -27,7 +27,6 @@ import java.util.List; import java.util.stream.Stream; import lombok.AllArgsConstructor; -import org.junit.jupiter.api.Disabled; import org.junit.jupiter.api.Test; import org.junit.jupiter.params.ParameterizedTest; import org.junit.jupiter.params.provider.Arguments; diff --git a/core/src/test/java/org/opensearch/sql/expression/datetime/ExtractTest.java b/core/src/test/java/org/opensearch/sql/expression/datetime/ExtractTest.java index 1dfa3908e6d..d7635de6102 100644 --- a/core/src/test/java/org/opensearch/sql/expression/datetime/ExtractTest.java +++ b/core/src/test/java/org/opensearch/sql/expression/datetime/ExtractTest.java @@ -11,7 +11,6 @@ import java.time.LocalDate; import java.util.stream.Stream; -import org.junit.jupiter.api.Disabled; import org.junit.jupiter.api.Test; import org.junit.jupiter.params.ParameterizedTest; import org.junit.jupiter.params.provider.Arguments; diff --git a/core/src/test/java/org/opensearch/sql/expression/datetime/YearweekTest.java b/core/src/test/java/org/opensearch/sql/expression/datetime/YearweekTest.java index e273ee9ed40..47225ac601e 100644 --- a/core/src/test/java/org/opensearch/sql/expression/datetime/YearweekTest.java +++ b/core/src/test/java/org/opensearch/sql/expression/datetime/YearweekTest.java @@ -14,7 +14,6 @@ import java.time.LocalDate; import java.util.stream.Stream; -import org.junit.jupiter.api.Disabled; import org.junit.jupiter.api.Test; import org.junit.jupiter.params.ParameterizedTest; import org.junit.jupiter.params.provider.Arguments; From 7ef6cc95f9dec0515a1316efafb3b48181433358 Mon Sep 17 00:00:00 2001 From: kenrickyap <121634635+kenrickyap@users.noreply.github.com> Date: Thu, 9 Jan 2025 13:01:48 -0800 Subject: [PATCH 11/21] Update docs/user/ppl/functions/json.rst Co-authored-by: Andrew Carbonetto Signed-off-by: kenrickyap <121634635+kenrickyap@users.noreply.github.com> --- docs/user/ppl/functions/json.rst | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/user/ppl/functions/json.rst b/docs/user/ppl/functions/json.rst index bf5bd46b7a3..a69101300bb 100644 --- a/docs/user/ppl/functions/json.rst +++ b/docs/user/ppl/functions/json.rst @@ -31,5 +31,5 @@ Example:: | json array | [1, 2, 3, 4] | True | | json scalar string | "abc" | True | | json empty string | | True | - | json invalid object | {"invalid":"json", "string"} | True | + | json invalid object | {"invalid":"json", "string"} | False | +---------------------+------------------------------+----------+ From e5e90acc98d2b71209fc3d450eb91899d00cb058 Mon Sep 17 00:00:00 2001 From: kenrickyap <121634635+kenrickyap@users.noreply.github.com> Date: Thu, 9 Jan 2025 13:02:10 -0800 Subject: [PATCH 12/21] Update integ-test/src/test/java/org/opensearch/sql/ppl/JsonFunctionIT.java Co-authored-by: Andrew Carbonetto Signed-off-by: kenrickyap <121634635+kenrickyap@users.noreply.github.com> --- .../src/test/java/org/opensearch/sql/ppl/JsonFunctionIT.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/integ-test/src/test/java/org/opensearch/sql/ppl/JsonFunctionIT.java b/integ-test/src/test/java/org/opensearch/sql/ppl/JsonFunctionIT.java index f02750147d6..501ef9448e6 100644 --- a/integ-test/src/test/java/org/opensearch/sql/ppl/JsonFunctionIT.java +++ b/integ-test/src/test/java/org/opensearch/sql/ppl/JsonFunctionIT.java @@ -15,7 +15,7 @@ import org.json.JSONObject; import org.junit.jupiter.api.Test; -public class JsonFunctionIT extends PPLIntegTestCase { +public class JsonFunctionsIT extends PPLIntegTestCase { @Override public void init() throws IOException { loadIndex(Index.JSON_TEST); From 2187a5ac0f053b6815c1900459e4ba295ca7d060 Mon Sep 17 00:00:00 2001 From: Kenrick Yap Date: Thu, 9 Jan 2025 13:05:12 -0800 Subject: [PATCH 13/21] nit Signed-off-by: Kenrick Yap --- ...json_test_index_mappping.json => json_test_index_mapping.json} | 0 1 file changed, 0 insertions(+), 0 deletions(-) rename integ-test/src/test/resources/indexDefinitions/{json_test_index_mappping.json => json_test_index_mapping.json} (100%) diff --git a/integ-test/src/test/resources/indexDefinitions/json_test_index_mappping.json b/integ-test/src/test/resources/indexDefinitions/json_test_index_mapping.json similarity index 100% rename from integ-test/src/test/resources/indexDefinitions/json_test_index_mappping.json rename to integ-test/src/test/resources/indexDefinitions/json_test_index_mapping.json From 3512b335980f8f8fcbc6cfa10accb1b6d622f877 Mon Sep 17 00:00:00 2001 From: Kenrick Yap Date: Thu, 9 Jan 2025 13:16:09 -0800 Subject: [PATCH 14/21] fixed integ test Signed-off-by: Kenrick Yap --- .../sql/ppl/{JsonFunctionIT.java => JsonFunctionsIT.java} | 0 1 file changed, 0 insertions(+), 0 deletions(-) rename integ-test/src/test/java/org/opensearch/sql/ppl/{JsonFunctionIT.java => JsonFunctionsIT.java} (100%) diff --git a/integ-test/src/test/java/org/opensearch/sql/ppl/JsonFunctionIT.java b/integ-test/src/test/java/org/opensearch/sql/ppl/JsonFunctionsIT.java similarity index 100% rename from integ-test/src/test/java/org/opensearch/sql/ppl/JsonFunctionIT.java rename to integ-test/src/test/java/org/opensearch/sql/ppl/JsonFunctionsIT.java From 9fea6060ba12f95f174041439422787b59cdc3d4 Mon Sep 17 00:00:00 2001 From: Kenrick Yap Date: Thu, 9 Jan 2025 13:29:03 -0800 Subject: [PATCH 15/21] change text type to keyword Signed-off-by: Kenrick Yap --- .../resources/indexDefinitions/json_test_index_mapping.json | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/integ-test/src/test/resources/indexDefinitions/json_test_index_mapping.json b/integ-test/src/test/resources/indexDefinitions/json_test_index_mapping.json index fb97836d5ec..86bd0c6e948 100644 --- a/integ-test/src/test/resources/indexDefinitions/json_test_index_mapping.json +++ b/integ-test/src/test/resources/indexDefinitions/json_test_index_mapping.json @@ -5,7 +5,7 @@ "type": "keyword" }, "json_string": { - "type": "text" + "type": "keyword" } } } From fbc54bc59f1e54b89d14dff96b2578772f2647c5 Mon Sep 17 00:00:00 2001 From: Kenrick Yap Date: Fri, 10 Jan 2025 10:37:08 -0800 Subject: [PATCH 16/21] addressed PR comments Signed-off-by: Kenrick Yap --- .../expression/json/JsonFunctionsTest.java | 3 +++ docs/user/ppl/functions/json.rst | 21 ++++++++++--------- doctest/test_data/json_test.json | 1 + .../opensearch/sql/ppl/JsonFunctionsIT.java | 1 + integ-test/src/test/resources/json_test.json | 2 ++ 5 files changed, 18 insertions(+), 10 deletions(-) diff --git a/core/src/test/java/org/opensearch/sql/expression/json/JsonFunctionsTest.java b/core/src/test/java/org/opensearch/sql/expression/json/JsonFunctionsTest.java index 2e8ece2817c..e374841e7f6 100644 --- a/core/src/test/java/org/opensearch/sql/expression/json/JsonFunctionsTest.java +++ b/core/src/test/java/org/opensearch/sql/expression/json/JsonFunctionsTest.java @@ -19,6 +19,8 @@ @ExtendWith(MockitoExtension.class) public class JsonFunctionsTest { + private static final ExprValue JsonNestedObject = + ExprValueUtils.stringValue("{\"a\":\"1\",\"b\":{\"c\":\"2\",\"d\":\"3\"}}"); private static final ExprValue JsonObject = ExprValueUtils.stringValue("{\"a\":\"1\",\"b\":\"2\"}"); private static final ExprValue JsonArray = ExprValueUtils.stringValue("[1, 2, 3, 4]"); @@ -36,6 +38,7 @@ public void json_valid_returns_false() { @Test public void json_valid_returns_true() { + assertEquals(LITERAL_TRUE, execute(JsonNestedObject)); assertEquals(LITERAL_TRUE, execute(JsonObject)); assertEquals(LITERAL_TRUE, execute(JsonArray)); assertEquals(LITERAL_TRUE, execute(JsonScalarString)); diff --git a/docs/user/ppl/functions/json.rst b/docs/user/ppl/functions/json.rst index a69101300bb..ce3d1a4c76d 100644 --- a/docs/user/ppl/functions/json.rst +++ b/docs/user/ppl/functions/json.rst @@ -1,5 +1,5 @@ ==================== -IP Address Functions +JSON Functions ==================== .. rubric:: Table of contents @@ -24,12 +24,13 @@ Example:: > source=json_test | eval is_valid = json_valid(json_string) | fields test_name, json_string, is_valid fetched rows / total rows = 4/4 - +---------------------+------------------------------+----------+ - | test_name | json_string | is_valid | - |---------------------|------------------------------|----------| - | json object | {"a":"1","b":"2"} | True | - | json array | [1, 2, 3, 4] | True | - | json scalar string | "abc" | True | - | json empty string | | True | - | json invalid object | {"invalid":"json", "string"} | False | - +---------------------+------------------------------+----------+ + +---------------------+---------------------------------+----------+ + | test_name | json_string | is_valid | + |---------------------|---------------------------------|----------| + | json nested object | {"a":"1","b":{"c":"2","d":"3"}} | True | + | json object | {"a":"1","b":"2"} | True | + | json array | [1, 2, 3, 4] | True | + | json scalar string | "abc" | True | + | json empty string | | True | + | json invalid object | {"invalid":"json", "string"} | False | + +---------------------+---------------------------------+----------+ diff --git a/doctest/test_data/json_test.json b/doctest/test_data/json_test.json index 2da491675e5..7494fc4aa91 100644 --- a/doctest/test_data/json_test.json +++ b/doctest/test_data/json_test.json @@ -1,3 +1,4 @@ +{"test_name":"json nested object", "json_string":"{\"a\":\"1\",\"b\":{\"c\":\"2\",\"d\":\"3\"}}"} {"test_name":"json object", "json_string":"{\"a\":\"1\",\"b\":\"2\"}"} {"test_name":"json array", "json_string":"[1, 2, 3, 4]"} {"test_name":"json scalar string", "json_string":"\"abc\""} diff --git a/integ-test/src/test/java/org/opensearch/sql/ppl/JsonFunctionsIT.java b/integ-test/src/test/java/org/opensearch/sql/ppl/JsonFunctionsIT.java index 501ef9448e6..f852a97d48c 100644 --- a/integ-test/src/test/java/org/opensearch/sql/ppl/JsonFunctionsIT.java +++ b/integ-test/src/test/java/org/opensearch/sql/ppl/JsonFunctionsIT.java @@ -33,6 +33,7 @@ public void test_json_valid() throws IOException { verifySchema(result, schema("test_name", null, "string")); verifyDataRows( result, + rows("json nested object"), rows("json object"), rows("json array"), rows("json scalar string"), diff --git a/integ-test/src/test/resources/json_test.json b/integ-test/src/test/resources/json_test.json index e198eb7c430..badb4f4f6e8 100644 --- a/integ-test/src/test/resources/json_test.json +++ b/integ-test/src/test/resources/json_test.json @@ -1,3 +1,5 @@ +{"index":{"_id":"0"}} +{"test_name":"json nested object", "json_string":"{\"a\":\"1\",\"b\":{\"c\":\"2\",\"d\":\"3\"}}"} {"index":{"_id":"1"}} {"test_name":"json object", "json_string":"{\"a\":\"1\",\"b\":\"2\"}"} {"index":{"_id":"2"}} From 31ad2a4286c7a1d4876eb9e42eb4a41777b9c826 Mon Sep 17 00:00:00 2001 From: Kenrick Yap Date: Fri, 10 Jan 2025 16:07:52 -0800 Subject: [PATCH 17/21] fix doc-test Signed-off-by: Kenrick Yap --- docs/user/ppl/functions/json.rst | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/user/ppl/functions/json.rst b/docs/user/ppl/functions/json.rst index ce3d1a4c76d..74c173be139 100644 --- a/docs/user/ppl/functions/json.rst +++ b/docs/user/ppl/functions/json.rst @@ -23,7 +23,7 @@ Return type: BOOLEAN Example:: > source=json_test | eval is_valid = json_valid(json_string) | fields test_name, json_string, is_valid - fetched rows / total rows = 4/4 + fetched rows / total rows = 6/6 +---------------------+---------------------------------+----------+ | test_name | json_string | is_valid | |---------------------|---------------------------------|----------| From 2b2a8f3a4143ead02a96373b58e0482388066c21 Mon Sep 17 00:00:00 2001 From: Kenrick Yap Date: Tue, 14 Jan 2025 09:10:06 -0800 Subject: [PATCH 18/21] added null test Signed-off-by: Kenrick Yap --- core/src/main/java/org/opensearch/sql/utils/JsonUtils.java | 5 +++++ .../test/java/org/opensearch/sql/ppl/JsonFunctionsIT.java | 2 +- integ-test/src/test/resources/json_test.json | 2 ++ 3 files changed, 8 insertions(+), 1 deletion(-) diff --git a/core/src/main/java/org/opensearch/sql/utils/JsonUtils.java b/core/src/main/java/org/opensearch/sql/utils/JsonUtils.java index d7f37b41970..37c374286e2 100644 --- a/core/src/main/java/org/opensearch/sql/utils/JsonUtils.java +++ b/core/src/main/java/org/opensearch/sql/utils/JsonUtils.java @@ -16,6 +16,11 @@ public class JsonUtils { */ public static ExprValue isValidJson(ExprValue jsonExprValue) { ObjectMapper objectMapper = new ObjectMapper(); + + if (jsonExprValue.isNull() || jsonExprValue.isMissing()) { + return ExprValueUtils.LITERAL_FALSE; + } + try { objectMapper.readTree(jsonExprValue.stringValue()); return ExprValueUtils.LITERAL_TRUE; diff --git a/integ-test/src/test/java/org/opensearch/sql/ppl/JsonFunctionsIT.java b/integ-test/src/test/java/org/opensearch/sql/ppl/JsonFunctionsIT.java index f852a97d48c..9e5ac041fb6 100644 --- a/integ-test/src/test/java/org/opensearch/sql/ppl/JsonFunctionsIT.java +++ b/integ-test/src/test/java/org/opensearch/sql/ppl/JsonFunctionsIT.java @@ -50,6 +50,6 @@ public void test_not_json_valid() throws IOException { "source=%s | where not json_valid(json_string) | fields test_name", TEST_INDEX_JSON_TEST)); verifySchema(result, schema("test_name", null, "string")); - verifyDataRows(result, rows("json invalid object")); + verifyDataRows(result, rows("json invalid object"), rows("json null")); } } diff --git a/integ-test/src/test/resources/json_test.json b/integ-test/src/test/resources/json_test.json index badb4f4f6e8..e393bfeb8e4 100644 --- a/integ-test/src/test/resources/json_test.json +++ b/integ-test/src/test/resources/json_test.json @@ -10,3 +10,5 @@ {"test_name":"json empty string","json_string":""} {"index":{"_id":"5"}} {"test_name":"json invalid object", "json_string":"{\"invalid\":\"json\", \"string\"}"} +{"index":{"_id":"6"}} +{"test_name":"json null", "json_string":null} From 1913bfe755abf3bfe3607bcb8f3e421efd1d9a17 Mon Sep 17 00:00:00 2001 From: Andrew Carbonetto Date: Tue, 14 Jan 2025 22:24:54 -0800 Subject: [PATCH 19/21] SQL: adding error case unit tests for json_valid Signed-off-by: Andrew Carbonetto --- .../sql/expression/json/JsonFunctionsTest.java | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/core/src/test/java/org/opensearch/sql/expression/json/JsonFunctionsTest.java b/core/src/test/java/org/opensearch/sql/expression/json/JsonFunctionsTest.java index e374841e7f6..c889a237d41 100644 --- a/core/src/test/java/org/opensearch/sql/expression/json/JsonFunctionsTest.java +++ b/core/src/test/java/org/opensearch/sql/expression/json/JsonFunctionsTest.java @@ -6,7 +6,9 @@ package org.opensearch.sql.expression.json; import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertThrows; import static org.opensearch.sql.data.model.ExprValueUtils.LITERAL_FALSE; +import static org.opensearch.sql.data.model.ExprValueUtils.LITERAL_NULL; import static org.opensearch.sql.data.model.ExprValueUtils.LITERAL_TRUE; import org.junit.jupiter.api.Test; @@ -14,6 +16,7 @@ import org.mockito.junit.jupiter.MockitoExtension; import org.opensearch.sql.data.model.ExprValue; import org.opensearch.sql.data.model.ExprValueUtils; +import org.opensearch.sql.exception.ExpressionEvaluationException; import org.opensearch.sql.expression.DSL; import org.opensearch.sql.expression.FunctionExpression; @@ -34,6 +37,15 @@ public class JsonFunctionsTest { public void json_valid_returns_false() { assertEquals(LITERAL_FALSE, execute(JsonInvalidObject)); assertEquals(LITERAL_FALSE, execute(JsonInvalidScalar)); + + // caught by nullMissingHandling and returns null + assertEquals(LITERAL_NULL, execute(LITERAL_NULL)); + } + + @Test + public void json_valid_throws_ExpressionEvaluationException() { + assertThrows( + ExpressionEvaluationException.class, () -> execute(ExprValueUtils.booleanValue(true))); } @Test From 67d979d2f61f0e6b94d394014b82950bceb00f57 Mon Sep 17 00:00:00 2001 From: Andrew Carbonetto Date: Wed, 15 Jan 2025 09:12:53 -0800 Subject: [PATCH 20/21] json_valid: null and missing should return false Signed-off-by: Andrew Carbonetto --- .../org/opensearch/sql/expression/json/JsonFunctions.java | 4 +--- .../opensearch/sql/expression/json/JsonFunctionsTest.java | 6 +++--- 2 files changed, 4 insertions(+), 6 deletions(-) diff --git a/core/src/main/java/org/opensearch/sql/expression/json/JsonFunctions.java b/core/src/main/java/org/opensearch/sql/expression/json/JsonFunctions.java index 49541e5d591..acc0c4c064a 100644 --- a/core/src/main/java/org/opensearch/sql/expression/json/JsonFunctions.java +++ b/core/src/main/java/org/opensearch/sql/expression/json/JsonFunctions.java @@ -9,7 +9,6 @@ import static org.opensearch.sql.data.type.ExprCoreType.STRING; import static org.opensearch.sql.expression.function.FunctionDSL.define; import static org.opensearch.sql.expression.function.FunctionDSL.impl; -import static org.opensearch.sql.expression.function.FunctionDSL.nullMissingHandling; import lombok.experimental.UtilityClass; import org.opensearch.sql.expression.function.BuiltinFunctionName; @@ -25,7 +24,6 @@ public void register(BuiltinFunctionRepository repository) { private DefaultFunctionResolver jsonValid() { return define( - BuiltinFunctionName.JSON_VALID.getName(), - impl(nullMissingHandling(JsonUtils::isValidJson), BOOLEAN, STRING)); + BuiltinFunctionName.JSON_VALID.getName(), impl(JsonUtils::isValidJson, BOOLEAN, STRING)); } } diff --git a/core/src/test/java/org/opensearch/sql/expression/json/JsonFunctionsTest.java b/core/src/test/java/org/opensearch/sql/expression/json/JsonFunctionsTest.java index c889a237d41..3228a565c2e 100644 --- a/core/src/test/java/org/opensearch/sql/expression/json/JsonFunctionsTest.java +++ b/core/src/test/java/org/opensearch/sql/expression/json/JsonFunctionsTest.java @@ -8,6 +8,7 @@ import static org.junit.jupiter.api.Assertions.assertEquals; import static org.junit.jupiter.api.Assertions.assertThrows; import static org.opensearch.sql.data.model.ExprValueUtils.LITERAL_FALSE; +import static org.opensearch.sql.data.model.ExprValueUtils.LITERAL_MISSING; import static org.opensearch.sql.data.model.ExprValueUtils.LITERAL_NULL; import static org.opensearch.sql.data.model.ExprValueUtils.LITERAL_TRUE; @@ -37,9 +38,8 @@ public class JsonFunctionsTest { public void json_valid_returns_false() { assertEquals(LITERAL_FALSE, execute(JsonInvalidObject)); assertEquals(LITERAL_FALSE, execute(JsonInvalidScalar)); - - // caught by nullMissingHandling and returns null - assertEquals(LITERAL_NULL, execute(LITERAL_NULL)); + assertEquals(LITERAL_FALSE, execute(LITERAL_NULL)); + assertEquals(LITERAL_FALSE, execute(LITERAL_MISSING)); } @Test From b0d7d1a5fd853faaf6cca6a6b7a8e2a7a2bf0e46 Mon Sep 17 00:00:00 2001 From: Andrew Carbonetto Date: Wed, 15 Jan 2025 11:08:20 -0800 Subject: [PATCH 21/21] Fix json doc Signed-off-by: Andrew Carbonetto --- docs/user/ppl/functions/json.rst | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/user/ppl/functions/json.rst b/docs/user/ppl/functions/json.rst index 74c173be139..fa704b6c65f 100644 --- a/docs/user/ppl/functions/json.rst +++ b/docs/user/ppl/functions/json.rst @@ -14,7 +14,7 @@ JSON_VALID Description >>>>>>>>>>> -Usage: `json_valid(json_string)` checks if `json_string` is a valid STRING string. +Usage: `json_valid(json_string)` checks if `json_string` is a valid JSON-encoded string. Argument type: STRING