clintropolis commented on code in PR #13732:
URL: https://github.com/apache/druid/pull/13732#discussion_r1098054421


##########
core/src/main/java/org/apache/druid/math/expr/ExprEval.java:
##########
@@ -463,6 +479,13 @@ public static ExprEval bestEffortOf(@Nullable Object val)
       return ofArray(coerced.lhs, coerced.rhs);
     }
 
+    // in 'best effort' mode, we couldn't possibly use byte[] as a complex or 
anything else useful without type
+    // knowledge, so lets turn it into a base64 encoded string so at least 
something downstream can use it by decoding
+    // back into bytes
+    if (val instanceof byte[]) {
+      return new StringExprEval(StringUtils.encodeBase64String((byte[]) val));
+    }

Review Comment:
   this is going _into_ the expression system, if we don't handle it like this, 
it ends up as "unknown complex" and is basically useless in that form since we 
don't have a native byte[] typed expressions, and complex types cannot be cast 
to anything else (not that would do anything useful here). 
   
   With nested columns, leaving it as a byte[] is a problem, since we are using 
this method to process inputs to determine their type, we have a [default 
case](https://github.com/apache/druid/blob/master/processing/src/main/java/org/apache/druid/segment/NestedDataColumnIndexer.java#L464)
 for anything that leaks through that isn't `LONG` or `DOUBLE` or `STRING` that 
effectively calls java toString on things that makes these end up not very 
useful `byte[].toString`.
   
   I could change this to be a parse exception, but even if we did that, i 
still think it would still be useful to feed these into the expressions as a 
base64 encoded string rather than throwing it away. Maybe in the future it 
would be nice to have a byte blob type to not have to encode it, but until 
then, I think this is the most useful thing we can do, and its consistent with 
the behavior we use to handle byte[] in 
[`Rows.objectToStrings`](https://github.com/apache/druid/blob/master/core/src/main/java/org/apache/druid/data/input/Rows.java#L63)
 that normal string columns go through.
   
   



-- 
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.

To unsubscribe, e-mail: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]


---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]

Reply via email to