github-actions[bot] commented on code in PR #63350:
URL: https://github.com/apache/doris/pull/63350#discussion_r3259418419
##########
fe/fe-core/src/main/java/org/apache/doris/nereids/trees/expressions/functions/agg/TopNArray.java:
##########
@@ -100,6 +101,19 @@ public void checkLegalityBeforeTypeCoercion() {
}
}
+ @Override
+ public void checkLegalityAfterRewrite() {
+ Expression topNCount = getArgument(1);
+ if (topNCount.isNullLiteral()) {
+ return;
+ }
+ if (!(topNCount instanceof Literal) || ((Literal)
topNCount).getDouble() <= 0) {
+ throw new AnalysisException(
+ "topn_array requires second parameter must be a constant
positive integer: "
Review Comment:
This is a distinct instance of the same overflow bug in `topn_array`.
Because the count argument is coerced to `IntegerType` before this check, a
positive literal such as `2147483648` is narrowed to a negative
`IntegerLiteral` and then rejected as non-positive. Please validate/reject the
original literal range before the narrowing cast, and add regression coverage
for `topn_array` positive out-of-range counts.
##########
fe/fe-core/src/main/java/org/apache/doris/nereids/trees/expressions/functions/agg/TopN.java:
##########
@@ -97,6 +98,19 @@ public void checkLegalityBeforeTypeCoercion() {
}
}
+ @Override
+ public void checkLegalityAfterRewrite() {
+ Expression topNCount = getArgument(1);
+ if (topNCount.isNullLiteral()) {
+ return;
+ }
+ if (!(topNCount instanceof Literal) || ((Literal)
topNCount).getDouble() <= 0) {
+ throw new AnalysisException(
+ "topn requires second parameter must be a constant
positive integer: "
Review Comment:
This has the same post-coercion overflow problem as the already reported
`TopNWeighted` case, but for `topn`. The second argument is coerced to
`IntegerType` before `checkLegalityAfterRewrite`, and literal coercion uses
`Number.intValue()`, so `topn(col, 2147483648)` reaches this check as
`IntegerLiteral(-2147483648)` and is reported as a non-positive value instead
of being handled as a positive out-of-range count. Please validate the original
literal range before narrowing, or explicitly reject out-of-range counts with
coverage for `topn`.
--
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]
