kgyrtkirk commented on code in PR #16741:
URL: https://github.com/apache/druid/pull/16741#discussion_r1680862698
##########
processing/src/main/java/org/apache/druid/query/rowsandcols/semantic/DefaultFramedOnHeapAggregatable.java:
##########
@@ -106,31 +110,53 @@ public void appendTo(AppendableRowsAndColumns rac)
public static Iterable<AggInterval>
buildIteratorFor(AppendableRowsAndColumns rac, WindowFrame frame)
{
int numRows = rac.numRows();
- if (frame.getLowerOffsetClamped(numRows) == -numRows &&
frame.getUpperOffsetClamped(numRows) == numRows) {
- return buildUnboundedIteratorFor(rac, frame);
- } else if (frame.getPeerType() == WindowFrame.PeerType.RANGE) {
- return buildGroupIteratorFor(rac, frame);
- } else {
- return buildRowIteratorFor(rac, frame);
+ if (isEffectivelyUnbounded(frame, numRows)) {
+ return buildUnboundedIteratorFor(rac);
}
+ Rows rowsFrame = frame.unwrap(WindowFrame.Rows.class);
+ if (rowsFrame != null) {
+ return buildRowIteratorFor(rac, rowsFrame);
+ }
+ Groups groupsFrame = frame.unwrap(WindowFrame.Groups.class);
+ if (groupsFrame != null) {
+ return buildGroupIteratorFor(rac, groupsFrame);
+ }
+ throw DruidException.defensive("Unable to handle WindowFrame [%s]!",
frame);
}
- private static Iterable<AggInterval>
buildUnboundedIteratorFor(AppendableRowsAndColumns rac, WindowFrame frame)
+ private static boolean isEffectivelyUnbounded(WindowFrame frame, int numRows)
{
- int[] groupBoundaries = new int[]{0, rac.numRows()};
- return new GroupIteratorForWindowFrame(frame, groupBoundaries);
+ if (frame.unwrap(WindowFrame.Unbounded.class) != null) {
+ return true;
+ }
+ OffsetFrame offsetFrame = frame.unwrap(WindowFrame.OffsetFrame.class);
+ if (offsetFrame.getLowerOffsetClamped(numRows) == -numRows
+ && offsetFrame.getUpperOffsetClamped(numRows) == numRows) {
+ // regardless the actual mode; all rows will be inside the frame!
+ return true;
+ }
+ return false;
}
- private static Iterable<AggInterval>
buildRowIteratorFor(AppendableRowsAndColumns rac, WindowFrame frame)
+ private static Iterable<AggInterval>
buildUnboundedIteratorFor(AppendableRowsAndColumns rac)
+ {
+ int[] groupBoundaries = new int[] {0, rac.numRows()};
+ return new GroupIteratorForWindowFrame(WindowFrame.rows(0, 0),
groupBoundaries);
Review Comment:
yes; it could be anything as long as `current row` is inside :D
changed it to `WindowFrame.rows(null, null)` :D
--
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...@druid.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscr...@druid.apache.org
For additional commands, e-mail: commits-h...@druid.apache.org
