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 c6196c0c05..f3239ae25a 100644 --- a/core/src/main/java/org/opensearch/sql/utils/JsonUtils.java +++ b/core/src/main/java/org/opensearch/sql/utils/JsonUtils.java @@ -11,14 +11,12 @@ import com.jayway.jsonpath.InvalidJsonException; import com.jayway.jsonpath.InvalidPathException; import com.jayway.jsonpath.JsonPath; - +import com.jayway.jsonpath.Option; +import com.jayway.jsonpath.PathNotFoundException; import java.util.LinkedHashMap; import java.util.LinkedList; import java.util.List; import java.util.Map; - -import com.jayway.jsonpath.Option; -import com.jayway.jsonpath.PathNotFoundException; import lombok.experimental.UtilityClass; import org.opensearch.sql.data.model.ExprCollectionValue; import org.opensearch.sql.data.model.ExprDoubleValue; @@ -67,9 +65,7 @@ public static ExprValue extractJson(ExprValue json, ExprValue path) { } try { - Configuration config = Configuration.builder() - .options(Option.AS_PATH_LIST) - .build(); + Configuration config = Configuration.builder().options(Option.AS_PATH_LIST).build(); List resultPaths = JsonPath.using(config).parse(jsonString).read(jsonPath); List elements = new LinkedList<>(); 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 5729b229a2..bb73a27fe9 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,7 +8,6 @@ import static org.junit.jupiter.api.Assertions.assertEquals; import static org.junit.jupiter.api.Assertions.assertThrows; import static org.junit.jupiter.api.Assertions.assertTrue; - 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; @@ -17,7 +16,6 @@ import java.util.LinkedHashMap; import java.util.List; import java.util.Map; - import org.junit.jupiter.api.Test; import org.junit.jupiter.api.extension.ExtendWith; import org.mockito.junit.jupiter.MockitoExtension; @@ -168,54 +166,25 @@ void json_returnsSemanticCheckException() { SemanticCheckException.class, () -> DSL.castJson(DSL.literal("\"missing quote")).valueOf()); } - @Test - void json_extract_return_object() { - List validJsonStrings = - List.of( - // test json objects are valid - "{\"a\":\"1\",\"b\":\"2\"}", - "{\"a\":1,\"b\":{\"c\":2,\"d\":3}}", - "{\"arr1\": [1,2,3], \"arr2\": [4,5,6]}", - - // test json arrays are valid - "[1, 2, 3, 4]", - "[{\"a\":1,\"b\":2}, {\"c\":3,\"d\":2}]", - - // test json scalars are valid - "\"abc\"", - "1234", - "12.34", - "true", - "false", - "null", - - // test empty string is valid - ""); - - validJsonStrings.stream() - .forEach( - str -> - assertEquals( - LITERAL_TRUE, - DSL.jsonValid(DSL.literal((ExprValueUtils.stringValue(str)))).valueOf(), - String.format("String %s must be valid json", str))); - } - @Test void json_extract_search_arrays() { - Expression jsonArray = DSL.literal(ExprValueUtils.stringValue("{\"a\":[1,2.3,\"abc\",true,null,{\"c\":1},[1,2,3]]}")); - List expectedExprValue = List.of( - new ExprIntegerValue(1), - new ExprFloatValue(2.3), - new ExprStringValue("abc"), - LITERAL_TRUE, - LITERAL_NULL, - ExprTupleValue.fromExprValueMap(Map.of("c", new ExprIntegerValue(1))), - new ExprCollectionValue(List.of(new ExprIntegerValue(1), new ExprIntegerValue(2), new ExprIntegerValue(3))) - ); + Expression jsonArray = + DSL.literal( + ExprValueUtils.stringValue("{\"a\":[1,2.3,\"abc\",true,null,{\"c\":1},[1,2,3]]}")); + List expectedExprValue = + List.of( + new ExprIntegerValue(1), + new ExprFloatValue(2.3), + new ExprStringValue("abc"), + LITERAL_TRUE, + LITERAL_NULL, + ExprTupleValue.fromExprValueMap(Map.of("c", new ExprIntegerValue(1))), + new ExprCollectionValue( + List.of( + new ExprIntegerValue(1), new ExprIntegerValue(2), new ExprIntegerValue(3)))); // extract specific index from JSON list - for (int i = 0 ; i < expectedExprValue.size() ; i++ ) { + for (int i = 0; i < expectedExprValue.size(); i++) { String path = String.format("$.a[%d]", i); Expression pathExpr = DSL.literal(ExprValueUtils.stringValue(path)); FunctionExpression expression = DSL.jsonExtract(jsonArray, pathExpr); @@ -224,9 +193,8 @@ void json_extract_search_arrays() { // extract * from JSON list Expression starPath = DSL.literal(ExprValueUtils.stringValue("$.a[*]")); - FunctionExpression starExpression = DSL.castInt(DSL.jsonExtract(jsonArray, starPath)); - assertEquals( - new ExprCollectionValue(expectedExprValue), starExpression.valueOf()); + FunctionExpression starExpression = DSL.jsonExtract(jsonArray, starPath); + assertEquals(new ExprCollectionValue(expectedExprValue), starExpression.valueOf()); } @Test @@ -243,7 +211,6 @@ void json_extract_returns_null() { "12.34", "true", "false", - "null", ""); jsonStrings.stream() @@ -252,18 +219,63 @@ void json_extract_returns_null() { assertEquals( LITERAL_NULL, DSL.jsonExtract( - DSL.literal((ExprValueUtils.stringValue(str))), - DSL.literal("$.a.path_not_found_key")).valueOf(), + DSL.literal((ExprValueUtils.stringValue(str))), + DSL.literal("$.a.path_not_found_key")) + .valueOf(), String.format("JSON string %s should return null", str))); } @Test - void json_returns_SemanticCheckException() { + void json_extract_throws_SemanticCheckException() { // invalid path assertThrows( - SemanticCheckException.class, () -> DSL.jsonExtract(DSL.literal("invalid"), DSL.literal("invalid")).valueOf()); + SemanticCheckException.class, + () -> + DSL.jsonExtract( + DSL.literal(new ExprStringValue("{\"a\":1}")), + DSL.literal(new ExprStringValue("$a"))) + .valueOf()); // invalid json - assertThrows(SemanticCheckException.class, () -> DSL.jsonExtract(DSL.literal("{\"invalid\":\"json\", \"string\"}"), DSL.literal("invalid")).valueOf()); + assertThrows( + SemanticCheckException.class, + () -> + DSL.jsonExtract( + DSL.literal(new ExprStringValue("{\"invalid\":\"json\", \"string\"}")), + DSL.literal(new ExprStringValue("$.a"))) + .valueOf()); + } + + @Test + void json_extract_throws_ExpressionEvaluationException() { + // null json + assertThrows( + ExpressionEvaluationException.class, + () -> + DSL.jsonExtract(DSL.literal(LITERAL_NULL), DSL.literal(new ExprStringValue("$.a"))) + .valueOf()); + + // null path + assertThrows( + ExpressionEvaluationException.class, + () -> + DSL.jsonExtract( + DSL.literal(new ExprStringValue("{\"a\":1}")), DSL.literal(LITERAL_NULL)) + .valueOf()); + + // missing json + assertThrows( + ExpressionEvaluationException.class, + () -> + DSL.jsonExtract(DSL.literal(LITERAL_MISSING), DSL.literal(new ExprStringValue("$.a"))) + .valueOf()); + + // missing path + assertThrows( + ExpressionEvaluationException.class, + () -> + DSL.jsonExtract( + DSL.literal(new ExprStringValue("{\"a\":1}")), DSL.literal(LITERAL_MISSING)) + .valueOf()); } } 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 3b15ca99e9..afaa989a74 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 @@ -41,8 +41,7 @@ public void test_json_valid() throws IOException { rows("json array"), rows("json scalar string"), rows("json empty string"), - rows("json nested list") - ); + rows("json nested list")); } @Test @@ -76,9 +75,9 @@ public void test_cast_json() throws IOException { rows("json array", new JSONArray(List.of(1, 2, 3, 4))), rows("json scalar string", "abc"), rows("json empty string", null), - rows("json nested list", - new JSONObject(Map.of("a", "1", "b", List.of(Map.of("c", "2"), Map.of("c", "3"))))) - ); + rows( + "json nested list", + new JSONObject(Map.of("a", "1", "b", List.of(Map.of("c", "2"), Map.of("c", "3")))))); } @Test @@ -102,9 +101,9 @@ public void test_json() throws IOException { rows("json array", new JSONArray(List.of(1, 2, 3, 4))), rows("json scalar string", "abc"), rows("json empty string", null), - rows("json nested list", - new JSONObject(Map.of("a", "1", "b", List.of(Map.of("c", "2"), Map.of("c", "3"))))) - ); + rows( + "json nested list", + new JSONObject(Map.of("a", "1", "b", List.of(Map.of("c", "2"), Map.of("c", "3")))))); } @Test @@ -113,9 +112,11 @@ public void test_json_extract() throws IOException { result = executeQuery( String.format( - "source=%s | where json_valid(json_string) | eval extracted=json_extract(json_string, '$.b') | fields test_name, extracted", + "source=%s | where json_valid(json_string) | eval" + + " extracted=json_extract(json_string, '$.b') | fields test_name, extracted", TEST_INDEX_JSON_TEST)); - verifySchema(result, schema("test_name", null, "string"), schema("extracted", null, "undefined")); + verifySchema( + result, schema("test_name", null, "string"), schema("extracted", null, "undefined")); verifyDataRows( result, rows("json nested object", new JSONObject(Map.of("c", "3"))), @@ -123,7 +124,6 @@ public void test_json_extract() throws IOException { rows("json array", null), rows("json scalar string", null), rows("json empty string", null), - rows("json nested list", new JSONArray(List.of(Map.of("c","2"), Map.of("c","3")))) - ); + rows("json nested list", new JSONArray(List.of(Map.of("c", "2"), Map.of("c", "3"))))); } }