yashmayya commented on code in PR #13711:
URL: https://github.com/apache/pinot/pull/13711#discussion_r1696983002
##########
pinot-core/src/main/java/org/apache/pinot/core/query/postaggregation/PostAggregationFunction.java:
##########
@@ -38,20 +37,20 @@ public class PostAggregationFunction {
public PostAggregationFunction(String functionName, ColumnDataType[]
argumentTypes) {
String canonicalName = FunctionRegistry.canonicalize(functionName);
- FunctionInfo functionInfo =
FunctionRegistry.lookupFunctionInfo(canonicalName, argumentTypes);
+ int numArguments = argumentTypes.length;
+ FunctionInfo functionInfo =
FunctionRegistry.lookupFunctionInfo(canonicalName, numArguments);
Review Comment:
> In case we want to be extra safe though, we could do this -
https://github.com/apache/pinot/pull/13711#issuecomment-2258300426.
To clarify, this will ensure that argument type based function lookup will
function the same as the earlier argument count based function lookup in case
of heterogeneous argument types in all cases (and we can un-revert the
`PostAggregationFunction` changes). Actually, on thinking about this a bit
more, I think we should definitely do so because even if we fix the literal
handling in `PostAggregationHandler`, we'll still run into issues with `DOUBLE`
/ `INT` comparisons and the like in the v1 engine.
--
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: commits-unsubscr...@pinot.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscr...@pinot.apache.org
For additional commands, e-mail: commits-h...@pinot.apache.org
