SQL: Remove the deprecated AttributeMap(Map) calls (#64664) (#64815)

* SQL: Remove the deprecated `AttributeMap(Map)` calls (#64664)

Use the `AttributeMap.builder()` instead of the `AttributeMap(Map)` to
construct immutable `AttributeMap`s.

Clean up after the `ctor` deprecation in #63710

Author: Andras Palinkas

x-pack/plugin/ql/src/main/java/org/elasticsearch/xpack/ql/expression/AttributeMap.java

@@ -15,7 +15,6 @@
 import java.util.function.BiConsumer;
 import java.util.stream.Stream;
 
-import static java.util.Collections.emptyMap;
 import static java.util.Collections.singletonMap;
 import static java.util.Collections.unmodifiableCollection;
 import static java.util.Collections.unmodifiableSet;
@@ -141,8 +140,8 @@ public String toString() {
     }
 
     @SuppressWarnings("rawtypes")
-    public static final AttributeMap EMPTY = new AttributeMap<>();
-    
+    private static final AttributeMap EMPTY = new AttributeMap<>();
+
     @SuppressWarnings("unchecked")
     public static final <E> AttributeMap<E> emptyAttributeMap() {
         return EMPTY;
@@ -157,23 +156,6 @@ public AttributeMap() {
         delegate = new LinkedHashMap<>();
     }
 
-    /**
-     * Please use the {@link AttributeMap#builder()} instead.
-     */
-    @Deprecated
-    public AttributeMap(Map<Attribute, E> attr) {
-        if (attr.isEmpty()) {
-            delegate = emptyMap();
-        }
-        else {
-            delegate = new LinkedHashMap<>(attr.size());
-
-            for (Entry<Attribute, E> entry : attr.entrySet()) {
-                delegate.put(new AttributeWrapper(entry.getKey()), entry.getValue());
-            }
-        }
-    }
-
     public AttributeMap(Attribute key, E value) {
         delegate = singletonMap(new AttributeWrapper(key), value);
     }
@@ -377,8 +359,12 @@ public static <E> Builder<E> builder() {
         return new Builder<>();
     }
 
+    public static <E> Builder<E> builder(AttributeMap<E> map) {
+        return new Builder<E>().putAll(map);
+    }
+
     public static class Builder<E> {
-        private final AttributeMap<E> map = new AttributeMap<>();
+        private AttributeMap<E> map = new AttributeMap<>();
 
         private Builder() {}
 
@@ -393,7 +379,7 @@ public Builder<E> putAll(AttributeMap<E> m) {
         }
 
         public AttributeMap<E> build() {
-            return new AttributeMap<>(map);
+            return map;
         }
     }
 }

x-pack/plugin/ql/src/main/java/org/elasticsearch/xpack/ql/expression/AttributeSet.java

@@ -13,11 +13,9 @@
 import java.util.function.Predicate;
 import java.util.stream.Stream;
 
-import static java.util.Collections.emptyMap;
-
 public class AttributeSet implements Set<Attribute> {
 
-    private static final AttributeMap<Object> EMPTY_DELEGATE = new AttributeMap<>(emptyMap());
+    private static final AttributeMap<Object> EMPTY_DELEGATE = AttributeMap.emptyAttributeMap();
 
     public static final AttributeSet EMPTY = new AttributeSet(EMPTY_DELEGATE);
 

x-pack/plugin/ql/src/main/java/org/elasticsearch/xpack/ql/expression/Expressions.java

@@ -21,7 +21,6 @@
 import java.util.function.Predicate;
 
 import static java.util.Collections.emptyList;
-import static java.util.Collections.emptyMap;
 
 public final class Expressions {
 
@@ -65,7 +64,7 @@ public static List<Attribute> asAttributes(List<? extends NamedExpression> named
 
     public static AttributeMap<Expression> asAttributeMap(List<? extends NamedExpression> named) {
         if (named.isEmpty()) {
-            return new AttributeMap<>(emptyMap());
+            return AttributeMap.emptyAttributeMap();
         }
 
         AttributeMap<Expression> map = new AttributeMap<>();

x-pack/plugin/ql/src/test/java/org/elasticsearch/xpack/ql/expression/AttributeMapTests.java

@@ -11,7 +11,6 @@
 
 import java.util.Arrays;
 import java.util.Collection;
-import java.util.LinkedHashMap;
 import java.util.List;
 import java.util.Map;
 import java.util.Map.Entry;
@@ -31,12 +30,12 @@ private static Attribute a(String name) {
     }
 
     private static AttributeMap<String> threeMap() {
-        Map<Attribute, String> map = new LinkedHashMap<>();
-        map.put(a("one"), "one");
-        map.put(a("two"), "two");
-        map.put(a("three"), "three");
+        AttributeMap.Builder<String> builder = AttributeMap.builder();
+        builder.put(a("one"), "one");
+        builder.put(a("two"), "two");
+        builder.put(a("three"), "three");
 
-        return new AttributeMap<>(map);
+        return builder.build();
     }
 
     public void testAttributeMapWithSameAliasesCanResolveAttributes() {
@@ -50,27 +49,16 @@ public void testAttributeMapWithSameAliasesCanResolveAttributes() {
         assertTrue(param1.toAttribute().equals(param2.toAttribute()));
         assertFalse(param1.toAttribute().semanticEquals(param2.toAttribute()));
 
-        Map<Attribute, Expression> collectRefs = new LinkedHashMap<>();
-        for (Alias a : Arrays.asList(param1, param2)) {
-            collectRefs.put(a.toAttribute(), a.child());
-        }
-        // we can look up the same item by both attributes
-        assertNotNull(collectRefs.get(param1.toAttribute()));
-        assertNotNull(collectRefs.get(param2.toAttribute()));
-        AttributeMap<Expression> attributeMap = new AttributeMap<>(collectRefs);
-
-        // validate that all Alias can be e
-        assertTrue(attributeMap.containsKey(param1.toAttribute()));
-        assertFalse(attributeMap.containsKey(param2.toAttribute())); // results in unknown attribute exception
-
         AttributeMap.Builder<Expression> mapBuilder = AttributeMap.builder();
         for (Alias a : Arrays.asList(param1, param2)) {
             mapBuilder.put(a.toAttribute(), a.child());
         }
         AttributeMap<Expression> newAttributeMap = mapBuilder.build();
 
         assertTrue(newAttributeMap.containsKey(param1.toAttribute()));
-        assertTrue(newAttributeMap.containsKey(param2.toAttribute())); // no more unknown attribute exception
+        assertTrue(newAttributeMap.get(param1.toAttribute()) == param1.child());
+        assertTrue(newAttributeMap.containsKey(param2.toAttribute()));
+        assertTrue(newAttributeMap.get(param2.toAttribute()) == param2.child());
     }
 
     private Alias createIntParameterAlias(int index, int value) {
@@ -86,13 +74,13 @@ public void testEmptyConstructor() {
         assertThat(m.isEmpty(), is(true));
     }
 
-    public void testMapConstructor() {
-        Map<Attribute, String> map = new LinkedHashMap<>();
-        map.put(a("one"), "one");
-        map.put(a("two"), "two");
-        map.put(a("three"), "three");
+    public void testBuilder() {
+        AttributeMap.Builder<String> builder = AttributeMap.builder();
+        builder.put(a("one"), "one");
+        builder.put(a("two"), "two");
+        builder.put(a("three"), "three");
 
-        AttributeMap<String> m = new AttributeMap<>(map);
+        AttributeMap<String> m = builder.build();
         assertThat(m.size(), is(3));
         assertThat(m.isEmpty(), is(false));
 
@@ -102,12 +90,7 @@ public void testMapConstructor() {
         assertThat(m.containsValue("one"), is(true));
         assertThat(m.containsValue("on"), is(false));
         assertThat(m.attributeNames(), contains("one", "two", "three"));
-        assertThat(m.values(), contains(map.values().toArray()));
-
-        // defensive copying
-        map.put(a("four"), "four");
-        assertThat(m.size(), is(3));
-        assertThat(m.isEmpty(), is(false));
+        assertThat(m.values(), contains("one", "two", "three"));
     }
 
     public void testSingleItemConstructor() {
@@ -165,12 +148,7 @@ public void testKeySet() {
         Attribute two = a("two");
         Attribute three = a("three");
 
-        Map<Attribute, String> map = new LinkedHashMap<>();
-        map.put(one, "one");
-        map.put(two, "two");
-        map.put(three, "three");
-
-        Set<Attribute> keySet = new AttributeMap<>(map).keySet();
+        Set<Attribute> keySet = threeMap().keySet();
         assertThat(keySet, contains(one, two, three));
 
         // toObject
@@ -193,12 +171,7 @@ public void testEntrySet() {
         Attribute two = a("two");
         Attribute three = a("three");
 
-        Map<Attribute, String> map = new LinkedHashMap<>();
-        map.put(one, "one");
-        map.put(two, "two");
-        map.put(three, "three");
-
-        Set<Entry<Attribute, String>> set = new AttributeMap<>(map).entrySet();
+        Set<Entry<Attribute, String>> set = threeMap().entrySet();
 
         assertThat(set, hasSize(3));
 
@@ -212,12 +185,9 @@ public void testEntrySet() {
         assertThat(values, contains("one", "two", "three"));
     }
 
-    public void testForEach() {
+    public void testCopy() {
         AttributeMap<String> m = threeMap();
-
-        Map<Attribute, String> collect = new LinkedHashMap<>();
-        m.forEach(collect::put);
-        AttributeMap<String> copy = new AttributeMap<>(collect);
+        AttributeMap<String> copy = AttributeMap.<String>builder().putAll(m).build();
 
         assertThat(m, is(copy));
     }

x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/optimizer/Optimizer.java

@@ -302,7 +302,7 @@ protected LogicalPlan rule(Project project) {
                 OrderBy ob = (OrderBy) project.child();
 
                 // resolve function references (that maybe hiding the target)
-                final Map<Attribute, Function> collectRefs = new LinkedHashMap<>();
+                AttributeMap.Builder<Function> collectRefs = AttributeMap.builder();
 
                 // collect Attribute sources
                 // only Aliases are interesting since these are the only ones that hide expressions
@@ -316,7 +316,7 @@ protected LogicalPlan rule(Project project) {
                     }
                 }));
 
-                AttributeMap<Function> functions = new AttributeMap<>(collectRefs);
+                AttributeMap<Function> functions = collectRefs.build();
 
                 // track the direct parents
                 Map<String, Order> nestedOrders = new LinkedHashMap<>();
@@ -541,14 +541,14 @@ private List<NamedExpression> combineProjections(List<? extends NamedExpression>
             //TODO: this need rewriting when moving functions of NamedExpression
 
             // collect aliases in the lower list
-            Map<Attribute, NamedExpression> map = new LinkedHashMap<>();
+            AttributeMap.Builder<NamedExpression> aliasesBuilder = AttributeMap.builder();
             for (NamedExpression ne : lower) {
                 if ((ne instanceof Attribute) == false) {
-                    map.put(ne.toAttribute(), ne);
+                    aliasesBuilder.put(ne.toAttribute(), ne);
                 }
             }
 
-            AttributeMap<NamedExpression> aliases = new AttributeMap<>(map);
+            AttributeMap<NamedExpression> aliases = aliasesBuilder.build();
             List<NamedExpression> replaced = new ArrayList<>();
 
             // replace any matching attribute with a lower alias (if there's a match)

x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/plan/logical/Pivot.java

@@ -23,9 +23,7 @@
 import org.elasticsearch.xpack.sql.SqlIllegalArgumentException;
 
 import java.util.ArrayList;
-import java.util.LinkedHashMap;
 import java.util.List;
-import java.util.Map;
 import java.util.Objects;
 
 import static java.util.Collections.singletonList;
@@ -134,7 +132,7 @@ private AttributeSet valuesOutput() {
 
     public AttributeMap<Literal> valuesToLiterals() {
         AttributeSet outValues = valuesOutput();
-        Map<Attribute, Literal> valuesMap = new LinkedHashMap<>();
+        AttributeMap.Builder<Literal> valuesMap = AttributeMap.builder();
 
         int index = 0;
         // for each attribute, associate its value
@@ -152,7 +150,7 @@ public AttributeMap<Literal> valuesToLiterals() {
             }
         }
 
-        return new AttributeMap<>(valuesMap);
+        return valuesMap.build();
     }
 
     @Override

x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/planner/QueryFolder.java

@@ -421,20 +421,21 @@ static EsQueryExec fold(AggregateExec a, EsQueryExec exec) {
 
             // track aliases defined in the SELECT and used inside GROUP BY
             // SELECT x AS a ... GROUP BY a
-            Map<Attribute, Expression> aliasMap = new LinkedHashMap<>();
             String id = null;
+
+            AttributeMap.Builder<Expression> aliases = AttributeMap.builder();
             for (NamedExpression ne : a.aggregates()) {
                 if (ne instanceof Alias) {
-                    aliasMap.put(ne.toAttribute(), ((Alias) ne).child());
+                    aliases.put(ne.toAttribute(), ((Alias) ne).child());
                 }
             }
 
-            if (aliasMap.isEmpty() == false) {
-                Map<Attribute, Expression> newAliases = new LinkedHashMap<>(queryC.aliases());
-                newAliases.putAll(aliasMap);
-                queryC = queryC.withAliases(new AttributeMap<>(newAliases));
+            if (aliases.build().isEmpty() == false) {
+                aliases.putAll(queryC.aliases());
+                queryC = queryC.withAliases(aliases.build());
             }
 
+
             // build the group aggregation
             // NB: any reference in grouping is already "optimized" by its source so there's no need to look for aliases
             GroupingContext groupingContext = groupBy(a.groupings());

x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/querydsl/container/QueryContainer.java

@@ -432,9 +432,8 @@ public FieldExtraction resolve(Attribute attribute) {
 
         // update proc (if needed)
         if (qContainer.scalarFunctions().size() != scalarFunctions.size()) {
-            Map<Attribute, Pipe> procs = new LinkedHashMap<>(qContainer.scalarFunctions());
-            procs.put(attr, proc);
-            qContainer = qContainer.withScalarProcessors(new AttributeMap<>(procs));
+            qContainer = qContainer.withScalarProcessors(
+                AttributeMap.builder(qContainer.scalarFunctions).put(attr, proc).build());
         }
 
         return new Tuple<>(qContainer, new ComputedRef(proc));

x-pack/plugin/sql/src/test/java/org/elasticsearch/xpack/sql/execution/search/SourceGeneratorTests.java

@@ -14,9 +14,7 @@
 import org.elasticsearch.search.sort.FieldSortBuilder;
 import org.elasticsearch.search.sort.SortOrder;
 import org.elasticsearch.test.ESTestCase;
-import org.elasticsearch.xpack.ql.expression.Attribute;
 import org.elasticsearch.xpack.ql.expression.AttributeMap;
-import org.elasticsearch.xpack.ql.expression.Expression;
 import org.elasticsearch.xpack.ql.expression.FieldAttribute;
 import org.elasticsearch.xpack.ql.expression.ReferenceAttribute;
 import org.elasticsearch.xpack.ql.querydsl.container.AttributeSort;
@@ -32,9 +30,6 @@
 import org.elasticsearch.xpack.sql.querydsl.container.QueryContainer;
 import org.elasticsearch.xpack.sql.querydsl.container.ScoreSort;
 
-import java.util.LinkedHashMap;
-import java.util.Map;
-
 import static java.util.Collections.singletonList;
 import static org.elasticsearch.index.query.QueryBuilders.boolQuery;
 import static org.elasticsearch.index.query.QueryBuilders.matchQuery;
@@ -89,9 +84,7 @@ public void testSortNoneSpecified() {
     public void testSelectScoreForcesTrackingScore() {
         Score score = new Score(Source.EMPTY);
         ReferenceAttribute attr = new ReferenceAttribute(score.source(), "score", score.dataType());
-        Map<Attribute, Expression> alias = new LinkedHashMap<>();
-        alias.put(attr, score);
-        QueryContainer container = new QueryContainer().withAliases(new AttributeMap<>(alias)).addColumn(attr);
+        QueryContainer container = new QueryContainer().withAliases(new AttributeMap<>(attr, score)).addColumn(attr);
         SearchSourceBuilder sourceBuilder = SourceGenerator.sourceBuilder(container, null, randomIntBetween(1, 10));
         assertTrue(sourceBuilder.trackScores());
     }

x-pack/plugin/sql/src/test/java/org/elasticsearch/xpack/sql/querydsl/container/QueryContainerTests.java

@@ -9,7 +9,6 @@
 import org.elasticsearch.xpack.ql.expression.Alias;
 import org.elasticsearch.xpack.ql.expression.Attribute;
 import org.elasticsearch.xpack.ql.expression.AttributeMap;
-import org.elasticsearch.xpack.ql.expression.Expression;
 import org.elasticsearch.xpack.ql.expression.FieldAttribute;
 import org.elasticsearch.xpack.ql.querydsl.query.BoolQuery;
 import org.elasticsearch.xpack.ql.querydsl.query.MatchAll;
@@ -24,8 +23,6 @@
 import java.util.AbstractMap.SimpleImmutableEntry;
 import java.util.Arrays;
 import java.util.BitSet;
-import java.util.LinkedHashMap;
-import java.util.Map;
 
 import static java.util.Collections.emptyMap;
 import static java.util.Collections.singletonMap;
@@ -86,11 +83,8 @@ public void testColumnMaskShouldDuplicateSameAttributes() {
         Attribute fourth = new FieldAttribute(Source.EMPTY, "fourth", esField);
         Alias firstAliased = new Alias(Source.EMPTY, "firstAliased", first);
 
-        Map<Attribute, Expression> aliasesMap = new LinkedHashMap<>();
-        aliasesMap.put(firstAliased.toAttribute(), first);
-
         QueryContainer queryContainer = new QueryContainer()
-            .withAliases(new AttributeMap<>(aliasesMap))
+            .withAliases(new AttributeMap<>(firstAliased.toAttribute(), first))
             .addColumn(third)
             .addColumn(first)
             .addColumn(fourth)