Skip to content

Commit

Permalink
addressed PR comments
Browse files Browse the repository at this point in the history
Signed-off-by: Kenrick Yap <[email protected]>
  • Loading branch information
kenrickyap committed Jan 29, 2025
1 parent 0b9e9e4 commit 6678be4
Show file tree
Hide file tree
Showing 3 changed files with 43 additions and 63 deletions.
10 changes: 6 additions & 4 deletions core/src/main/java/org/opensearch/sql/utils/JsonUtils.java
Original file line number Diff line number Diff line change
@@ -1,9 +1,5 @@
package org.opensearch.sql.utils;

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 com.fasterxml.jackson.core.JsonProcessingException;
import com.fasterxml.jackson.databind.JsonNode;
import com.fasterxml.jackson.databind.ObjectMapper;
Expand All @@ -27,6 +23,8 @@
import org.opensearch.sql.data.model.ExprValueUtils;
import org.opensearch.sql.exception.SemanticCheckException;

import static org.opensearch.sql.data.model.ExprValueUtils.*;

@UtilityClass
public class JsonUtils {
/**
Expand Down Expand Up @@ -86,6 +84,10 @@ public static ExprValue castJson(ExprValue json) {
* @return ExprValue of value at given path of json string.
*/
public static ExprValue extractJson(ExprValue json, ExprValue path) {
if (json == LITERAL_NULL || json == LITERAL_MISSING) {
return json;
}

String jsonString = json.stringValue();
String jsonPath = path.stringValue();

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -193,34 +193,18 @@ void json_returnsSemanticCheckException() {

@Test
void json_extract_search() {
Expression jsonArray = DSL.literal(ExprValueUtils.stringValue("{\"a\":1}"));
ExprValue expectedExprValue = new ExprIntegerValue(1);
Expression pathExpr = DSL.literal(ExprValueUtils.stringValue("$.a"));
FunctionExpression expression = DSL.jsonExtract(jsonArray, pathExpr);
assertEquals(expectedExprValue, expression.valueOf());
ExprValue expected = new ExprIntegerValue(1);
execute_extract_json(expected, "{\"a\":1}", "$.a");
}

@Test
void json_extract_search_arrays_out_of_bound() {
Expression jsonArray = DSL.literal(ExprValueUtils.stringValue("{\"a\":[1,2,3}"));

// index out of bounds
assertThrows(
SemanticCheckException.class,
() -> DSL.jsonExtract(jsonArray, DSL.literal(new ExprStringValue("$.a[3]"))).valueOf());

// negative index
assertThrows(
SemanticCheckException.class,
() -> DSL.jsonExtract(jsonArray, DSL.literal(new ExprStringValue("$.a[-1]"))).valueOf());
execute_extract_json(LITERAL_NULL, "{\"a\":[1,2,3]}", "$.a[4]");
}

@Test
void json_extract_search_arrays() {
Expression jsonArray =
DSL.literal(
ExprValueUtils.stringValue(
"{\"a\":[1,2.3,\"abc\",true,null,{\"c\":{\"d\":1}},[1,2,3]]}"));
String jsonArray = "{\"a\":[1,2.3,\"abc\",true,null,{\"c\":{\"d\":1}},[1,2,3]]}";
List<ExprValue> expectedExprValue =
List.of(
new ExprIntegerValue(1),
Expand All @@ -237,22 +221,17 @@ void json_extract_search_arrays() {
// extract specific index from JSON list
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);
assertEquals(expectedExprValue.get(i), expression.valueOf());
execute_extract_json(expectedExprValue.get(i), jsonArray, path);
}

// extract nested object
ExprValue nestedExpected =
ExprTupleValue.fromExprValueMap(Map.of("d", new ExprIntegerValue(1)));
Expression nestedPath = DSL.literal(ExprValueUtils.stringValue("$.a[5].c"));
FunctionExpression nestedExpression = DSL.jsonExtract(jsonArray, nestedPath);
assertEquals(nestedExpected, nestedExpression.valueOf());
execute_extract_json(nestedExpected, jsonArray, "$.a[5].c");

// extract * from JSON list
Expression starPath = DSL.literal(ExprValueUtils.stringValue("$.a[*]"));
FunctionExpression starExpression = DSL.jsonExtract(jsonArray, starPath);
assertEquals(new ExprCollectionValue(expectedExprValue), starExpression.valueOf());
ExprValue starExpected = new ExprCollectionValue(expectedExprValue);
execute_extract_json(starExpected, jsonArray, "$.a[*]");
}

@Test
Expand All @@ -271,48 +250,47 @@ void json_extract_returns_null() {
"false",
"");

jsonStrings.stream()
.forEach(
str ->
assertEquals(
LITERAL_NULL,
DSL.jsonExtract(
DSL.literal((ExprValueUtils.stringValue(str))),
DSL.literal("$.a.path_not_found_key"))
.valueOf(),
String.format("JSON string %s should return null", str)));
jsonStrings.forEach(
str -> execute_extract_json(LITERAL_NULL, str, "$.a.path_not_found_key")
);

// null json
assertEquals(LITERAL_NULL, DSL.jsonExtract(DSL.literal(LITERAL_NULL), DSL.literal(new ExprStringValue("$.a"))).valueOf());

// missing json
assertEquals(LITERAL_MISSING, DSL.jsonExtract(DSL.literal(LITERAL_MISSING), DSL.literal(new ExprStringValue("$.a"))).valueOf());
}

@Test
void json_extract_throws_SemanticCheckException() {
// invalid path
assertThrows(
SemanticCheckException invalidPathError = assertThrows(
SemanticCheckException.class,
() ->
DSL.jsonExtract(
DSL.literal(new ExprStringValue("{\"a\":1}")),
DSL.literal(new ExprStringValue("$a")))
.valueOf());
assertEquals(
"JSON path '\"$a\"' is not valid. Error details: Illegal character at position 1 expected '.' or '['",
invalidPathError.getMessage());


// invalid json
assertThrows(
SemanticCheckException invalidJsonError = assertThrows(
SemanticCheckException.class,
() ->
DSL.jsonExtract(
DSL.literal(new ExprStringValue("{\"invalid\":\"json\", \"string\"}")),
DSL.literal(new ExprStringValue("$.a")))
.valueOf());
assertEquals(
"JSON string '\"{\"invalid\":\"json\", \"string\"}\"' is not valid. Error details: net.minidev.json.parser.ParseException: Unexpected character (}) at position 26.",
invalidJsonError.getMessage());
}

@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,
Expand All @@ -321,13 +299,6 @@ void json_extract_throws_ExpressionEvaluationException() {
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,
Expand All @@ -336,4 +307,11 @@ void json_extract_throws_ExpressionEvaluationException() {
DSL.literal(new ExprStringValue("{\"a\":1}")), DSL.literal(LITERAL_MISSING))
.valueOf());
}

private static void execute_extract_json(ExprValue expected, String json, String path) {
Expression pathExpr = DSL.literal(ExprValueUtils.stringValue(path));
Expression jsonExpr = DSL.literal(ExprValueUtils.stringValue(json));
ExprValue actual = DSL.jsonExtract(jsonExpr, pathExpr).valueOf();
assertEquals(expected, actual);
}
}
8 changes: 4 additions & 4 deletions docs/user/ppl/functions/json.rst
Original file line number Diff line number Diff line change
Expand Up @@ -75,10 +75,10 @@ Argument type: STRING, STRING

Return type: BOOLEAN/DOUBLE/INTEGER/NULL/STRUCT/ARRAY

- Returns a JSON array for multiple paths or if the path leads to an array.
- Return null if path is not valid.
- Throws error if `doc` or `path` is malformed.
- Throws error if `doc` or `path` is MISSING or NULL.
- Returns a JSON array if path points to multiple results (e.g. $.a[*]) or if the path points to an array.
- Return null if path is not valid is MISSING or NULL.
- Throws SemanticCheckException if `doc` or `path` is malformed.
- Throws ExpressionEvaluationException if `path` is missing.

Example::

Expand Down

0 comments on commit 6678be4

Please sign in to comment.