cheddar commented on code in PR #13209:
URL: https://github.com/apache/druid/pull/13209#discussion_r992870933
##########
processing/src/main/java/org/apache/druid/segment/virtual/SingleStringInputDeferredEvaluationExpressionDimensionVectorSelector.java:
##########
@@ -118,7 +119,7 @@ public int getCurrentVectorSize()
*/
private static final class StringLookupVectorInputBindings implements
Expr.VectorInputBinding
{
- private final String[] currentValue = new String[1];
+ private final Object[] currentValue = new Object[1];
Review Comment:
Nitish: The naming of this private static class doesn't really line up with
its implementation anymore?
##########
core/src/main/java/org/apache/druid/math/expr/vector/VectorProcessors.java:
##########
@@ -851,17 +861,19 @@ public void processIndex(
outputNulls[i] = true;
return;
}
- final boolean bool = Evals.asBoolean(rightInput[i]);
+ final boolean bool = Evals.asBoolean((String) rightInput[i]);
output[i] = Evals.asLong(bool);
outputNulls[i] = bool;
return;
} else if (rightNull) {
- final boolean bool = Evals.asBoolean(leftInput[i]);
+ final boolean bool = Evals.asBoolean((String) leftInput[i]);
Review Comment:
Here too, why is it safe to cast? do we need to check the type first? Or
maybe check the type of the array that we are gotten, so we can amortize the
cost of the check?
##########
core/src/main/java/org/apache/druid/math/expr/vector/VectorStringProcessors.java:
##########
@@ -32,32 +33,35 @@ public static <T> ExprVectorProcessor<T>
concat(Expr.VectorInputBindingInspector
{
final ExprVectorProcessor processor;
if (NullHandling.sqlCompatible()) {
- processor = new StringOutStringsInFunctionVectorProcessor(
+ processor = new ObjectOutObjectsInFunctionVectorProcessor(
left.buildVectorized(inspector),
right.buildVectorized(inspector),
- inspector.getMaxVectorSize()
+ inspector.getMaxVectorSize(),
+ ExpressionType.STRING
)
{
@Nullable
@Override
- protected String processValue(@Nullable String leftVal, @Nullable
String rightVal)
+ protected String processValue(@Nullable Object leftVal, @Nullable
Object rightVal)
{
// in sql compatible mode, nulls are handled by super class and
never make it here...
- return leftVal + rightVal;
+ return leftVal + (String) rightVal;
Review Comment:
You could call `.toString()` and avoid ClassCastExceptions here? Or, if
it's important to do the cast, we should really do an instanceof check first so
that we can generate a better error message than just `ClassCastException`
--
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]
