ankitsultana commented on code in PR #14631:
URL: https://github.com/apache/pinot/pull/14631#discussion_r1882702453
##########
pinot-timeseries/pinot-timeseries-planner/src/main/java/org/apache/pinot/tsdb/planner/TimeSeriesPlanFragmenter.java:
##########
@@ -102,8 +104,15 @@ public static List<BaseTimeSeriesPlanNode>
getFragments(BaseTimeSeriesPlanNode r
private static BaseTimeSeriesPlanNode
fragmentRecursively(BaseTimeSeriesPlanNode planNode, Context context) {
if (planNode instanceof LeafTimeSeriesPlanNode) {
LeafTimeSeriesPlanNode leafNode = (LeafTimeSeriesPlanNode) planNode;
- context._fragments.add(leafNode.withInputs(Collections.emptyList()));
- return new TimeSeriesExchangeNode(planNode.getId(),
Collections.emptyList(), leafNode.getAggInfo());
+ AggInfo currentAggInfo = leafNode.getAggInfo();
+ if (currentAggInfo == null) {
+ context._fragments.add(leafNode.withInputs(Collections.emptyList()));
+ } else {
+ Preconditions.checkState(!currentAggInfo.getIsPartial(),
+ "Leaf node in the logical plan should not have partial agg");
+
context._fragments.add(leafNode.withAggInfo(currentAggInfo.withPartialAggregation()));
Review Comment:
The leaf node created by the users should have agg-mode=full, but the leaf
node generated by Pinot should have partial aggregation since the
TimeSeriesExchangeNode will have the full aggregation.
There's a cleanup pending here: since users are returning Logical plan, we
shouldn't allow them to choose between partial or full aggregation. But
implementation wise it's a bit tricky.. but I should add a TODO for this (I
thought I already did).
--
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]
