dario-liberman commented on code in PR #11092:
URL: https://github.com/apache/pinot/pull/11092#discussion_r1272704204
##########
pinot-core/src/main/java/org/apache/pinot/core/query/aggregation/function/FunnelCountAggregationFunction.java:
##########
@@ -180,8 +256,12 @@ public String toExplainString() {
return stringBuilder.append(')').toString();
}
- private static LongArrayList toLongArrayList(List<Long> longList) {
- return longList instanceof LongArrayList ? ((LongArrayList)
longList).clone() : new LongArrayList(longList);
+ private Dictionary getDictionary(Map<ExpressionContext, BlockValSet>
blockValSetMap) {
+ final Dictionary primaryCorrelationDictionary =
blockValSetMap.get(_primaryCorrelationCol).getDictionary();
+ Preconditions.checkArgument(primaryCorrelationDictionary != null,
+ "CORRELATE_BY column in FUNNELCOUNT aggregation function not
supported, please use a dictionary encoded "
Review Comment:
It is mentioned as a limitation in the documentation for the function
(linked above in this PR). I personally have no plans to remove this
limitation, as it would be rare to correlate by something other than an actual
column. Someone else in the community could obviously contribute the necessary
changes, but I think it is a fair limitation for a funnel analytics function. I
might work in the future on supporting a secondary set of correlations though,
in addition to a primary correlation (eg. correlate by user id + order id).
Depending on how that is implemented perhaps I could remove the limitation.
--
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
