github-actions[bot] commented on code in PR #65846:
URL: https://github.com/apache/doris/pull/65846#discussion_r3654004954
##########
fe/fe-core/src/main/java/org/apache/doris/nereids/rules/rewrite/SetPreAggStatus.java:
##########
@@ -315,39 +424,53 @@ 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()) {
+ // only value slots
+ if (aggFunc.children().size() == 1 && aggFunc.child(0)
instanceof SlotReference) {
Review Comment:
[P2] Preserve safe value-column casts before the direct-slot gate
A reduced normalized plan is:
```text
Aggregate(max(x))
Project(CAST(v9 AS DOUBLE) AS x)
Scan(preagg_t1 AGG_KEYS; v9 MAX)
```
`addAggregateFunctions` expands `x` back to the cast, so this scan has only
the local value slot `v9`, but `child(0)` is not a `SlotReference` and this
branch returns OFF. Before this change the expression reached
`checkAggWithKeyAndValueSlots`, whose numeric-cast loop unwrapped it and
matched `v9`'s MAX aggregation type. That ON decision is safe for a monotone
widening cast (`max(cast(v9 as double))` commutes with storage MAX), so the new
gate causes an avoidable full storage merge. Please preserve the supported
safe-cast path (at least for MAX/MIN) and add a positive EXPLAIN case.
--
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]
