cheddar commented on code in PR #13214:
URL: https://github.com/apache/druid/pull/13214#discussion_r992878156
##########
sql/src/main/java/org/apache/druid/sql/calcite/expression/builtin/NestedDataOperatorConversions.java:
##########
@@ -276,7 +276,8 @@ public SqlRexConvertlet createConvertlet(PlannerContext
plannerContext)
call.operand(0),
call.operand(1)
);
- } else if
(SqlTypeName.APPROX_TYPES.contains(sqlType.getSqlTypeName())) {
+ } else if (SqlTypeName.APPROX_TYPES.contains(sqlType.getSqlTypeName())
||
+ SqlTypeName.DECIMAL.equals(sqlType.getSqlTypeName())) {
Review Comment:
perhaps invert these? Equality is faster than `.contains()` so will be
better to check the equality first and then do the `.contains()` instead of
forcing the `.contains()` to fail first before doing the equality.
##########
sql/src/test/java/org/apache/druid/sql/calcite/CalciteNestedDataQueryTest.java:
##########
@@ -2003,6 +2032,35 @@ public void testReturningAndSumPathDecimal()
);
}
+ @Test
+ public void testReturningAndSumPathDecimalWithMaths()
+ {
+ testQuery(
Review Comment:
How did these tests fail before this change? The description makes it sound
like the failure is actually a failure to pick the vectorized version rather
than a failure in actual results, but I don't see this test ensuring that the
vectorized version is used?
##########
core/src/main/java/org/apache/druid/math/expr/vector/UnivariateFunctionVectorValueProcessor.java:
##########
@@ -74,5 +76,7 @@ public final ExprEvalVector<TOutput>
evalVector(Expr.VectorInputBinding bindings
abstract void processIndex(TInput input, int i);
+ abstract void processNull(int i);
Review Comment:
Instead of another abstract method call, I wonder how far we could get with
a `.getValueForNull()` method that return a value that we can set? With that
setup, we would be able to call that once and then reuse the value, allowing
for inlining and stuff. It should work well for the scope of changes in this
PR...
--
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]
