Skip to content

Commit 1d96c16

Browse files
dnhatnncordon
authored andcommitted
do not deduplicate intermediate attributes
1 parent 0a81875 commit 1d96c16

File tree

2 files changed

+8
-20
lines changed

2 files changed

+8
-20
lines changed

x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/planner/AbstractPhysicalOperationProviders.java

Lines changed: 4 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -40,10 +40,8 @@
4040

4141
import java.time.Duration;
4242
import java.util.ArrayList;
43-
import java.util.HashMap;
4443
import java.util.HashSet;
4544
import java.util.List;
46-
import java.util.Map;
4745
import java.util.Set;
4846
import java.util.function.Consumer;
4947

@@ -250,7 +248,6 @@ private record AggFunctionSupplierContext(AggregatorFunctionSupplier supplier, L
250248
private static class IntermediateInputs {
251249
private final List<Attribute> inputAttributes;
252250
private int nextOffset;
253-
private final Map<AggregateFunction, Integer> offsets = new HashMap<>();
254251

255252
IntermediateInputs(AggregateExec aggregateExec) {
256253
inputAttributes = aggregateExec.child().output();
@@ -259,12 +256,10 @@ private static class IntermediateInputs {
259256

260257
List<Attribute> nextInputAttributes(AggregateFunction af, boolean grouping) {
261258
int intermediateStateSize = AggregateMapper.intermediateStateDesc(af, grouping).size();
262-
int offset = offsets.computeIfAbsent(af, unused -> {
263-
int v = nextOffset;
264-
nextOffset += intermediateStateSize;
265-
return v;
266-
});
267-
return inputAttributes.subList(offset, offset + intermediateStateSize);
259+
int endOffset = nextOffset + intermediateStateSize;
260+
List<Attribute> attributes = inputAttributes.subList(nextOffset, endOffset);
261+
nextOffset = endOffset;
262+
return attributes;
268263
}
269264
}
270265

x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/planner/AggregateMapper.java

Lines changed: 4 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -13,7 +13,6 @@
1313
import org.elasticsearch.xpack.esql.EsqlIllegalArgumentException;
1414
import org.elasticsearch.xpack.esql.core.expression.Alias;
1515
import org.elasticsearch.xpack.esql.core.expression.Attribute;
16-
import org.elasticsearch.xpack.esql.core.expression.AttributeMap;
1716
import org.elasticsearch.xpack.esql.core.expression.Expression;
1817
import org.elasticsearch.xpack.esql.core.expression.FieldAttribute;
1918
import org.elasticsearch.xpack.esql.core.expression.MetadataAttribute;
@@ -23,9 +22,8 @@
2322
import org.elasticsearch.xpack.esql.core.type.DataType;
2423
import org.elasticsearch.xpack.esql.expression.function.aggregate.AggregateFunction;
2524

26-
import java.util.HashSet;
25+
import java.util.ArrayList;
2726
import java.util.List;
28-
import java.util.Set;
2927
import java.util.stream.Stream;
3028

3129
/**
@@ -42,17 +40,12 @@ public static List<NamedExpression> mapGrouping(List<? extends NamedExpression>
4240
}
4341

4442
private static List<NamedExpression> doMapping(List<? extends NamedExpression> aggregates, boolean grouping) {
45-
Set<Expression> seen = new HashSet<>();
46-
AttributeMap.Builder<NamedExpression> attrToExpressionsBuilder = AttributeMap.builder();
43+
List<NamedExpression> namedExpressions = new ArrayList<>();
4744
for (NamedExpression agg : aggregates) {
4845
Expression inner = Alias.unwrap(agg);
49-
if (seen.add(inner)) {
50-
for (var ne : computeEntryForAgg(agg.name(), inner, grouping)) {
51-
attrToExpressionsBuilder.put(ne.toAttribute(), ne);
52-
}
53-
}
46+
namedExpressions.addAll(computeEntryForAgg(agg.name(), inner, grouping));
5447
}
55-
return attrToExpressionsBuilder.build().values().stream().toList();
48+
return namedExpressions;
5649
}
5750

5851
public static List<IntermediateStateDesc> intermediateStateDesc(AggregateFunction fn, boolean grouping) {

0 commit comments

Comments
 (0)