Copilot commented on code in PR #17348:
URL: https://github.com/apache/pinot/pull/17348#discussion_r2609814292
##########
pinot-query-runtime/src/test/java/org/apache/pinot/query/runtime/operator/WindowAggregateOperatorTest.java:
##########
@@ -637,6 +637,63 @@ public void testLeadLagWindowFunction2() {
assertTrue(operator.nextBlock().isSuccess(), "Second block is EOS (done
processing)");
}
+ @Test
+ public void testLeadLagWindowFunctionWithOffsetGreaterThanNumberOfRows() {
+ // Given: Test with offset much larger than partition size to verify
overflow handling
+ // Input should be in sorted order on the order by key as SortExchange
will handle pre-sorting the data
+ DataSchema inputSchema = new DataSchema(new String[]{"group", "arg"}, new
ColumnDataType[]{INT, STRING});
+ MultiStageOperator input = new
BlockListMultiStageOperator.Builder(inputSchema)
+ .addRow(1, "alpha")
+ .addRow(1, "beta")
+ .addRow(1, "gamma")
+ .addRow(2, "bar")
+ .addRow(2, "foo")
+ .addRow(3, "single")
+ .buildWithEos();
+ DataSchema resultSchema =
+ new DataSchema(
+ new String[]{"group", "arg", "lead_no_default", "lag_no_default",
"lead_with_default", "lag_with_default"},
+ new ColumnDataType[]{INT, STRING, INT, INT, INT, INT});
+ List<Integer> keys = List.of(0);
+ List<RelFieldCollation> collations =
+ List.of(new RelFieldCollation(1,
RelFieldCollation.Direction.ASCENDING, RelFieldCollation.NullDirection.LAST));
+ List<RexExpression.FunctionCall> aggCalls = List.of(
+ // LEAD with offset 1000, no default value - should return null
+ new RexExpression.FunctionCall(ColumnDataType.INT, SqlKind.LEAD.name(),
+ List.of(new RexExpression.InputRef(0), new
RexExpression.Literal(ColumnDataType.INT, 1000))),
+ // LAG with offset 1000, no default value - should return null
+ new RexExpression.FunctionCall(ColumnDataType.INT, SqlKind.LAG.name(),
+ List.of(new RexExpression.InputRef(0), new
RexExpression.Literal(ColumnDataType.INT, 1000))),
+ // LEAD with offset Integer.MAX_VALUE and default value 9999
+ new RexExpression.FunctionCall(ColumnDataType.INT, SqlKind.LEAD.name(),
+ List.of(new RexExpression.InputRef(0), new
RexExpression.Literal(ColumnDataType.INT, Integer.MAX_VALUE),
+ new RexExpression.Literal(ColumnDataType.INT, 9999))),
+ // LAG with offset Integer.MAX_VALUE and default value 8888
Review Comment:
The test uses `Integer.MAX_VALUE` as an offset, which is an extreme value
that could expose integer overflow issues in the underlying implementation.
While the fix uses `Math.max()` and `Math.min()` which should handle this
safely, consider adding an explicit test case with an offset that would cause
`numRows - _offset` to overflow (e.g., when `_offset > Integer.MAX_VALUE -
numRows`). This would verify that the boundary checks truly prevent arithmetic
overflow, not just array index errors.
--
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]
