Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

PPL: Add json_object command #3242

Open
wants to merge 29 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
29 commits
Select commit Hold shift + click to select a range
f82554b
added implementation
14yapkc1 Jan 3, 2025
08e16ff
added doctest, integ-tests, and unit tests
14yapkc1 Jan 6, 2025
ebb5cc3
addressed pr comments
kenrickyap Jan 7, 2025
b094dec
addressed PR comments
kenrickyap Jan 7, 2025
373e53d
removed unused dependencies
kenrickyap Jan 7, 2025
18a3cb3
linting
kenrickyap Jan 7, 2025
372fd33
addressed pr comment and rolling back disabled test case
kenrickyap Jan 8, 2025
be33ff4
removed disabled import
kenrickyap Jan 9, 2025
b3a5eae
nit
kenrickyap Jan 9, 2025
6397af4
Update integ-test/src/test/java/org/opensearch/sql/ppl/JsonFunctionIT…
kenrickyap Jan 9, 2025
6d9c50a
fixed integ test
kenrickyap Jan 9, 2025
5b1f6de
json_valid: null and missing should return false
acarbonetto Jan 15, 2025
182a266
PPL: Add json and cast to json functions
acarbonetto Jan 8, 2025
80afe48
PPL: Update json cast for review
acarbonetto Jan 8, 2025
7d212b8
Fix testes
acarbonetto Jan 9, 2025
0e8bf0b
SPOTLESS
acarbonetto Jan 14, 2025
bc11951
Clean up for merge
acarbonetto Jan 15, 2025
d708a69
Add cast to scalar from undefined expression
acarbonetto Jan 16, 2025
c76a3fa
Add test for missing/null
acarbonetto Jan 16, 2025
26623c9
Clean up merge conflicts
acarbonetto Jan 17, 2025
1af9053
Fix jacoco coverage
acarbonetto Jan 17, 2025
122cd1d
Move to Switch by json type
acarbonetto Jan 17, 2025
cbc0552
Add JSON_OBJECT to ppl
acarbonetto Jan 20, 2025
423365b
SPOTLESS
acarbonetto Jan 20, 2025
09231fb
Fix doctest
acarbonetto Jan 21, 2025
4759311
Fix merge conflicts and tests
acarbonetto Feb 12, 2025
2dd3fc6
Fix merge conflicts
acarbonetto Feb 12, 2025
bb46eb9
Fix IT tests
acarbonetto Feb 12, 2025
bc4ca96
Fix IT tests
acarbonetto Feb 12, 2025
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions core/src/main/java/org/opensearch/sql/expression/DSL.java
Original file line number Diff line number Diff line change
Expand Up @@ -691,6 +691,10 @@ public static FunctionExpression stringToJson(Expression value) {
return compile(FunctionProperties.None, BuiltinFunctionName.JSON, value);
}

public static FunctionExpression jsonObject(Expression... expressions) {
return compile(FunctionProperties.None, BuiltinFunctionName.JSON_OBJECT, expressions);
}

public static Aggregator avg(Expression... expressions) {
return aggregate(BuiltinFunctionName.AVG, expressions);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -207,6 +207,7 @@ public enum BuiltinFunctionName {
/** Json Functions. */
JSON_VALID(FunctionName.of("json_valid")),
JSON(FunctionName.of("json")),
JSON_OBJECT(FunctionName.of("json_object")),

/** GEOSPATIAL Functions. */
GEOIP(FunctionName.of("geoip")),
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,22 +7,41 @@

import static org.opensearch.sql.data.type.ExprCoreType.BOOLEAN;
import static org.opensearch.sql.data.type.ExprCoreType.STRING;
import static org.opensearch.sql.data.type.ExprCoreType.STRUCT;
import static org.opensearch.sql.data.type.ExprCoreType.UNDEFINED;
import static org.opensearch.sql.expression.DSL.jsonObject;
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 java.util.Iterator;
import java.util.LinkedHashMap;
import java.util.List;
import lombok.experimental.UtilityClass;
import org.apache.commons.lang3.tuple.Pair;
import org.opensearch.sql.data.model.ExprTupleValue;
import org.opensearch.sql.data.model.ExprValue;
import org.opensearch.sql.data.type.ExprCoreType;
import org.opensearch.sql.data.type.ExprType;
import org.opensearch.sql.exception.SemanticCheckException;
import org.opensearch.sql.expression.Expression;
import org.opensearch.sql.expression.FunctionExpression;
import org.opensearch.sql.expression.env.Environment;
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.expression.function.FunctionBuilder;
import org.opensearch.sql.expression.function.FunctionName;
import org.opensearch.sql.expression.function.FunctionResolver;
import org.opensearch.sql.expression.function.FunctionSignature;
import org.opensearch.sql.utils.JsonUtils;

@UtilityClass
public class JsonFunctions {
public void register(BuiltinFunctionRepository repository) {
repository.register(jsonValid());
repository.register(jsonFunction());
repository.register(jsonObject());
}

private DefaultFunctionResolver jsonValid() {
Expand All @@ -35,4 +54,61 @@ private DefaultFunctionResolver jsonFunction() {
BuiltinFunctionName.JSON.getName(),
impl(nullMissingHandling(JsonUtils::castJson), UNDEFINED, STRING));
}

/** Creates a JSON Object/tuple expr from a given list of kv pairs. */
private static FunctionResolver jsonObject() {
return new FunctionResolver() {
@Override
public FunctionName getFunctionName() {
return BuiltinFunctionName.JSON_OBJECT.getName();
}

@Override
public Pair<FunctionSignature, FunctionBuilder> resolve(
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@acarbonetto a naive question - do we have more use cases of kv pairs such as this one ?
if so would it make sense to move this code into a more generic place?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Interesting question - no. json_object is very unique. Although we do have cases of path-value pairings, we're going to treat them differently.

FunctionSignature unresolvedSignature) {
List<ExprType> paramList = unresolvedSignature.getParamTypeList();
// check that we got an even number of arguments
if (paramList.size() % 2 != 0) {
throw new SemanticCheckException(
String.format(
"Expected an even number of arguments but instead got %d arguments",
paramList.size()));
}

// check that each "key" argument (of key-value pair) is a string
for (int i = 0; i < paramList.size(); i = i + 2) {
ExprType paramType = paramList.get(i);
if (!ExprCoreType.STRING.equals(paramType)) {
throw new SemanticCheckException(
String.format(
"Expected type %s instead of %s for parameter #%d",
ExprCoreType.STRING, paramType.typeName(), i + 1));
}
}

// return the unresolved signature and function builder
return Pair.of(
unresolvedSignature,
(functionProperties, arguments) ->
new FunctionExpression(getFunctionName(), arguments) {
@Override
public ExprValue valueOf(Environment<Expression, ExprValue> valueEnv) {
LinkedHashMap<String, ExprValue> tupleValues = new LinkedHashMap<>();
Iterator<Expression> iter = getArguments().iterator();
while (iter.hasNext()) {
tupleValues.put(
iter.next().valueOf(valueEnv).stringValue(),
iter.next().valueOf(valueEnv));
}
return ExprTupleValue.fromExprValueMap(tupleValues);
}

@Override
public ExprType type() {
return STRUCT;
}
});
}
};
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -194,27 +194,91 @@ void json_returnsScalar() {

@Test
void json_returnsSemanticCheckException() {
List<LiteralExpression> expressions =
List.of(
DSL.literal("invalid"), // invalid type
DSL.literal("{{[}}"), // missing bracket
DSL.literal("[}"), // missing bracket
DSL.literal("}"), // missing bracket
DSL.literal("\"missing quote"), // missing quote
DSL.literal("abc"), // not a type
DSL.literal("97ab"), // not a type
DSL.literal("{1, 2, 3, 4}"), // invalid object
DSL.literal("{123: 1, true: 2, null: 3}"), // invalid object
DSL.literal("{\"invalid\":\"json\", \"string\"}"), // invalid object
DSL.literal("[\"a\": 1, \"b\": 2]") // invalid array
);
// invalid type
assertThrows(
SemanticCheckException.class, () -> DSL.castJson(DSL.literal("invalid")).valueOf());

expressions.stream()
.forEach(
expr ->
assertThrows(
SemanticCheckException.class,
() -> DSL.castJson(expr).valueOf(),
"Expected to throw SemanticCheckException when calling castJson with " + expr));
// missing bracket
assertThrows(SemanticCheckException.class, () -> DSL.castJson(DSL.literal("{{[}}")).valueOf());

// missing quote
assertThrows(
SemanticCheckException.class, () -> DSL.castJson(DSL.literal("\"missing quote")).valueOf());
}

@Test
public void json_object_returns_tuple() {
FunctionExpression exp;

// Setup
LinkedHashMap<String, ExprValue> objectMap = new LinkedHashMap<>();
objectMap.put("foo", new ExprStringValue("foo"));
objectMap.put("fuzz", ExprBooleanValue.of(true));
objectMap.put("bar", new ExprLongValue(1234));
objectMap.put("bar2", new ExprDoubleValue(12.34));
objectMap.put("baz", ExprNullValue.of());
objectMap.put(
"obj", ExprTupleValue.fromExprValueMap(Map.of("internal", new ExprStringValue("value"))));
// TODO: requires json_array()
// objectMap.put(
// "arr",
// new ExprCollectionValue(
// List.of(new ExprStringValue("string"), ExprBooleanValue.of(true),
// ExprNullValue.of())));
ExprValue expectedTupleExpr = ExprTupleValue.fromExprValueMap(objectMap);

// exercise
exp =
DSL.jsonObject(
DSL.literal("foo"), DSL.literal("foo"),
DSL.literal("fuzz"), DSL.literal(true),
DSL.literal("bar"), DSL.literal(1234),
DSL.literal("bar2"), DSL.literal(12.34),
DSL.literal("baz"), DSL.literal(LITERAL_NULL),
DSL.literal("obj"), DSL.jsonObject(DSL.literal("internal"), DSL.literal("value")));

// Verify
var value = exp.valueOf();
assertTrue(value instanceof ExprTupleValue);
assertEquals(expectedTupleExpr, value);
}

@Test
public void json_object_returns_empty_tuple() {
FunctionExpression exp;

// Setup
LinkedHashMap<String, ExprValue> objectMap = new LinkedHashMap<>();
ExprValue expectedTupleExpr = ExprTupleValue.fromExprValueMap(objectMap);

// exercise
exp = DSL.jsonObject();

// Verify
var value = exp.valueOf();
assertTrue(value instanceof ExprTupleValue);
assertEquals(expectedTupleExpr, value);
}

@Test
public void json_object_throws_SemanticCheckException() {
// wrong number of arguments
assertThrows(
SemanticCheckException.class, () -> DSL.jsonObject(DSL.literal("only one")).valueOf());
assertThrows(
SemanticCheckException.class,
() ->
DSL.jsonObject(DSL.literal("one"), DSL.literal("two"), DSL.literal("three")).valueOf());

// key argument is not a string
assertThrows(
SemanticCheckException.class,
() -> DSL.jsonObject(DSL.literal(1234), DSL.literal("two")).valueOf());
assertThrows(
SemanticCheckException.class,
() ->
DSL.jsonObject(
DSL.literal("one"), DSL.literal(true), DSL.literal(true), DSL.literal("four"))
.valueOf());
}
}
40 changes: 40 additions & 0 deletions docs/user/ppl/functions/json.rst
Original file line number Diff line number Diff line change
Expand Up @@ -60,3 +60,43 @@ Example::
| json scalar string | "abc" | "abc" |
| json empty string | | null |
+---------------------+---------------------------------+-------------------------+

JSON_OBJECT
-----------

Description
>>>>>>>>>>>

Usage: `json_object(<key>, <value>[, <key>, <value>]...)` returns a JSON object from key-value pairs.

Argument type:
- A \<key\> must be STRING.
- A \<value\> can be a scalar, another json object, or json array type. Note: scalar fields will be treated as single-value. Use `json_array` to construct an array value from a multi-value.

Return type: STRUCT

Example:

os> source=people | eval result = json_object('key', 123.45) | fields result
fetched rows / total rows = 1/1
+-----------------+
| result |
|-----------------|
| {'key': 123.45} |
+-----------------+

os> source=people | eval result = json_object('outer', json_object('inner', 123.45)) | fields result
fetched rows / total rows = 1/1
+------------------------------+
| result |
|------------------------------|
| {'outer': {'inner': 123.45}} |
+------------------------------+

source=people | eval result = json_object('array_doc', json_array(123.45, "string", true, null)) | fields result
fetched rows / total rows = 1/1
+------------------------------------------------+
| result |
+------------------------------------------------+
| {"array_doc":[123.45, "string", true, null]} |
+------------------------------------------------+
Original file line number Diff line number Diff line change
Expand Up @@ -183,4 +183,38 @@ public void test_cast_json_scalar_to_type() throws IOException {
verifyDataRows(
result, rows("json scalar boolean true", true), rows("json scalar boolean false", false));
}

@Test
public void test_json_object() throws IOException {
JSONObject result;

result =
executeQuery(
String.format(
"source=%s | where json_valid(json_string) | "
+ "eval obj=json_object('key', json(json_string)) | "
+ "fields test_name, obj",
TEST_INDEX_JSON_TEST));
verifySchema(result, schema("test_name", null, "string"), schema("obj", null, "struct"));
verifyDataRows(
result,
rows(
"json nested object",
new JSONObject(
Map.of(
"key",
Map.of("a", "1", "b", Map.of("c", "3"), "d", List.of(Boolean.FALSE, 3))))),
rows("json object", new JSONObject(Map.of("key", Map.of("a", "1", "b", "2")))),
rows(
"json nested array",
new JSONObject(Map.of("key", List.of(1, 2, 3, Map.of("true", true, "number", 123))))),
rows("json array", new JSONObject(Map.of("key", List.of(1, 2, 3, 4)))),
rows("json scalar string", Map.of("key", "abc")),
rows("json scalar int", Map.of("key", 1234)),
rows("json scalar float", Map.of("key", 12.34)),
rows("json scalar double", Map.of("key", 2.99792458e8)),
rows("json scalar boolean true", Map.of("key", true)),
rows("json scalar boolean false", Map.of("key", false)),
rows("json empty string", Map.of()));
}
}
1 change: 1 addition & 0 deletions ppl/src/main/antlr/OpenSearchPPLLexer.g4
Original file line number Diff line number Diff line change
Expand Up @@ -334,6 +334,7 @@ CIDRMATCH: 'CIDRMATCH';
// JSON FUNCTIONS
JSON_VALID: 'JSON_VALID';
JSON: 'JSON';
JSON_OBJECT: 'JSON_OBJECT';

// FLOWCONTROL FUNCTIONS
IFNULL: 'IFNULL';
Expand Down
10 changes: 10 additions & 0 deletions ppl/src/main/antlr/OpenSearchPPLParser.g4
Original file line number Diff line number Diff line change
Expand Up @@ -310,6 +310,7 @@ valueExpression
| extractFunction # extractFunctionCall
| getFormatFunction # getFormatFunctionCall
| timestampFunction # timestampFunctionCall
| jsonObjectFunction # jsonObjectFunctionCall
| LT_PRTHS valueExpression RT_PRTHS # parentheticValueExpr
;

Expand All @@ -324,6 +325,10 @@ positionFunction
: positionFunctionName LT_PRTHS functionArg IN functionArg RT_PRTHS
;

jsonObjectFunction
: jsonObjectFunctionName LT_PRTHS (functionArg COMMA functionArg (COMMA functionArg COMMA functionArg)*)? RT_PRTHS
;

booleanExpression
: booleanFunctionCall
;
Expand Down Expand Up @@ -420,6 +425,7 @@ evalFunctionName
| flowControlFunctionName
| systemFunctionName
| positionFunctionName
| jsonObjectFunctionName
| jsonFunctionName
| geoipFunctionName
;
Expand Down Expand Up @@ -707,6 +713,10 @@ positionFunctionName
: POSITION
;

jsonObjectFunctionName
: JSON_OBJECT
;

jsonFunctionName
: JSON
;
Expand Down
Loading