diff --git a/x-pack/plugin/esql-core/src/main/java/org/elasticsearch/xpack/esql/core/expression/Attribute.java b/x-pack/plugin/esql-core/src/main/java/org/elasticsearch/xpack/esql/core/expression/Attribute.java index 829943d245149..32898bf976397 100644 --- a/x-pack/plugin/esql-core/src/main/java/org/elasticsearch/xpack/esql/core/expression/Attribute.java +++ b/x-pack/plugin/esql-core/src/main/java/org/elasticsearch/xpack/esql/core/expression/Attribute.java @@ -66,7 +66,7 @@ public Nullability nullable() { @Override public AttributeSet references() { - return new AttributeSet(this); + return AttributeSet.of(this); } public Attribute withLocation(Source source) { diff --git a/x-pack/plugin/esql-core/src/main/java/org/elasticsearch/xpack/esql/core/expression/AttributeMap.java b/x-pack/plugin/esql-core/src/main/java/org/elasticsearch/xpack/esql/core/expression/AttributeMap.java index 64a7b36cf1624..b04c14877d72d 100644 --- a/x-pack/plugin/esql-core/src/main/java/org/elasticsearch/xpack/esql/core/expression/AttributeMap.java +++ b/x-pack/plugin/esql-core/src/main/java/org/elasticsearch/xpack/esql/core/expression/AttributeMap.java @@ -16,6 +16,7 @@ import java.util.Map; import java.util.Set; import java.util.function.BiConsumer; +import java.util.function.Function; import java.util.stream.Stream; import static java.util.Collections.emptyMap; @@ -34,7 +35,7 @@ */ public final class AttributeMap implements Map { - static class AttributeWrapper { + private static class AttributeWrapper { private final Attribute attr; @@ -165,7 +166,7 @@ private AttributeMap(Map other) { delegate = other; } - public AttributeMap() { + private AttributeMap() { delegate = new LinkedHashMap<>(); } @@ -220,12 +221,22 @@ public boolean subsetOf(AttributeMap other) { return true; } - public void add(Attribute key, E value) { - put(key, value); + private E add(Attribute key, E value) { + return delegate.put(new AttributeWrapper(key), value); + } + + private void addAll(AttributeMap other) { + for (Entry entry : other.entrySet()) { + add(entry.getKey(), entry.getValue()); + } } - public void addAll(AttributeMap other) { - putAll(other); + private E addIfAbsent(Attribute key, Function mappingFunction) { + return delegate.computeIfAbsent(new AttributeWrapper(key), k -> mappingFunction.apply(k.attr)); + } + + private E delete(Object key) { + return key instanceof NamedExpression ne ? delegate.remove(new AttributeWrapper(ne.toAttribute())) : null; } public Set attributeNames() { @@ -249,7 +260,7 @@ public boolean isEmpty() { @Override public boolean containsKey(Object key) { - return key instanceof NamedExpression ne ? delegate.containsKey(new AttributeWrapper(ne.toAttribute())) : false; + return key instanceof NamedExpression ne && delegate.containsKey(new AttributeWrapper(ne.toAttribute())); } @Override @@ -293,24 +304,22 @@ public E resolve(Object key, E defaultValue) { @Override public E put(Attribute key, E value) { - return delegate.put(new AttributeWrapper(key), value); + throw new UnsupportedOperationException(); } @Override public void putAll(Map m) { - for (Entry entry : m.entrySet()) { - put(entry.getKey(), entry.getValue()); - } + throw new UnsupportedOperationException(); } @Override public E remove(Object key) { - return key instanceof NamedExpression ne ? delegate.remove(new AttributeWrapper(ne.toAttribute())) : null; + throw new UnsupportedOperationException(); } @Override public void clear() { - delegate.clear(); + throw new UnsupportedOperationException(); } @Override @@ -376,22 +385,23 @@ public String toString() { return delegate.toString(); } - public static Builder builder() { - return new Builder<>(); + public static AttributeMap of(Attribute key, E value) { + final AttributeMap map = new AttributeMap<>(); + map.add(key, value); + return map; } - public static Builder builder(AttributeMap map) { - return new Builder().putAll(map); + public static Builder builder() { + return new Builder<>(); } public static class Builder { - private AttributeMap map = new AttributeMap<>(); + private final AttributeMap map = new AttributeMap<>(); private Builder() {} - public Builder put(Attribute attr, E value) { - map.add(attr, value); - return this; + public E put(Attribute attr, E value) { + return map.add(attr, value); } public Builder putAll(AttributeMap m) { @@ -399,6 +409,26 @@ public Builder putAll(AttributeMap m) { return this; } + public E computeIfAbsent(Attribute key, Function mappingFunction) { + return map.addIfAbsent(key, mappingFunction); + } + + public E remove(Object o) { + return map.delete(o); + } + + public Set keySet() { + return map.keySet(); + } + + public boolean containsKey(Object key) { + return map.containsKey(key); + } + + public boolean isEmpty() { + return map.isEmpty(); + } + public AttributeMap build() { return map; } diff --git a/x-pack/plugin/esql-core/src/main/java/org/elasticsearch/xpack/esql/core/expression/AttributeSet.java b/x-pack/plugin/esql-core/src/main/java/org/elasticsearch/xpack/esql/core/expression/AttributeSet.java index 8a075e8887512..5bcb8fdd0ec5e 100644 --- a/x-pack/plugin/esql-core/src/main/java/org/elasticsearch/xpack/esql/core/expression/AttributeSet.java +++ b/x-pack/plugin/esql-core/src/main/java/org/elasticsearch/xpack/esql/core/expression/AttributeSet.java @@ -26,22 +26,6 @@ public class AttributeSet implements Set { private final AttributeMap delegate; - public AttributeSet() { - delegate = new AttributeMap<>(); - } - - public AttributeSet(Attribute attr) { - delegate = new AttributeMap<>(attr, PRESENT); - } - - public AttributeSet(Collection attr) { - delegate = new AttributeMap<>(); - - for (Attribute a : attr) { - delegate.add(a, PRESENT); - } - } - private AttributeSet(AttributeMap delegate) { this.delegate = delegate; } @@ -113,44 +97,32 @@ public T[] toArray(T[] a) { @Override public boolean add(Attribute e) { - return delegate.put(e, PRESENT) == null; + throw new UnsupportedOperationException(); } @Override public boolean remove(Object o) { - return delegate.remove(o) != null; - } - - public void addAll(AttributeSet other) { - delegate.addAll(other.delegate); + throw new UnsupportedOperationException(); } @Override public boolean addAll(Collection c) { - int size = delegate.size(); - for (var e : c) { - delegate.put(e, PRESENT); - } - return delegate.size() != size; + throw new UnsupportedOperationException(); } @Override public boolean retainAll(Collection c) { - return delegate.keySet().removeIf(e -> c.contains(e) == false); + throw new UnsupportedOperationException(); } @Override public boolean removeAll(Collection c) { - int size = delegate.size(); - for (var e : c) { - delegate.remove(e); - } - return delegate.size() != size; + throw new UnsupportedOperationException(); } @Override public void clear() { - delegate.clear(); + throw new UnsupportedOperationException(); } @Override @@ -160,7 +132,7 @@ public Spliterator spliterator() { @Override public boolean removeIf(Predicate filter) { - return delegate.keySet().removeIf(filter); + throw new UnsupportedOperationException(); } @Override @@ -191,4 +163,76 @@ public int hashCode() { public String toString() { return delegate.keySet().toString(); } + + public Builder asBuilder() { + return new Builder().addAll(this); + } + + public static AttributeSet of(Attribute... attrs) { + final AttributeMap.Builder mapBuilder = AttributeMap.builder(); + for (var a : attrs) { + mapBuilder.put(a, PRESENT); + } + return new AttributeSet(mapBuilder.build()); + } + + public static AttributeSet of(Collection c) { + final AttributeMap.Builder mapBuilder = AttributeMap.builder(); + for (var a : c) { + mapBuilder.put(a, PRESENT); + } + return new AttributeSet(mapBuilder.build()); + } + + public static Builder builder() { + return new Builder(); + } + + public static class Builder { + private final AttributeMap.Builder mapBuilder = AttributeMap.builder(); + + private Builder() {} + + public Builder add(Attribute attr) { + mapBuilder.put(attr, PRESENT); + return this; + } + + public boolean remove(Object o) { + return mapBuilder.remove(o) != null; + } + + public Builder addAll(AttributeSet other) { + mapBuilder.putAll(other.delegate); + return this; + } + + public Builder addAll(Builder other) { + mapBuilder.putAll(other.mapBuilder.build()); + return this; + } + + public Builder addAll(Collection c) { + for (var e : c) { + mapBuilder.put(e, PRESENT); + } + return this; + } + + public boolean removeIf(Predicate filter) { + return mapBuilder.keySet().removeIf(filter); + } + + public boolean contains(Object o) { + return mapBuilder.containsKey(o); + } + + public boolean isEmpty() { + return mapBuilder.isEmpty(); + } + + public AttributeSet build() { + return new AttributeSet(mapBuilder.build()); + } + } } diff --git a/x-pack/plugin/esql-core/src/main/java/org/elasticsearch/xpack/esql/core/expression/Expressions.java b/x-pack/plugin/esql-core/src/main/java/org/elasticsearch/xpack/esql/core/expression/Expressions.java index 739333ded0fde..ee7ff9e21db27 100644 --- a/x-pack/plugin/esql-core/src/main/java/org/elasticsearch/xpack/esql/core/expression/Expressions.java +++ b/x-pack/plugin/esql-core/src/main/java/org/elasticsearch/xpack/esql/core/expression/Expressions.java @@ -40,11 +40,11 @@ public static AttributeMap asAttributeMap(List map = new AttributeMap<>(); + AttributeMap.Builder mapBuilder = AttributeMap.builder(); for (NamedExpression exp : named) { - map.add(exp.toAttribute(), exp); + mapBuilder.put(exp.toAttribute(), exp); } - return map; + return mapBuilder.build(); } public static boolean anyMatch(List exps, Predicate predicate) { @@ -121,11 +121,11 @@ public static AttributeSet references(List exps) { return AttributeSet.EMPTY; } - AttributeSet set = new AttributeSet(); + var setBuilder = AttributeSet.builder(); for (Expression exp : exps) { - set.addAll(exp.references()); + setBuilder.addAll(exp.references()); } - return set; + return setBuilder.build(); } public static String name(Expression e) { diff --git a/x-pack/plugin/esql-core/src/test/java/org/elasticsearch/xpack/esql/core/expression/AttributeMapTests.java b/x-pack/plugin/esql-core/src/test/java/org/elasticsearch/xpack/esql/core/expression/AttributeMapTests.java index ade79c8168076..b22121cd8246d 100644 --- a/x-pack/plugin/esql-core/src/test/java/org/elasticsearch/xpack/esql/core/expression/AttributeMapTests.java +++ b/x-pack/plugin/esql-core/src/test/java/org/elasticsearch/xpack/esql/core/expression/AttributeMapTests.java @@ -130,7 +130,7 @@ private Alias createIntParameterAlias(int index, int value) { } public void testEmptyConstructor() { - AttributeMap m = new AttributeMap<>(); + AttributeMap m = AttributeMap.builder().build(); assertThat(m.size(), is(0)); assertThat(m.isEmpty(), is(true)); } @@ -156,7 +156,7 @@ public void testBuilder() { public void testSingleItemConstructor() { Attribute one = a("one"); - AttributeMap m = new AttributeMap<>(one, "one"); + AttributeMap m = AttributeMap.of(one, "one"); assertThat(m.size(), is(1)); assertThat(m.isEmpty(), is(false)); @@ -168,8 +168,8 @@ public void testSingleItemConstructor() { public void testSubtract() { AttributeMap m = threeMap(); - AttributeMap mo = new AttributeMap<>(m.keySet().iterator().next(), "one"); - AttributeMap empty = new AttributeMap<>(); + AttributeMap mo = AttributeMap.of(m.keySet().iterator().next(), "one"); + AttributeMap empty = AttributeMap.emptyAttributeMap(); assertThat(m.subtract(empty), is(m)); assertThat(m.subtract(m), is(empty)); @@ -184,7 +184,7 @@ public void testSubtract() { public void testIntersect() { AttributeMap m = threeMap(); AttributeMap mo = new AttributeMap<>(m.keySet().iterator().next(), "one"); - AttributeMap empty = new AttributeMap<>(); + AttributeMap empty = AttributeMap.emptyAttributeMap(); assertThat(m.intersect(empty), is(empty)); assertThat(m.intersect(m), is(m)); @@ -194,7 +194,7 @@ public void testIntersect() { public void testSubsetOf() { AttributeMap m = threeMap(); AttributeMap mo = new AttributeMap<>(m.keySet().iterator().next(), "one"); - AttributeMap empty = new AttributeMap<>(); + AttributeMap empty = AttributeMap.emptyAttributeMap(); assertThat(m.subsetOf(empty), is(false)); assertThat(m.subsetOf(m), is(true)); @@ -254,37 +254,49 @@ public void testCopy() { public void testEmptyMapIsImmutable() { var empty = AttributeMap.emptyAttributeMap(); - var ex = expectThrows(UnsupportedOperationException.class, () -> empty.add(a("one"), new Object())); + expectThrows(UnsupportedOperationException.class, () -> empty.put(a("one"), new Object())); + } + + public void testMapIsImmutable() { + var map = threeMap(); + expectThrows(UnsupportedOperationException.class, () -> map.put(a("one"), "one")); + expectThrows(UnsupportedOperationException.class, () -> map.putAll(threeMap())); + expectThrows(UnsupportedOperationException.class, () -> map.remove(a("one"))); + expectThrows(UnsupportedOperationException.class, map::clear); } public void testAddPutEntriesIntoMap() { - var map = new AttributeMap(); + var builder = AttributeMap.builder(); var one = a("one"); var two = a("two"); var three = a("three"); for (var i : asList(one, two, three)) { - map.add(i, i.name()); + builder.put(i, i.name()); } + var map = builder.build(); + assertThat(map.size(), is(3)); - assertThat(map.remove(one), is("one")); - assertThat(map.remove(two), is("two")); + assertThat(builder.remove(one), is("one")); + assertThat(builder.remove(two), is("two")); assertThat(map.size(), is(1)); } public void testKeyIteratorRemoval() { - var map = new AttributeMap(); + var builder = AttributeMap.builder(); var one = a("one"); var two = a("two"); var three = a("three"); for (var i : asList(one, two, three)) { - map.add(i, i.name()); + builder.put(i, i.name()); } + var map = builder.build(); + assertThat(map.attributeNames(), contains("one", "two", "three")); assertThat(map.size(), is(3)); @@ -305,15 +317,17 @@ public void testKeyIteratorRemoval() { } public void testValuesIteratorRemoval() { - var map = new AttributeMap(); + AttributeMap.Builder builder = AttributeMap.builder(); var one = a("one"); var two = a("two"); var three = a("three"); for (var i : asList(one, two, three)) { - map.add(i, i.name()); + builder.put(i, i.name()); } + var map = builder.build(); + assertThat(map.values(), contains("one", "two", "three")); map.values().removeIf(v -> v.contains("o")); diff --git a/x-pack/plugin/esql-core/src/test/java/org/elasticsearch/xpack/esql/core/expression/AttributeSetTests.java b/x-pack/plugin/esql-core/src/test/java/org/elasticsearch/xpack/esql/core/expression/AttributeSetTests.java index 0e97773fb90d2..e7ff089631f02 100644 --- a/x-pack/plugin/esql-core/src/test/java/org/elasticsearch/xpack/esql/core/expression/AttributeSetTests.java +++ b/x-pack/plugin/esql-core/src/test/java/org/elasticsearch/xpack/esql/core/expression/AttributeSetTests.java @@ -8,8 +8,6 @@ import org.elasticsearch.test.ESTestCase; -import java.util.List; - import static org.elasticsearch.xpack.esql.core.expression.AttributeMapTests.a; public class AttributeSetTests extends ESTestCase { @@ -18,20 +16,22 @@ public void testEquals() { Attribute a1 = a("1"); Attribute a2 = a("2"); - AttributeSet first = new AttributeSet(List.of(a1, a2)); + AttributeSet first = AttributeSet.of(a1, a2); assertEquals(first, first); - AttributeSet second = new AttributeSet(); - second.add(a1); - second.add(a2); + var secondBuilder = AttributeSet.builder(); + secondBuilder.add(a1); + secondBuilder.add(a2); + var second = secondBuilder.build(); assertEquals(first, second); assertEquals(second, first); - AttributeSet third = new AttributeSet(); - third.add(a("1")); - third.add(a("2")); + var thirdBuilder = AttributeSet.builder(); + thirdBuilder.add(a("1")); + thirdBuilder.add(a("2")); + AttributeSet third = thirdBuilder.build(); assertNotEquals(first, third); assertNotEquals(third, first); @@ -39,4 +39,15 @@ public void testEquals() { assertEquals(AttributeSet.EMPTY, first.intersect(third)); assertEquals(third.intersect(first), AttributeSet.EMPTY); } + + public void testSetIsImmutable() { + AttributeSet set = AttributeSet.of(a("1"), a("2")); + expectThrows(UnsupportedOperationException.class, () -> set.add(a("3"))); + expectThrows(UnsupportedOperationException.class, () -> set.remove(a("1"))); + expectThrows(UnsupportedOperationException.class, () -> set.addAll(set)); + expectThrows(UnsupportedOperationException.class, () -> set.retainAll(set)); + expectThrows(UnsupportedOperationException.class, () -> set.removeAll(set)); + expectThrows(UnsupportedOperationException.class, set::clear); + expectThrows(UnsupportedOperationException.class, () -> set.removeIf((x -> true))); + } } diff --git a/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/optimizer/rules/logical/CombineProjections.java b/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/optimizer/rules/logical/CombineProjections.java index 957db4a7273e5..0371510d6f306 100644 --- a/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/optimizer/rules/logical/CombineProjections.java +++ b/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/optimizer/rules/logical/CombineProjections.java @@ -96,7 +96,7 @@ private static List projectAggregations( List upperProjection, List lowerAggregations ) { - AttributeSet seen = new AttributeSet(); + AttributeSet.Builder seen = AttributeSet.builder(); for (NamedExpression upper : upperProjection) { Expression unwrapped = Alias.unwrap(upper); // projection contains an inner alias (point to an existing fields inside the projection) @@ -117,20 +117,21 @@ private static List projectAggregations( private static List combineProjections(List upper, List lower) { // collect named expressions declaration in the lower list - AttributeMap namedExpressions = new AttributeMap<>(); + AttributeMap.Builder namedExpressionsBuilder = AttributeMap.builder(); // while also collecting the alias map for resolving the source (f1 = 1, f2 = f1, etc..) - AttributeMap aliases = new AttributeMap<>(); + AttributeMap.Builder aliasesBuilder = AttributeMap.builder(); for (NamedExpression ne : lower) { // record the alias - aliases.put(ne.toAttribute(), Alias.unwrap(ne)); + aliasesBuilder.put(ne.toAttribute(), Alias.unwrap(ne)); // record named expression as is if (ne instanceof Alias as) { Expression child = as.child(); - namedExpressions.put(ne.toAttribute(), as.replaceChild(aliases.resolve(child, child))); + namedExpressionsBuilder.put(ne.toAttribute(), as.replaceChild(aliasesBuilder.build().resolve(child, child))); } } List replaced = new ArrayList<>(); + var namedExpressions = namedExpressionsBuilder.build(); // replace any matching attribute with a lower alias (if there's a match) // but clean-up non-top aliases at the end @@ -149,12 +150,13 @@ private static List combineUpperGroupingsAndLowerProjections( || upperGroupings.stream().anyMatch(group -> group.anyMatch(expr -> expr instanceof Categorize)) == false : "CombineProjections only tested with a single CATEGORIZE with no additional groups"; // Collect the alias map for resolving the source (f1 = 1, f2 = f1, etc..) - AttributeMap aliases = new AttributeMap<>(); + AttributeMap.Builder aliasesBuilder = AttributeMap.builder(); for (NamedExpression ne : lowerProjections) { // Record the aliases. // Projections are just aliases for attributes, so casting is safe. - aliases.put(ne.toAttribute(), (Attribute) Alias.unwrap(ne)); + aliasesBuilder.put(ne.toAttribute(), (Attribute) Alias.unwrap(ne)); } + var aliases = aliasesBuilder.build(); // Propagate any renames from the lower projection into the upper groupings. // This can lead to duplicates: e.g. @@ -180,18 +182,19 @@ private List replacePrunedAliasesUsedInGroupBy( List oldAggs, List newAggs ) { - AttributeMap removedAliases = new AttributeMap<>(); - AttributeSet currentAliases = new AttributeSet(Expressions.asAttributes(newAggs)); + AttributeMap.Builder removedAliasesBuilder = AttributeMap.builder(); + AttributeSet currentAliases = AttributeSet.of(Expressions.asAttributes(newAggs)); // record only removed aliases for (NamedExpression ne : oldAggs) { if (ne instanceof Alias alias) { var attr = ne.toAttribute(); if (currentAliases.contains(attr) == false) { - removedAliases.put(attr, alias.child()); + removedAliasesBuilder.put(attr, alias.child()); } } } + var removedAliases = removedAliasesBuilder.build(); if (removedAliases.isEmpty()) { return groupings; diff --git a/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/optimizer/rules/logical/PropagateEvalFoldables.java b/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/optimizer/rules/logical/PropagateEvalFoldables.java index 66cdc992a91cb..5e094ce2138df 100644 --- a/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/optimizer/rules/logical/PropagateEvalFoldables.java +++ b/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/optimizer/rules/logical/PropagateEvalFoldables.java @@ -26,9 +26,9 @@ public final class PropagateEvalFoldables extends ParameterizedRule(); + AttributeMap.Builder collectRefsBuilder = AttributeMap.builder(); - java.util.function.Function replaceReference = r -> collectRefs.resolve(r, r); + java.util.function.Function replaceReference = r -> collectRefsBuilder.build().resolve(r, r); // collect aliases bottom-up plan.forEachExpressionUp(Alias.class, a -> { @@ -40,10 +40,10 @@ public LogicalPlan apply(LogicalPlan plan, LogicalOptimizerContext ctx) { shouldCollect = c.foldable(); } if (shouldCollect) { - collectRefs.put(a.toAttribute(), Literal.of(ctx.foldCtx(), c)); + collectRefsBuilder.put(a.toAttribute(), Literal.of(ctx.foldCtx(), c)); } }); - if (collectRefs.isEmpty()) { + if (collectRefsBuilder.isEmpty()) { return plan; } diff --git a/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/optimizer/rules/logical/PropgateUnmappedFields.java b/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/optimizer/rules/logical/PropgateUnmappedFields.java index 570b5b7e82be6..6163a50a42ea4 100644 --- a/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/optimizer/rules/logical/PropgateUnmappedFields.java +++ b/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/optimizer/rules/logical/PropgateUnmappedFields.java @@ -27,12 +27,13 @@ public LogicalPlan apply(LogicalPlan logicalPlan) { if (logicalPlan instanceof EsRelation) { return logicalPlan; } - var unmappedFields = new AttributeSet(); + var unmappedFieldsBuilder = AttributeSet.builder(); logicalPlan.forEachExpressionDown(FieldAttribute.class, fa -> { if (fa.field() instanceof PotentiallyUnmappedKeywordEsField) { - unmappedFields.add(fa); + unmappedFieldsBuilder.add(fa); } }); + var unmappedFields = unmappedFieldsBuilder.build(); return unmappedFields.isEmpty() ? logicalPlan : logicalPlan.transformUp( diff --git a/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/optimizer/rules/logical/PruneColumns.java b/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/optimizer/rules/logical/PruneColumns.java index 512a7253b813f..b6a1a40462d13 100644 --- a/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/optimizer/rules/logical/PruneColumns.java +++ b/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/optimizer/rules/logical/PruneColumns.java @@ -35,7 +35,7 @@ public final class PruneColumns extends Rule { @Override public LogicalPlan apply(LogicalPlan plan) { // track used references - var used = plan.outputSet(); + var used = plan.outputSet().asBuilder(); // while going top-to-bottom (upstream) var pl = plan.transformDown(p -> { // Note: It is NOT required to do anything special for binary plans like JOINs. It is perfectly fine that transformDown descends @@ -126,7 +126,7 @@ public LogicalPlan apply(LogicalPlan plan) { * Prunes attributes from the list not found in the given set. * Returns null if no changed occurred. */ - private static List removeUnused(List named, AttributeSet used) { + private static List removeUnused(List named, AttributeSet.Builder used) { var clone = new ArrayList<>(named); var it = clone.listIterator(clone.size()); diff --git a/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/optimizer/rules/logical/PushDownAndCombineFilters.java b/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/optimizer/rules/logical/PushDownAndCombineFilters.java index a5f7ea326eb34..f1f139bc2b0f2 100644 --- a/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/optimizer/rules/logical/PushDownAndCombineFilters.java +++ b/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/optimizer/rules/logical/PushDownAndCombineFilters.java @@ -68,11 +68,11 @@ protected LogicalPlan rule(Filter filter) { plan = maybePushDownPastUnary(filter, eval, evalAliases::containsKey, resolveRenames); } else if (child instanceof RegexExtract re) { // Push down filters that do not rely on attributes created by RegexExtract - var attributes = new AttributeSet(Expressions.asAttributes(re.extractedFields())); + var attributes = AttributeSet.of(Expressions.asAttributes(re.extractedFields())); plan = maybePushDownPastUnary(filter, re, attributes::contains, NO_OP); } else if (child instanceof Enrich enrich) { // Push down filters that do not rely on attributes created by Enrich - var attributes = new AttributeSet(Expressions.asAttributes(enrich.enrichFields())); + var attributes = AttributeSet.of(Expressions.asAttributes(enrich.enrichFields())); plan = maybePushDownPastUnary(filter, enrich, attributes::contains, NO_OP); } else if (child instanceof Project) { return PushDownUtils.pushDownPastProject(filter); diff --git a/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/optimizer/rules/logical/PushDownUtils.java b/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/optimizer/rules/logical/PushDownUtils.java index 6c8caaf783f43..6d374892ddc60 100644 --- a/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/optimizer/rules/logical/PushDownUtils.java +++ b/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/optimizer/rules/logical/PushDownUtils.java @@ -149,13 +149,13 @@ private static AttributeReplacement renameAttributesInExpressions( Set attributeNamesToRename, List expressions ) { - AttributeMap aliasesForReplacedAttributes = new AttributeMap<>(); + AttributeMap.Builder aliasesForReplacedAttributesBuilder = AttributeMap.builder(); List rewrittenExpressions = new ArrayList<>(); for (Expression expr : expressions) { rewrittenExpressions.add(expr.transformUp(Attribute.class, attr -> { if (attributeNamesToRename.contains(attr.name())) { - Alias renamedAttribute = aliasesForReplacedAttributes.computeIfAbsent(attr, a -> { + Alias renamedAttribute = aliasesForReplacedAttributesBuilder.computeIfAbsent(attr, a -> { String tempName = TemporaryNameUtils.locallyUniqueTemporaryName(a.name(), "temp_name"); return new Alias(a.source(), tempName, a, null, true); }); @@ -166,7 +166,7 @@ private static AttributeReplacement renameAttributesInExpressions( })); } - return new AttributeReplacement(rewrittenExpressions, aliasesForReplacedAttributes); + return new AttributeReplacement(rewrittenExpressions, aliasesForReplacedAttributesBuilder.build()); } private static Map newNamesForConflictingAttributes( diff --git a/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/optimizer/rules/logical/ReplaceAggregateAggExpressionWithEval.java b/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/optimizer/rules/logical/ReplaceAggregateAggExpressionWithEval.java index c36d4caf7f599..46e47d097d3fd 100644 --- a/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/optimizer/rules/logical/ReplaceAggregateAggExpressionWithEval.java +++ b/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/optimizer/rules/logical/ReplaceAggregateAggExpressionWithEval.java @@ -51,8 +51,9 @@ public ReplaceAggregateAggExpressionWithEval() { @Override protected LogicalPlan rule(Aggregate aggregate) { // build alias map - AttributeMap aliases = new AttributeMap<>(); - aggregate.forEachExpressionUp(Alias.class, a -> aliases.put(a.toAttribute(), a.child())); + AttributeMap.Builder aliasesBuilder = AttributeMap.builder(); + aggregate.forEachExpressionUp(Alias.class, a -> aliasesBuilder.put(a.toAttribute(), a.child())); + var aliases = aliasesBuilder.build(); // Build Categorize grouping functions map. // Functions like BUCKET() shouldn't reach this point, diff --git a/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/optimizer/rules/logical/ReplaceAliasingEvalWithProject.java b/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/optimizer/rules/logical/ReplaceAliasingEvalWithProject.java index dc9421a22a69c..bd506862108ee 100644 --- a/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/optimizer/rules/logical/ReplaceAliasingEvalWithProject.java +++ b/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/optimizer/rules/logical/ReplaceAliasingEvalWithProject.java @@ -54,9 +54,9 @@ private LogicalPlan rule(Eval eval) { LogicalPlan plan = eval; // holds simple aliases such as b = a, c = b, d = c - AttributeMap basicAliases = new AttributeMap<>(); + AttributeMap.Builder basicAliasesBuilder = AttributeMap.builder(); // same as above but keeps the original expression - AttributeMap basicAliasSources = new AttributeMap<>(); + AttributeMap.Builder basicAliasSourcesBuilder = AttributeMap.builder(); List keptFields = new ArrayList<>(); @@ -67,22 +67,23 @@ private LogicalPlan rule(Eval eval) { var attribute = field.toAttribute(); // put the aliases in a separate map to separate the underlying resolve from other aliases if (child instanceof Attribute) { - basicAliases.put(attribute, child); - basicAliasSources.put(attribute, field); + basicAliasesBuilder.put(attribute, child); + basicAliasSourcesBuilder.put(attribute, field); } else { // be lazy and start replacing name aliases only if needed - if (basicAliases.size() > 0) { + if (basicAliasesBuilder.build().size() > 0) { // update the child through the field - field = (Alias) field.transformUp(e -> basicAliases.resolve(e, e)); + field = (Alias) field.transformUp(e -> basicAliasesBuilder.build().resolve(e, e)); } keptFields.add(field); } } // at least one alias encountered, move it into a project - if (basicAliases.size() > 0) { + if (basicAliasesBuilder.build().size() > 0) { // preserve the eval output (takes care of shadowing and order) but replace the basic aliases List projections = new ArrayList<>(eval.output()); + var basicAliasSources = basicAliasSourcesBuilder.build(); // replace the removed aliases with their initial definition - however use the output to preserve the shadowing for (int i = projections.size() - 1; i >= 0; i--) { NamedExpression project = projections.get(i); diff --git a/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/optimizer/rules/logical/TranslateMetricsAggregate.java b/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/optimizer/rules/logical/TranslateMetricsAggregate.java index 5f34899875efd..172cbc945aa6f 100644 --- a/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/optimizer/rules/logical/TranslateMetricsAggregate.java +++ b/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/optimizer/rules/logical/TranslateMetricsAggregate.java @@ -215,7 +215,7 @@ LogicalPlan translate(Aggregate metrics) { private static Aggregate toStandardAggregate(Aggregate metrics) { final LogicalPlan child = metrics.child().transformDown(EsRelation.class, r -> { - var attributes = new ArrayList<>(new AttributeSet(metrics.inputSet())); + var attributes = new ArrayList<>(AttributeSet.of(metrics.inputSet())); attributes.removeIf(a -> a.name().equals(MetadataAttribute.TSID_FIELD)); if (attributes.stream().noneMatch(a -> a.name().equals(MetadataAttribute.TIMESTAMP_FIELD))) { attributes.removeIf(a -> a.name().equals(MetadataAttribute.TIMESTAMP_FIELD)); diff --git a/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/optimizer/rules/logical/local/InferIsNotNull.java b/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/optimizer/rules/logical/local/InferIsNotNull.java index f27be368f12b4..18159c112914e 100644 --- a/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/optimizer/rules/logical/local/InferIsNotNull.java +++ b/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/optimizer/rules/logical/local/InferIsNotNull.java @@ -47,17 +47,17 @@ public class InferIsNotNull extends Rule { @Override public LogicalPlan apply(LogicalPlan plan) { // the alias map is shared across the whole plan - AttributeMap aliases = new AttributeMap<>(); + AttributeMap.Builder aliasesBuilder = AttributeMap.builder(); // traverse bottom-up to pick up the aliases as we go - plan = plan.transformUp(p -> inspectPlan(p, aliases)); + plan = plan.transformUp(p -> inspectPlan(p, aliasesBuilder)); return plan; } - private LogicalPlan inspectPlan(LogicalPlan plan, AttributeMap aliases) { + private LogicalPlan inspectPlan(LogicalPlan plan, AttributeMap.Builder aliasesBuilder) { // inspect just this plan properties - plan.forEachExpression(Alias.class, a -> aliases.put(a.toAttribute(), a.child())); + plan.forEachExpression(Alias.class, a -> aliasesBuilder.put(a.toAttribute(), a.child())); // now go about finding isNull/isNotNull - LogicalPlan newPlan = plan.transformExpressionsOnlyUp(IsNotNull.class, inn -> inferNotNullable(inn, aliases)); + LogicalPlan newPlan = plan.transformExpressionsOnlyUp(IsNotNull.class, inn -> inferNotNullable(inn, aliasesBuilder.build())); return newPlan; } diff --git a/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/optimizer/rules/logical/local/ReplaceMissingFieldWithNull.java b/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/optimizer/rules/logical/local/ReplaceMissingFieldWithNull.java index 4d24197de4c7c..64512351644b4 100644 --- a/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/optimizer/rules/logical/local/ReplaceMissingFieldWithNull.java +++ b/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/optimizer/rules/logical/local/ReplaceMissingFieldWithNull.java @@ -42,7 +42,7 @@ public class ReplaceMissingFieldWithNull extends ParameterizedRule { // Looking only for indices in LOOKUP mode is correct: during parsing, we assign the expected mode and even if a lookup index // is used in the FROM command, it will not be marked with LOOKUP mode there - but STANDARD. @@ -51,14 +51,14 @@ public LogicalPlan apply(LogicalPlan plan, LocalLogicalOptimizerContext localLog // we're inside the right (or left) branch of a JOIN node. (See PlannerUtils.localPlan - this looks for FragmentExecs and // performs local logical optimization of the fragments; the right hand side of a LookupJoinExec can be a FragmentExec.) if (esRelation.indexMode() == IndexMode.LOOKUP) { - lookupFields.addAll(esRelation.output()); + lookupFieldsBuilder.addAll(esRelation.output()); } }); // Do not use the attribute name, this can deviate from the field name for union types; use fieldName() instead. // Also retain fields from lookup indices because we do not have stats for these. Predicate shouldBeRetained = f -> f.field() instanceof PotentiallyUnmappedKeywordEsField - || (localLogicalOptimizerContext.searchStats().exists(f.fieldName()) || lookupFields.contains(f)); + || (localLogicalOptimizerContext.searchStats().exists(f.fieldName()) || lookupFieldsBuilder.contains(f)); return plan.transformUp(p -> missingToNull(p, shouldBeRetained)); } diff --git a/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/optimizer/rules/physical/local/PushStatsToSource.java b/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/optimizer/rules/physical/local/PushStatsToSource.java index 1e4c12ca5a26c..e3954688aed41 100644 --- a/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/optimizer/rules/physical/local/PushStatsToSource.java +++ b/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/optimizer/rules/physical/local/PushStatsToSource.java @@ -74,13 +74,13 @@ private Tuple, List> pushableStats( AggregateExec aggregate, LocalPhysicalOptimizerContext context ) { - AttributeMap stats = new AttributeMap<>(); + AttributeMap.Builder statsBuilder = AttributeMap.builder(); Tuple, List> tuple = new Tuple<>(new ArrayList<>(), new ArrayList<>()); if (aggregate.groupings().isEmpty()) { for (NamedExpression agg : aggregate.aggregates()) { var attribute = agg.toAttribute(); - EsStatsQueryExec.Stat stat = stats.computeIfAbsent(attribute, a -> { + EsStatsQueryExec.Stat stat = statsBuilder.computeIfAbsent(attribute, a -> { if (agg instanceof Alias as) { Expression child = as.child(); if (child instanceof Count count) { diff --git a/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/plan/QueryPlan.java b/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/plan/QueryPlan.java index 5ebf3a211a57c..dc6bb0503d2a1 100644 --- a/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/plan/QueryPlan.java +++ b/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/plan/QueryPlan.java @@ -41,7 +41,7 @@ public QueryPlan(Source source, List children) { public AttributeSet outputSet() { if (lazyOutputSet == null) { - lazyOutputSet = new AttributeSet(output()); + lazyOutputSet = AttributeSet.of(output()); } return lazyOutputSet; } @@ -52,7 +52,7 @@ public AttributeSet inputSet() { for (PlanType child : children()) { attrs.addAll(child.output()); } - lazyInputSet = new AttributeSet(attrs); + lazyInputSet = AttributeSet.of(attrs); } return lazyInputSet; } diff --git a/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/plan/logical/Aggregate.java b/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/plan/logical/Aggregate.java index 8cff1d4c88e90..4a48810c041fb 100644 --- a/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/plan/logical/Aggregate.java +++ b/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/plan/logical/Aggregate.java @@ -173,13 +173,13 @@ protected AttributeSet computeReferences() { } public static AttributeSet computeReferences(List aggregates, List groupings) { - AttributeSet result = Expressions.references(groupings).combine(Expressions.references(aggregates)); + var result = Expressions.references(groupings).combine(Expressions.references(aggregates)).asBuilder(); for (Expression grouping : groupings) { if (grouping instanceof Alias) { result.remove(((Alias) grouping).toAttribute()); } } - return result; + return result.build(); } @Override @@ -206,7 +206,7 @@ public boolean equals(Object obj) { @Override public void postAnalysisVerification(Failures failures) { - AttributeSet groupRefs = new AttributeSet(); + var groupRefsBuilder = AttributeSet.builder(); // check grouping // The grouping can not be an aggregate function groupings.forEach(e -> { @@ -233,12 +233,13 @@ public void postAnalysisVerification(Failures failures) { // keep the grouping attributes (common case) Attribute attr = Expressions.attribute(e); if (attr != null) { - groupRefs.add(attr); + groupRefsBuilder.add(attr); } if (e instanceof FieldAttribute f && f.dataType().isCounter()) { failures.add(fail(e, "cannot group by on [{}] type for grouping [{}]", f.dataType().typeName(), e.sourceText())); } }); + var groupRefs = groupRefsBuilder.build(); // check aggregates - accept only aggregate functions or expressions over grouping // don't allow the group by itself to avoid duplicates in the output @@ -316,14 +317,15 @@ private void checkCategorizeGrouping(Failures failures) { ); // Forbid CATEGORIZE being referenced as a child of an aggregation function - AttributeMap categorizeByAttribute = new AttributeMap<>(); + AttributeMap.Builder categorizeByAttributeBuilder = AttributeMap.builder(); groupings.forEach(g -> { g.forEachDown(Alias.class, alias -> { if (alias.child() instanceof Categorize categorize) { - categorizeByAttribute.put(alias.toAttribute(), categorize); + categorizeByAttributeBuilder.put(alias.toAttribute(), categorize); } }); }); + AttributeMap categorizeByAttribute = categorizeByAttributeBuilder.build(); aggregates.forEach(a -> a.forEachDown(AggregateFunction.class, aggregate -> aggregate.forEachDown(Attribute.class, attribute -> { var categorize = categorizeByAttribute.get(attribute); if (categorize != null) { diff --git a/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/plan/logical/Eval.java b/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/plan/logical/Eval.java index e85c63e79a7e9..5c0aa35f13880 100644 --- a/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/plan/logical/Eval.java +++ b/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/plan/logical/Eval.java @@ -82,7 +82,7 @@ protected AttributeSet computeReferences() { } public static AttributeSet computeReferences(List fields) { - AttributeSet generated = new AttributeSet(asAttributes(fields)); + AttributeSet generated = AttributeSet.of(asAttributes(fields)); return Expressions.references(fields).subtract(generated); } diff --git a/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/plan/logical/UnaryPlan.java b/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/plan/logical/UnaryPlan.java index 6160c82c78c0b..71c802c7eb923 100644 --- a/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/plan/logical/UnaryPlan.java +++ b/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/plan/logical/UnaryPlan.java @@ -44,14 +44,20 @@ public List output() { return child.output(); } + @Override public AttributeSet outputSet() { if (lazyOutputSet == null) { List output = output(); - lazyOutputSet = (output == child.output() ? child.outputSet() : new AttributeSet(output)); + lazyOutputSet = output == child.output() ? child.outputSet() : AttributeSet.of(output); } return lazyOutputSet; } + @Override + public AttributeSet inputSet() { + return child.outputSet(); + } + @Override public int hashCode() { return Objects.hashCode(child()); diff --git a/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/plan/logical/join/InlineJoin.java b/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/plan/logical/join/InlineJoin.java index ce8cedd00abd8..8ce476120abcf 100644 --- a/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/plan/logical/join/InlineJoin.java +++ b/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/plan/logical/join/InlineJoin.java @@ -139,7 +139,7 @@ public List computeOutput(List left, List right JoinType joinType = config().type(); List output; if (LEFT.equals(joinType)) { - AttributeSet rightFields = new AttributeSet(config().rightFields()); + AttributeSet rightFields = AttributeSet.of(config().rightFields()); List leftOutputWithoutMatchFields = new ArrayList<>(); // at this point "left" part of the join contains all the attributes that represent the input of the join // including any aliasing (evals) of expressions used as grouping attributes (or join "match fields") in the join itself diff --git a/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/plan/logical/join/Join.java b/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/plan/logical/join/Join.java index ea1731126cc2e..e12f84efe4c2a 100644 --- a/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/plan/logical/join/Join.java +++ b/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/plan/logical/join/Join.java @@ -140,7 +140,7 @@ public static List computeOutput(List leftOutput, List rightOutputWithoutMatchFields = rightOutput.stream().filter(attr -> rightKeys.contains(attr) == false).toList(); output = mergeOutputAttributes(rightOutputWithoutMatchFields, leftOutput); } else { diff --git a/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/plan/physical/AggregateExec.java b/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/plan/physical/AggregateExec.java index 6d3648d8e37b3..2a6ecb0c6132d 100644 --- a/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/plan/physical/AggregateExec.java +++ b/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/plan/physical/AggregateExec.java @@ -186,8 +186,8 @@ public List output() { @Override protected AttributeSet computeReferences() { return mode.isInputPartial() - ? new AttributeSet(intermediateAttributes) - : Aggregate.computeReferences(aggregates, groupings).subtract(new AttributeSet(ordinalAttributes())); + ? AttributeSet.of(intermediateAttributes) + : Aggregate.computeReferences(aggregates, groupings).subtract(AttributeSet.of(ordinalAttributes())); } /** Returns the attributes that can be loaded from ordinals -- no explicit extraction is needed */ diff --git a/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/plan/physical/FieldExtractExec.java b/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/plan/physical/FieldExtractExec.java index e9783a241f0b9..8a3b61cd2ce1a 100644 --- a/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/plan/physical/FieldExtractExec.java +++ b/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/plan/physical/FieldExtractExec.java @@ -108,7 +108,7 @@ public String getWriteableName() { @Override protected AttributeSet computeReferences() { - return sourceAttribute != null ? new AttributeSet(sourceAttribute) : AttributeSet.EMPTY; + return sourceAttribute != null ? AttributeSet.of(sourceAttribute) : AttributeSet.EMPTY; } @Override diff --git a/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/plan/physical/HashJoinExec.java b/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/plan/physical/HashJoinExec.java index babe6f254ed3f..3818b4e5a4c32 100644 --- a/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/plan/physical/HashJoinExec.java +++ b/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/plan/physical/HashJoinExec.java @@ -93,7 +93,7 @@ public List rightFields() { public Set addedFields() { if (lazyAddedFields == null) { - lazyAddedFields = new AttributeSet(addedFields); + lazyAddedFields = AttributeSet.of(addedFields); } return lazyAddedFields; } diff --git a/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/plan/physical/UnaryExec.java b/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/plan/physical/UnaryExec.java index d787faf7b1b0b..06506b5ff4407 100644 --- a/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/plan/physical/UnaryExec.java +++ b/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/plan/physical/UnaryExec.java @@ -45,12 +45,17 @@ public List output() { public AttributeSet outputSet() { if (lazyOutputSet == null) { List output = output(); - lazyOutputSet = (output == child.output() ? child.outputSet() : new AttributeSet(output)); + lazyOutputSet = output == child.output() ? child.outputSet() : AttributeSet.of(output); return lazyOutputSet; } return lazyOutputSet; } + @Override + public AttributeSet inputSet() { + return child.outputSet(); + } + @Override public int hashCode() { return Objects.hashCode(child()); diff --git a/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/planner/AggregateMapper.java b/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/planner/AggregateMapper.java index 740e39ea77bb4..56293b42f4585 100644 --- a/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/planner/AggregateMapper.java +++ b/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/planner/AggregateMapper.java @@ -53,9 +53,9 @@ public List mapGrouping(List aggrega } private List doMapping(List aggregates, boolean grouping) { - AttributeMap attrToExpressions = new AttributeMap<>(); - aggregates.stream().flatMap(ne -> map(ne, grouping)).forEach(ne -> attrToExpressions.put(ne.toAttribute(), ne)); - return attrToExpressions.values().stream().toList(); + AttributeMap.Builder attrToExpressionsBuilder = AttributeMap.builder(); + aggregates.stream().flatMap(ne -> map(ne, grouping)).forEach(ne -> attrToExpressionsBuilder.put(ne.toAttribute(), ne)); + return attrToExpressionsBuilder.build().values().stream().toList(); } public List mapGrouping(NamedExpression aggregate) { diff --git a/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/planner/PlannerUtils.java b/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/planner/PlannerUtils.java index d58ce1105dc38..cffca7d0f4198 100644 --- a/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/planner/PlannerUtils.java +++ b/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/planner/PlannerUtils.java @@ -209,12 +209,12 @@ static QueryBuilder detectFilter(PhysicalPlan plan, Predicate fieldName) var conjunctions = Predicates.splitAnd(f.condition()); // look only at expressions that contain literals and the target field for (var exp : conjunctions) { - var refs = new AttributeSet(exp.references()); + var refsBuilder = AttributeSet.builder().addAll(exp.references()); // remove literals or attributes that match by name - boolean matchesField = refs.removeIf(e -> fieldName.test(e.name())); + boolean matchesField = refsBuilder.removeIf(e -> fieldName.test(e.name())); // the expression only contains the target reference // and the expression is pushable (functions can be fully translated) - if (matchesField && refs.isEmpty() && canPushToSource(exp)) { + if (matchesField && refsBuilder.isEmpty() && canPushToSource(exp)) { matches.add(exp); } } diff --git a/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/session/EsqlSession.java b/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/session/EsqlSession.java index 94bf414da1b9d..8bb960414a4d7 100644 --- a/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/session/EsqlSession.java +++ b/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/session/EsqlSession.java @@ -567,55 +567,55 @@ static PreAnalysisResult fieldNames(LogicalPlan parsed, Set enrichPolicy return result.withFieldNames(IndexResolver.ALL_FIELDS); } - AttributeSet references = new AttributeSet(); + var referencesBuilder = AttributeSet.builder(); // "keep" attributes are special whenever a wildcard is used in their name // ie "from test | eval lang = languages + 1 | keep *l" should consider both "languages" and "*l" as valid fields to ask for - AttributeSet keepCommandReferences = new AttributeSet(); - AttributeSet keepJoinReferences = new AttributeSet(); + var keepCommandRefsBuilder = AttributeSet.builder(); + var keepJoinRefsBuilder = AttributeSet.builder(); Set wildcardJoinIndices = new java.util.HashSet<>(); parsed.forEachDown(p -> {// go over each plan top-down if (p instanceof RegexExtract re) { // for Grok and Dissect // remove other down-the-tree references to the extracted fields for (Attribute extracted : re.extractedFields()) { - references.removeIf(attr -> matchByName(attr, extracted.name(), false)); + referencesBuilder.removeIf(attr -> matchByName(attr, extracted.name(), false)); } // but keep the inputs needed by Grok/Dissect - references.addAll(re.input().references()); + referencesBuilder.addAll(re.input().references()); } else if (p instanceof Enrich enrich) { - AttributeSet enrichRefs = Expressions.references(enrich.enrichFields()); - enrichRefs = enrichRefs.combine(enrich.matchField().references()); + AttributeSet enrichFieldRefs = Expressions.references(enrich.enrichFields()); + AttributeSet.Builder enrichRefs = enrichFieldRefs.combine(enrich.matchField().references()).asBuilder(); // Enrich adds an EmptyAttribute if no match field is specified // The exact name of the field will be added later as part of enrichPolicyMatchFields Set enrichRefs.removeIf(attr -> attr instanceof EmptyAttribute); - references.addAll(enrichRefs); + referencesBuilder.addAll(enrichRefs); } else if (p instanceof LookupJoin join) { if (join.config().type() instanceof JoinTypes.UsingJoinType usingJoinType) { - keepJoinReferences.addAll(usingJoinType.columns()); + keepJoinRefsBuilder.addAll(usingJoinType.columns()); } - if (keepCommandReferences.isEmpty()) { + if (keepCommandRefsBuilder.isEmpty()) { // No KEEP commands after the JOIN, so we need to mark this index for "*" field resolution wildcardJoinIndices.add(((UnresolvedRelation) join.right()).indexPattern().indexPattern()); } else { // Keep commands can reference the join columns with names that shadow aliases, so we block their removal - keepJoinReferences.addAll(keepCommandReferences); + keepJoinRefsBuilder.addAll(keepCommandRefsBuilder); } } else { - references.addAll(p.references()); + referencesBuilder.addAll(p.references()); if (p instanceof UnresolvedRelation ur && ur.indexMode() == IndexMode.TIME_SERIES) { // METRICS aggs generally rely on @timestamp without the user having to mention it. - references.add(new UnresolvedAttribute(ur.source(), MetadataAttribute.TIMESTAMP_FIELD)); + referencesBuilder.add(new UnresolvedAttribute(ur.source(), MetadataAttribute.TIMESTAMP_FIELD)); } // special handling for UnresolvedPattern (which is not an UnresolvedAttribute) p.forEachExpression(UnresolvedNamePattern.class, up -> { var ua = new UnresolvedAttribute(up.source(), up.name()); - references.add(ua); + referencesBuilder.add(ua); if (p instanceof Keep) { - keepCommandReferences.add(ua); + keepCommandRefsBuilder.add(ua); } }); if (p instanceof Keep) { - keepCommandReferences.addAll(p.references()); + keepCommandRefsBuilder.addAll(p.references()); } } @@ -630,11 +630,11 @@ static PreAnalysisResult fieldNames(LogicalPlan parsed, Set enrichPolicy if (fieldNames.contains(alias.name())) { return; } - references.removeIf(attr -> matchByName(attr, alias.name(), keepCommandReferences.contains(attr))); + referencesBuilder.removeIf(attr -> matchByName(attr, alias.name(), keepCommandRefsBuilder.contains(attr))); }); }); // Add JOIN ON column references afterward to avoid Alias removal - references.addAll(keepJoinReferences); + referencesBuilder.addAll(keepJoinRefsBuilder); // If any JOIN commands need wildcard field-caps calls, persist the index names if (wildcardJoinIndices.isEmpty() == false) { result = result.withWildcardJoinIndices(wildcardJoinIndices); @@ -642,8 +642,8 @@ static PreAnalysisResult fieldNames(LogicalPlan parsed, Set enrichPolicy // remove valid metadata attributes because they will be filtered out by the IndexResolver anyway // otherwise, in some edge cases, we will fail to ask for "*" (all fields) instead - references.removeIf(a -> a instanceof MetadataAttribute || MetadataAttribute.isSupported(a.name())); - Set fieldNames = references.names(); + referencesBuilder.removeIf(a -> a instanceof MetadataAttribute || MetadataAttribute.isSupported(a.name())); + Set fieldNames = referencesBuilder.build().names(); if (fieldNames.isEmpty() && enrichPolicyMatchFields.isEmpty()) { // there cannot be an empty list of fields, we'll ask the simplest and lightest one instead: _index diff --git a/x-pack/plugin/esql/src/test/java/org/elasticsearch/xpack/esql/optimizer/LogicalPlanOptimizerTests.java b/x-pack/plugin/esql/src/test/java/org/elasticsearch/xpack/esql/optimizer/LogicalPlanOptimizerTests.java index 55cfac0ea36d8..70835fc58aa34 100644 --- a/x-pack/plugin/esql/src/test/java/org/elasticsearch/xpack/esql/optimizer/LogicalPlanOptimizerTests.java +++ b/x-pack/plugin/esql/src/test/java/org/elasticsearch/xpack/esql/optimizer/LogicalPlanOptimizerTests.java @@ -5447,11 +5447,11 @@ public void testPushdownWithOverwrittenName() { renamingEval = as(enrich.child(), Eval.class); } - AttributeSet attributesCreatedInEval = new AttributeSet(); + var attributesCreatedInEval = AttributeSet.builder(); for (Alias field : renamingEval.fields()) { attributesCreatedInEval.add(field.toAttribute()); } - assertThat(attributesCreatedInEval, allOf(hasItem(renamed_emp_no), hasItem(renamed_salary), hasItem(renamed_emp_no2))); + assertThat(attributesCreatedInEval.build(), allOf(hasItem(renamed_emp_no), hasItem(renamed_salary), hasItem(renamed_emp_no2))); assertThat(renamingEval.fields().size(), anyOf(equalTo(2), equalTo(4))); // 4 for EVAL, 3 for the other overwritingCommands // emp_no ASC nulls first