mrhhsg opened a new pull request, #68488:
URL: https://github.com/apache/doris/pull/68488
### What problem does this PR solve?
Issue Number: None
Problem Summary:
`percentile_reservoir` validated its level argument only in
`checkLegalityBeforeTypeCoercion`, which runs during analysis before
constant folding. Besides requiring the argument to be constant, it also
required it to already be a `Literal`, so any constant expression that
only becomes a literal after folding was rejected:
```sql
SELECT percentile_reservoir(number, 0.25 + 0.25) FROM numbers('number' =
'10');
-- ERROR: percentile_reservoir requires second parameter must be a constant
```
The equivalent literal `0.5` works, and sibling functions such as
`percentile_approx` accept the same foldable expression, so the
restriction was inconsistent and unnecessary.
Both `checkLegalityBeforeTypeCoercion` and `checkLegalityAfterRewrite`
now share one check that folds the level itself with
`FoldConstantRuleOnFE.evaluateWithoutContext` (as `stack` already does
for its row count), casts the folded literal to DOUBLE and range checks
it. Folding inside the check instead of relying on the rewrite phase
matters because:
- a constant expression such as `0.25 + 0.25` is only a literal after
folding,
- `INSERT ... VALUES` and load column mappings never run the rewrite
phase, so an analysis-time check is the only one on those paths,
- `debug_skip_fold_constant` turns off the regular constant folding, and
a plain literal `0.5` would otherwise stay an unfolded cast.
Foldable constants such as `0.25 + 0.25`, `cast('0.5' as double)` or
`1 - 0.75` are now accepted for the plain aggregate, the window form,
the `_state` combinator and `INSERT ... VALUES`. A constant outside
`[0, 1]`, a non-constant argument and a constant that FE cannot fold
(for example `pow(0.5, 1)`) are still rejected with the same error
messages as before; `cast('NaN' as double)` is now rejected as out of
range instead of as a non-constant.
### Release note
None
### Check List (For Author)
- Test:
- Unit Test: `PercentileReservoirParameterTest` updated to cover
literal, foldable, unfoldable, non-constant, string and NULL levels
through both check phases.
- Regression test: new `test_percentile_reservoir_constant_level`
(query_p0/sql_functions/aggregate_functions) including the
`INSERT ... VALUES` path and `debug_skip_fold_constant = true`;
existing `test_aggregate_all_functions2`, `agg_distinct_function`,
`test_agg_state_parameters` and `test_agg_state_nullable_rewrite`
re-run locally.
- Behavior changed: Yes. Constant expressions that fold to a valid level
are accepted instead of raising an analysis error. A string literal
level is cast to DOUBLE before the range check (`'0.5'` is accepted as
0.5, `'5'` is rejected as out of range, a non-numeric string literal
is rejected as a cast error) instead of being compared as a
meaningless hash value. An explicit `cast('abc' as double)` level
folds to NULL under the default non-strict cast and is then treated
like a plain NULL level, which was already accepted.
- Does this need documentation: No
--
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]
