kgyrtkirk commented on code in PR #16741:
URL: https://github.com/apache/druid/pull/16741#discussion_r1680860887
##########
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);
+ }
+
+ private static Iterable<AggInterval>
buildRowIteratorFor(AppendableRowsAndColumns rac, WindowFrame.Rows frame)
{
int[] groupBoundaries = new int[rac.numRows() + 1];
for (int j = 0; j < groupBoundaries.length; j++) {
groupBoundaries[j] = j;
}
+ if (isEffectivelyUnbounded(frame, groupBoundaries.length - 1)) {
+ return buildUnboundedIteratorFor(rac);
+ }
Review Comment:
this was added to catch more tricky unbounded cases ; however as we don't
support `group` in the sql - and restrict `range` to `unbounded`/`current row`
this won't make any difference right now...
removed this condition
--
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]
