-
Notifications
You must be signed in to change notification settings - Fork 3.8k
feat: json_merge expression and sql function #15926
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
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -22,6 +22,7 @@ | |
|
|
||
| import com.fasterxml.jackson.core.JsonProcessingException; | ||
| import com.fasterxml.jackson.databind.ObjectMapper; | ||
| import com.fasterxml.jackson.databind.ObjectReader; | ||
| import org.apache.druid.guice.annotations.Json; | ||
| import org.apache.druid.math.expr.Expr; | ||
| import org.apache.druid.math.expr.ExprEval; | ||
|
|
@@ -99,6 +100,117 @@ public ExpressionType getOutputType(InputBindingInspector inspector) | |
| } | ||
| } | ||
|
|
||
| public static class JsonMergeExprMacro implements ExprMacroTable.ExprMacro | ||
| { | ||
| public static final String NAME = "json_merge"; | ||
|
|
||
| private final ObjectMapper jsonMapper; | ||
|
|
||
| @Inject | ||
| public JsonMergeExprMacro( | ||
| @Json ObjectMapper jsonMapper | ||
| ) | ||
| { | ||
| this.jsonMapper = jsonMapper; | ||
| } | ||
|
|
||
| @Override | ||
| public String name() | ||
| { | ||
| return NAME; | ||
| } | ||
|
|
||
| @Override | ||
| public Expr apply(List<Expr> args) | ||
| { | ||
| if (args.size() < 2) { | ||
| throw validationFailed("must have at least two arguments"); | ||
| } | ||
|
|
||
| final class ParseJsonExpr extends ExprMacroTable.BaseScalarMacroFunctionExpr | ||
| { | ||
| public ParseJsonExpr(List<Expr> args) | ||
| { | ||
| super(JsonMergeExprMacro.this, args); | ||
| } | ||
|
|
||
| @Override | ||
| public ExprEval eval(ObjectBinding bindings) | ||
| { | ||
| ExprEval arg = args.get(0).eval(bindings); | ||
| Object obj; | ||
|
|
||
| if (arg.value() == null) { | ||
| throw JsonMergeExprMacro.this.validationFailed( | ||
| "invalid input expected %s but got %s instead", | ||
| ExpressionType.STRING, | ||
| arg.type() | ||
|
Comment on lines
+144
to
+147
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The error is misleading since the issue is that the value is null. I am ok with strict validation, assuming there is a way to get past these null values somehow.
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I could tolerate null in the first argument, so essentially just return the 2nd argument unmodified. |
||
| ); | ||
| } | ||
|
|
||
| try { | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. there could be a nice optimization where any literals are parsed into a map (again assuming that parsing string to json would happen inside readValue) only once. |
||
| obj = jsonMapper.readValue(getArgAsJson(arg), Object.class); | ||
| } | ||
| catch (JsonProcessingException e) { | ||
| throw JsonMergeExprMacro.this.processingFailed(e, "bad string input [%s]", arg.asString()); | ||
| } | ||
|
|
||
| ObjectReader updater = jsonMapper.readerForUpdating(obj); | ||
|
|
||
| for (int i = 1; i < args.size(); i++) { | ||
| ExprEval argSub = args.get(i).eval(bindings); | ||
|
|
||
| try { | ||
| String str = getArgAsJson(argSub); | ||
| if (str != null) { | ||
| obj = updater.readValue(str); | ||
| } | ||
| } | ||
| catch (JsonProcessingException e) { | ||
| throw JsonMergeExprMacro.this.processingFailed(e, "bad string input [%s]", argSub.asString()); | ||
| } | ||
| } | ||
|
|
||
| return ExprEval.ofComplex(ExpressionType.NESTED_DATA, obj); | ||
| } | ||
|
|
||
| @Nullable | ||
| @Override | ||
| public ExpressionType getOutputType(InputBindingInspector inspector) | ||
| { | ||
| return ExpressionType.NESTED_DATA; | ||
| } | ||
|
|
||
| private String getArgAsJson(ExprEval arg) | ||
| { | ||
| if (arg.value() == null) { | ||
| return null; | ||
| } | ||
|
|
||
| if (arg.type().is(ExprType.STRING)) { | ||
| return arg.asString(); | ||
| } | ||
|
|
||
| if (arg.type().is(ExprType.COMPLEX)) { | ||
| try { | ||
| return jsonMapper.writeValueAsString(unwrap(arg)); | ||
| } | ||
| catch (JsonProcessingException e) { | ||
| throw JsonMergeExprMacro.this.processingFailed(e, "bad complex input [%s]", arg.asString()); | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The error message should also tell what is bad about this input and what column does this message correspond to. |
||
| } | ||
| } | ||
|
Comment on lines
+194
to
+201
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. is there a way to avoid the hop of json -> string -> json when arg is already a json?
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. There isn't a way for the second parameter, because I'm using the Jackson updater, but for the first there is. However, that leads to the first variable being used again somehow on the second runs, that's the reason for the test case Happy to take any pointers here. I suspect I would need to copy the object in memory to avoid the reference in which case I'm not sure how much of an optimisation it really would be. |
||
|
|
||
| throw JsonMergeExprMacro.this.validationFailed( | ||
| "invalid input expected %s but got %s instead", | ||
| ExpressionType.STRING, | ||
| arg.type() | ||
| ); | ||
| } | ||
| } | ||
| return new ParseJsonExpr(args); | ||
| } | ||
| } | ||
|
|
||
| public static class ToJsonStringExprMacro implements ExprMacroTable.ExprMacro | ||
| { | ||
| public static final String NAME = "to_json_string"; | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -368,6 +368,7 @@ json_paths | |
| json_query | ||
| json_query_array | ||
| json_value | ||
| json_merge | ||
| karlkfi | ||
| kerberos | ||
| keystore | ||
|
|
||
Uh oh!
There was an error while loading. Please reload this page.