You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@druid.apache.org by GitBox <gi...@apache.org> on 2021/05/18 10:30:51 UTC
[GitHub] [druid] clintropolis opened a new pull request #11270: fix issue with empty array converting to string expression instead of string array
clintropolis opened a new pull request #11270:
URL: https://github.com/apache/druid/pull/11270
### Description
This PR fixes an issue with expression evaluation for array value bindings when input is provided by JSON deserialized values (rather than parsing a constant or provided by a column for example), such as when results are being merged on a broker.
The main issue is that an empty array result from a historical is later deserialized from JSON into an empty ArrayList, which `ExprEval.coerceListToArray` converts into `Object[0]`, but `ExprEval.bestEffortOf` is unable to handle this. This PR fixes the issue by treating this empty list as an empty string array (since string array is the default in the other cases of this method, as strings are traditionally the more flexible type in druid)
<hr>
This PR has:
- [x] been self-reviewed.
- [x] added comments explaining the "why" and the intent of the code wherever would not be obvious for an unfamiliar reader.
- [x] added unit tests or modified existing tests to cover new code paths, ensuring the threshold for [code coverage](https://github.com/apache/druid/blob/master/dev/code-review/code-coverage.md) is met.
- [x] been tested in a test Druid cluster.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@druid.apache.org
For additional commands, e-mail: commits-help@druid.apache.org
[GitHub] [druid] asdf2014 merged pull request #11270: fix issue with empty array converting to string expression instead of string array
Posted by GitBox <gi...@apache.org>.
asdf2014 merged pull request #11270:
URL: https://github.com/apache/druid/pull/11270
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@druid.apache.org
For additional commands, e-mail: commits-help@druid.apache.org
[GitHub] [druid] jihoonson commented on a change in pull request #11270: fix issue with empty array converting to string expression instead of string array
Posted by GitBox <gi...@apache.org>.
jihoonson commented on a change in pull request #11270:
URL: https://github.com/apache/druid/pull/11270#discussion_r635548194
##########
File path: core/src/main/java/org/apache/druid/math/expr/ExprEval.java
##########
@@ -378,7 +378,7 @@ public static Object coerceListToArray(@Nullable List<?> val, boolean homogenize
return new String[]{null};
} else {
if (val != null) {
- return val.toArray();
+ return new String[0];
Review comment:
nit: we can cache the instance of an empty array. A similar technique can be applied for the single element array of null above. I think we can do this as a follow-up.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@druid.apache.org
For additional commands, e-mail: commits-help@druid.apache.org