github-actions[bot] commented on code in PR #68503:
URL: https://github.com/apache/doris/pull/68503#discussion_r4255590141
##########
fe/fe-core/src/main/java/org/apache/doris/nereids/trees/expressions/functions/window/Ntile.java:
##########
@@ -80,12 +82,27 @@ public void checkLegalityBeforeTypeCoercion() {
throw new AnalysisException(
"The bucket of NTILE must be a constant value: " +
this.toSql());
}
- if (buckets instanceof Literal) {
- if (((Literal) buckets).getDouble() <= 0) {
- throw new AnalysisException(
- "The bucket parameter of NTILE must be a constant positive
integer: " + this.toSql());
- }
- } else {
+ // The bucket may be a constant expression such as `1 + 1`, which is
folded to a literal only by the
+ // rewrite phase after this check runs. Reject early a bucket that FE
already evaluates to a non-positive
+ // or NULL value. A bucket FE can not evaluate, e.g. `3 % 2`, may
still be folded by BE when
+ // enable_fold_constant_by_be is set, so it is left to
checkLegalityAfterRewrite.
+ Expression evaluated = ExpressionUtils.foldConstantArgument(buckets);
+ if (evaluated instanceof Literal) {
+ checkPositiveBucket(evaluated);
+ }
+ }
+
+ @Override
+ public void checkLegalityAfterRewrite() {
+ // The backend reads the bucket from the argument column and registers
ntile only for a non-nullable
+ // integer argument, so the bucket must have been folded to a literal
before the plan is translated.
+ // That does not happen when constant folding is skipped, e.g. with
debug_skip_fold_constant=true, or
+ // when the configured folding can not evaluate the bucket.
+ checkPositiveBucket(getArgument(0));
Review Comment:
[P2] Preserve literal buckets through ORDER BY alias normalization
For a nonempty ordinary table `t` with nonunique `k1`, `SELECT 2 AS two, k1,
RAND() AS r FROM t ORDER BY NTILE(2) OVER (ORDER BY k1), r + r` reaches this
guard with `two#` rather than literal `2`. `ReplaceExpressionByChildOutput`
recursively replaces the bucket literal with the selected alias slot, and
`NormalizeSort` materializes the window above the selected projection.
`PlanUtils.canMergeWithProjections` correctly prevents merging that projection
because `r#` is used both in the output and in `r + r`, so the constant is not
inlined back into NTILE. The resulting relevant subtree is:
```text
Window(NTILE(two#) OVER (ORDER BY k1#))
Project(2 AS two#, k1#, RAND() AS r#)
Scan(t)
```
Later constant propagation does not restore arguments in `LogicalWindow`, so
the new literal-only check rejects this previously executable query even though
the non-nullable bucket column always contains `2`. Please preserve or restore
the validated constant argument through ORDER BY alias normalization before
applying the final guard, and add a regression with a matching selected
constant and a repeatedly used volatile projection. The unconditional
positive-bucket guard should remain.
--
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]
