kgyrtkirk commented on code in PR #15365:
URL: https://github.com/apache/druid/pull/15365#discussion_r1400229729


##########
processing/src/main/java/org/apache/druid/query/rowsandcols/semantic/DefaultFramedOnHeapAggregatable.java:
##########
@@ -85,10 +88,234 @@ public RowsAndColumns aggregateAll(
         }
       }
     } else {
-      throw new UOE("RANGE peer groupings are unsupported");
+      return computeRangeAggregates(aggFactories, frame);
+    }
+  }
+
+  private RowsAndColumns computeRangeAggregates(
+      AggregatorFactory[] aggFactories,
+      WindowFrame frame)
+  {
+    RangeIteratorForWindow iter = new RangeIteratorForWindow(rac, frame);
+
+    int numRows = rac.numRows();
+    Object[][] results = new Object[aggFactories.length][numRows];
+
+    AggCell cell = new AggCell(rac, aggFactories);
+
+    for (Range xRange : iter) {
+
+      cell.moveTo(xRange.inputRows);
+      // TODO: if(xRange.outputRows.a ==0 && xRange.outputRows.b == numRows) { 
return Const };
+
+      // note: would be better with results.setX()?
+      cell.setOutputs(results, xRange.outputRows);
     }
+    return makeReturnRAC(aggFactories, results);
   }
 
+  static class RangeIteratorForWindow implements Iterable<Range>
+  {
+    private final int[] rangeToRowId;
+    private final int numRows;
+    private final int numRanges;
+    private final int lowerOffset;
+    private final int upperOffset;
+
+    public RangeIteratorForWindow(AppendableRowsAndColumns rac, WindowFrame 
frame)
+    {
+      assert (frame.getPeerType() == PeerType.RANGE);
+      rangeToRowId = 
ClusteredGroupPartitioner.fromRAC(rac).computeBoundaries(frame.getOrderByColNames());
+      numRows = rac.numRows();
+      numRanges = rangeToRowId.length - 1;
+      lowerOffset = frame.getLowerOffsetClamped(numRanges);
+      upperOffset = frame.getUpperOffsetClamped(numRanges) + 1;
+    }

Review Comment:
   I think its a good think to make the contract here that this class will 
provide an iterator based on the Frame for that given RAC
   
   passing `int[]` would make the contract more complicated
   
   I think this same call pattern can be reused to provide other iterators as 
well - so that it becomes the iterators job to select the right rowranges
   



-- 
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]

Reply via email to