github-actions[bot] commented on code in PR #68455:
URL: https://github.com/apache/doris/pull/68455#discussion_r4091103028


##########
fe/fe-core/src/main/java/org/apache/doris/nereids/rules/analysis/WindowFunctionChecker.java:
##########
@@ -236,8 +234,8 @@ private void checkFrameBoundOffset(FrameBoundary 
frameBoundary) {
         Preconditions.checkArgument(isPositive, "BoundOffset of WindowFrame 
must be positive");
 
         if (frameUnits == FrameUnitsType.ROWS) {
-            
Preconditions.checkArgument(offsetValue.compareTo(MAX_ROWS_OFFSET_VALUE) <= 0,
-                    "BoundOffset of ROWS WindowFrame must not exceed " + 
Long.MAX_VALUE);
+            
Preconditions.checkArgument(offsetValue.compareTo(WindowFrame.MAX_ROWS_OFFSET) 
<= 0,
+                    "BoundOffset of ROWS WindowFrame must not exceed " + 
WindowFrame.MAX_ROWS_OFFSET);

Review Comment:
   [P2] Preserve safe 64-bit PRECEDING frames
   
   This shared cap also rejects ordinary frames that do not enter the 
overflowing positive-end path. For example:
   
   ```text
   Window[SUM(k), ROWS start = -2147483648, end = 0]
     Sort(k)
       Values(1, 2)
   ```
   
   This is `ROWS BETWEEN 2147483648 PRECEDING AND CURRENT ROW`, which the 
previous i64 checks accepted. BE computes starts `-2147483648/-2147483647` and 
ends `1/2`, then clamps them to `[0,1)` and `[0,2)`, so the query safely 
returns `1, 3`; incremental removal and cleanup arithmetic also remain in 
range. The new FE rejects it while an old FE against the same BE still accepts 
it. Please keep the LEAD/LAG-specific policy separate, or make shared BE 
arithmetic checked/direction-aware so safe explicit PRECEDING frames retain 
their prior i64 range. The matching blanket guard in 
`AnalyticWindow.toThrift()` must be reconciled too, and this needs an accepted 
greater-than-INT32 PRECEDING regression case.



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