github-actions[bot] commented on code in PR #65846:
URL: https://github.com/apache/doris/pull/65846#discussion_r3654281691
##########
fe/fe-core/src/main/java/org/apache/doris/nereids/rules/rewrite/SetPreAggStatus.java:
##########
@@ -315,39 +424,64 @@ private PreAggStatus createPreAggStatus(LogicalOlapScan
logicalOlapScan, PreAggI
return !aggregateFuncs.isEmpty() || !groupingExprs.isEmpty() ?
PreAggStatus.on()
: PreAggStatus.off("No aggregate on scan.");
} else {
- return checkAggregateFunctions(candidateAggFuncs,
candidateGroupByInputSlots);
+ return checkAggregateFunctions(candidateAggFuncs,
candidateGroupByInputSlots, outputSlots);
}
}
private PreAggStatus checkAggregateFunctions(Set<AggregateFunction>
aggregateFuncs,
- Set<Slot> groupingExprsInputSlots) {
+ Set<Slot> groupingExprsInputSlots, Set<Slot> outputSlots) {
if (aggregateFuncs.isEmpty() && groupingExprsInputSlots.isEmpty())
{
return PreAggStatus.off("No aggregate on scan.");
}
PreAggStatus preAggStatus = PreAggStatus.on();
for (AggregateFunction aggFunc : aggregateFuncs) {
- if (aggFunc.children().isEmpty()) {
+ Set<Slot> aggSlots = Sets.intersection(
+ aggFunc.getInputSlots(), outputSlots);
+ if (aggSlots.isEmpty()) {
preAggStatus = PreAggStatus.off(
String.format("can't turn preAgg on for aggregate
function %s", aggFunc));
- } else if (aggFunc.children().size() == 1 && aggFunc.child(0)
instanceof Slot) {
- Slot aggSlot = (Slot) aggFunc.child(0);
- if (aggSlot instanceof SlotReference
- && ((SlotReference)
aggSlot).getOriginalColumn().isPresent()) {
- if (((SlotReference)
aggSlot).getOriginalColumn().get().isKey()) {
- preAggStatus =
OneKeySlotAggChecker.INSTANCE.check(aggFunc);
+ } else {
+ Pair<Set<SlotReference>, Set<SlotReference>> splitSlots =
splitKeyValueSlots(aggSlots);
+ if (splitSlots.first.isEmpty()) {
Review Comment:
[P2] Keep the IF/CASE checker reachable for a value-only scan
In this reduced plan:
```text
Aggregate(sum(if(l.k1 > 0, r.v7, 0)))
Join(l.k1 = r.k1)
Scan(l AGG_KEYS)
Scan(r AGG_KEYS; v7 SUM)
```
the scan-relative split correctly turns `l` OFF, but for `r` the local set
is only `{r.v7}` and this branch rejects the IF because `child(0)` is not a
direct slot. Enabling `r` is safe: for each joined left row the foreign
condition is constant across one right key's partial rows, and SUM distributes
over those partial values. The added regression comment even says 'r can be ON'
but asserts only `l` is OFF. Please route unary IF/CASE shapes through an
ownership-aware version of the existing condition/return checker and assert `r`
ON (plus CASE symmetry). This is a different expression path from the existing
safe-cast thread.
--
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]
