github-actions[bot] commented on code in PR #64624:
URL: https://github.com/apache/doris/pull/64624#discussion_r3557578436
##########
regression-test/conf/regression-conf.groovy:
##########
@@ -24,18 +24,18 @@ defaultDb = "regression_test"
// init cmd like: select @@session.tx_read_only
// at each time we connect.
// add allowLoadLocalInfile so that the jdbc can execute mysql load data from
client.
-jdbcUrl =
"jdbc:mysql://127.0.0.1:9030/?useLocalSessionState=true&allowLoadLocalInfile=true&zeroDateTimeBehavior=round"
-targetJdbcUrl =
"jdbc:mysql://127.0.0.1:9030/?useLocalSessionState=true&allowLoadLocalInfile=true&zeroDateTimeBehavior=round"
+jdbcUrl =
"jdbc:mysql://127.0.0.1:9330/?useLocalSessionState=true&allowLoadLocalInfile=true&zeroDateTimeBehavior=round"
Review Comment:
This file is the repository default regression harness config, but the PR
commits local runner settings unrelated to aggregate const-argument handling:
the default FE ports move from 9030/9020/8030 to 9330/9220/8330,
`max_failure_num` changes from unlimited (`0`) to stopping after one suite
failure, and `pythonUdfRuntimeVersion` is forced to 3.12.11. These values are
consumed directly by the framework when no override is supplied, so a normal
regression run can connect to the wrong cluster and broad runs will stop
collecting failures after the first suite. Please drop these local
`regression-conf.groovy` changes and keep any port/runtime overrides in
runner-specific custom config instead.
##########
fe/fe-core/src/main/java/org/apache/doris/nereids/rules/analysis/NormalizeRepeat.java:
##########
@@ -247,10 +247,9 @@ private static Set<Expression>
collectNeedToSlotArgsOfGroupingScalarFuncAndAggFu
ImmutableSet.Builder<Expression> argumentsOfAggregateFunctionBuilder =
ImmutableSet.builder();
for (AggregateFunction function : aggregateFunctions) {
for (Expression arg : function.getArguments()) {
- if (arg instanceof OrderExpression) {
- argumentsOfAggregateFunctionBuilder.add(arg.child(0));
- } else {
- argumentsOfAggregateFunctionBuilder.add(arg);
+ Expression realArg = arg instanceof OrderExpression ?
arg.child(0) : arg;
+ if (!realArg.isConstant()) {
Review Comment:
This fixes the repeat path, but the same constant-preservation rule is still
missing from the normal `NormalizeAggregate` path for distinct aggregates. That
rule still skips only `Literal`s; for a distinct call such as
`percentile_approx(DISTINCT v, CAST('0.5' AS DOUBLE))` (or an arithmetic
constant when constant folding is skipped), `FoldConstantRuleOnFE` deliberately
does not fold inside distinct aggregate functions, so the non-literal constant
is pushed below the aggregate and `normalizeAggFuncChildren()` replaces it with
a `SlotReference`. The new percentile legality/BE paths then no longer see the
required quantile argument as constant. Please apply this same `isConstant()`
preservation in `NormalizeAggregate`, including `OrderExpression` children, so
required-constant aggregate arguments are not slotized by the normal aggregate
path.
--
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]
