suneet-s commented on a change in pull request #10401:
URL: https://github.com/apache/druid/pull/10401#discussion_r492499110
##########
File path: core/src/main/java/org/apache/druid/math/expr/ApplyFunction.java
##########
@@ -49,6 +51,16 @@
*/
String name();
+ default boolean canVectorize(Expr.InputBindingTypes inputTypes, Expr lambda,
List<Expr> args)
+ {
+ return false;
+ }
+
+ default <T> ExprVectorProcessor<T>
asVectorProcessor(Expr.VectorInputBindingTypes inputTypes, Expr lambda,
List<Expr> args)
Review comment:
javadocs for these functions please.
##########
File path: core/src/main/java/org/apache/druid/math/expr/ApplyFunction.java
##########
@@ -49,6 +51,16 @@
*/
String name();
+ default boolean canVectorize(Expr.InputBindingTypes inputTypes, Expr lambda,
List<Expr> args)
+ {
+ return false;
+ }
+
+ default <T> VectorExprProcessor<T>
asVectorProcessor(Expr.VectorInputBindingTypes inputTypes, Expr lambda,
List<Expr> args)
Review comment:
javadocs for both of these functions please
##########
File path: sql/src/test/java/org/apache/druid/sql/calcite/CalciteQueryTest.java
##########
@@ -5912,9 +5912,6 @@ public void testExpressionFilteringAndGrouping() throws
Exception
@Test
public void testExpressionFilteringAndGroupingUsingCastToLong() throws
Exception
{
- // Cannot vectorize due to virtual columns.
Review comment:
🎉 so much more vectorization!
##########
File path: core/src/main/java/org/apache/druid/math/expr/Expr.java
##########
@@ -148,6 +171,47 @@ default ExprType getOutputType(InputBindingTypes
inputTypes)
{
@Nullable
ExprType getType(String name);
+
+ default boolean areNumeric(List<Expr> args)
+ {
+ boolean numeric = args.size() > 0;
Review comment:
Why is an empty list non numeric?
Maybe add javadocs to clarify this behavior.
##########
File path:
sql/src/main/java/org/apache/druid/sql/calcite/rel/VirtualColumnRegistry.java
##########
@@ -136,9 +137,11 @@ public RowSignature getFullRowSignature()
final RowSignature.Builder builder =
RowSignature.builder().addAll(baseRowSignature);
+ ColumnInspector baseRowsInspector = builder.build().asColumnInspector();
Review comment:
Does this mean a virtual column can't reference another virtual column?
since the ColumnInspector is built with just he `baseRowSignature`?
----------------------------------------------------------------
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:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
