Improve explain by predictably ordering predicates

presto-raptor/src/test/java/com/facebook/presto/raptor/metadata/TestShardPredicate.java

@@ -130,9 +130,11 @@ public void testMultipleColumnsMultipleRanges()
                 new RaptorColumnHandle("test", "col", 1, INTEGER),
                 create(SortedRangeSet.copyOf(INTEGER, ImmutableList.of(equal(INTEGER, 1L), equal(INTEGER, 3L))), false)));
         ShardPredicate shardPredicate = ShardPredicate.create(tupleDomain);
-        assertEquals(shardPredicate.getPredicate(), "(((c1_max >= ? OR c1_max IS NULL) AND (c1_min <= ? OR c1_min IS NULL)) " +
-                "OR ((c1_max >= ? OR c1_max IS NULL) AND (c1_min <= ? OR c1_min IS NULL))) " +
-                "AND (((bucket_number >= ? OR bucket_number IS NULL) AND (bucket_number <= ? OR bucket_number IS NULL)) " +
-                "OR ((bucket_number >= ? OR bucket_number IS NULL) AND (bucket_number <= ? OR bucket_number IS NULL)))");
+        assertEquals(
+                shardPredicate.getPredicate(),
+                "(((bucket_number >= ? OR bucket_number IS NULL) AND (bucket_number <= ? OR bucket_number IS NULL)) " +
+                        "OR ((bucket_number >= ? OR bucket_number IS NULL) AND (bucket_number <= ? OR bucket_number IS NULL))) " +
+                        "AND (((c1_max >= ? OR c1_max IS NULL) AND (c1_min <= ? OR c1_min IS NULL)) " +
+                        "OR ((c1_max >= ? OR c1_max IS NULL) AND (c1_min <= ? OR c1_min IS NULL)))");
     }
 }

presto-spi/src/main/java/com/facebook/presto/spi/GroupingProperty.java

@@ -17,15 +17,16 @@
 import com.fasterxml.jackson.annotation.JsonProperty;
 
 import java.util.Collection;
-import java.util.Collections;
-import java.util.HashSet;
+import java.util.LinkedHashSet;
 import java.util.Objects;
 import java.util.Optional;
 import java.util.Set;
 import java.util.function.Function;
 import java.util.stream.Collectors;
 
+import static java.util.Collections.unmodifiableSet;
 import static java.util.Objects.requireNonNull;
+import static java.util.stream.Collectors.toCollection;
 
 public final class GroupingProperty<E>
         implements LocalProperty<E>
@@ -37,7 +38,7 @@ public GroupingProperty(@JsonProperty("columns") Collection<E> columns)
     {
         requireNonNull(columns, "columns is null");
 
-        this.columns = Collections.unmodifiableSet(new HashSet<>(columns));
+        this.columns = unmodifiableSet(new LinkedHashSet<>(columns));
     }
 
     @Override
@@ -76,12 +77,12 @@ public <T> Optional<LocalProperty<T>> translate(Function<E, Optional<T>> transla
     {
         Set<Optional<T>> translated = columns.stream()
                 .map(translator)
-                .collect(Collectors.toSet());
+                .collect(toCollection(LinkedHashSet::new));
 
         if (translated.stream().allMatch(Optional::isPresent)) {
             Set<T> columns = translated.stream()
                     .map(Optional::get)
-                    .collect(Collectors.toSet());
+                    .collect(toCollection(LinkedHashSet::new));
 
             return Optional.of(new GroupingProperty<>(columns));
         }

presto-spi/src/main/java/com/facebook/presto/spi/LocalProperty.java

@@ -16,7 +16,7 @@
 import com.fasterxml.jackson.annotation.JsonSubTypes;
 import com.fasterxml.jackson.annotation.JsonTypeInfo;
 
-import java.util.HashSet;
+import java.util.LinkedHashSet;
 import java.util.Optional;
 import java.util.Set;
 import java.util.function.Function;
@@ -48,7 +48,7 @@ public interface LocalProperty<E>
      */
     default Optional<LocalProperty<E>> withConstants(Set<E> constants)
     {
-        Set<E> set = new HashSet<>(getColumns());
+        Set<E> set = new LinkedHashSet<>(getColumns());
         set.removeAll(constants);
 
         if (set.isEmpty()) {

presto-spi/src/main/java/com/facebook/presto/spi/predicate/EquatableValueSet.java

@@ -22,17 +22,20 @@
 import java.util.Collection;
 import java.util.Collections;
 import java.util.HashSet;
+import java.util.LinkedHashSet;
 import java.util.Objects;
 import java.util.Set;
 import java.util.function.Consumer;
 import java.util.function.Function;
+import java.util.stream.Collector;
 import java.util.stream.Collectors;
 import java.util.stream.Stream;
 
 import static java.util.Collections.unmodifiableCollection;
+import static java.util.Collections.unmodifiableSet;
 import static java.util.Objects.requireNonNull;
+import static java.util.stream.Collectors.toCollection;
 import static java.util.stream.Collectors.toList;
-import static java.util.stream.Collectors.toSet;
 
 /**
  * A set containing values that are uniquely identifiable.
@@ -63,7 +66,7 @@ public EquatableValueSet(
         }
         this.type = type;
         this.whiteList = whiteList;
-        this.entries = Collections.unmodifiableSet(new HashSet<>(entries));
+        this.entries = unmodifiableSet(new LinkedHashSet<>(entries));
     }
 
     static EquatableValueSet none(Type type)
@@ -78,7 +81,7 @@ static EquatableValueSet all(Type type)
 
     static EquatableValueSet of(Type type, Object first, Object... rest)
     {
-        HashSet<ValueEntry> set = new HashSet<>(rest.length + 1);
+        HashSet<ValueEntry> set = new LinkedHashSet<>(rest.length + 1);
         set.add(ValueEntry.create(type, first));
         for (Object value : rest) {
             set.add(ValueEntry.create(type, value));
@@ -90,7 +93,7 @@ static EquatableValueSet copyOf(Type type, Collection<Object> values)
     {
         return new EquatableValueSet(type, true, values.stream()
                 .map(value -> ValueEntry.create(type, value))
-                .collect(toSet()));
+                .collect(toLinkedSet()));
     }
 
     @JsonProperty
@@ -246,20 +249,20 @@ private static <T> Set<T> intersect(Set<T> set1, Set<T> set2)
     {
         return set1.stream()
                 .filter(set2::contains)
-                .collect(toSet());
+                .collect(toLinkedSet());
     }
 
     private static <T> Set<T> union(Set<T> set1, Set<T> set2)
     {
         return Stream.concat(set1.stream(), set2.stream())
-                .collect(toSet());
+                .collect(toLinkedSet());
     }
 
     private static <T> Set<T> subtract(Set<T> set1, Set<T> set2)
     {
         return set1.stream()
                 .filter(value -> !set2.contains(value))
-                .collect(toSet());
+                .collect(toLinkedSet());
     }
 
     private EquatableValueSet checkCompatibility(ValueSet other)
@@ -294,6 +297,11 @@ public boolean equals(Object obj)
                 && Objects.equals(this.entries, other.entries);
     }
 
+    private static <T> Collector<T, ?, Set<T>> toLinkedSet()
+    {
+        return toCollection(LinkedHashSet::new);
+    }
+
     public static class ValueEntry
     {
         private final Type type;

presto-spi/src/main/java/com/facebook/presto/spi/predicate/TupleDomain.java

@@ -25,13 +25,14 @@
 import java.util.HashMap;
 import java.util.HashSet;
 import java.util.Iterator;
+import java.util.LinkedHashMap;
 import java.util.List;
 import java.util.Map;
 import java.util.Objects;
 import java.util.Optional;
 import java.util.Set;
 import java.util.function.Function;
-import java.util.stream.Collectors;
+import java.util.stream.Collector;
 
 import static java.util.Objects.requireNonNull;
 import static java.util.stream.Collectors.toList;
@@ -97,7 +98,7 @@ public static <T> Optional<Map<T, NullableValue>> extractFixedValues(TupleDomain
         return Optional.of(tupleDomain.getDomains().get()
                 .entrySet().stream()
                 .filter(entry -> entry.getValue().isNullableSingleValue())
-                .collect(toMap(Map.Entry::getKey, entry -> new NullableValue(entry.getValue().getType(), entry.getValue().getNullableSingleValue()))));
+                .collect(toLinkedMap(Map.Entry::getKey, entry -> new NullableValue(entry.getValue().getType(), entry.getValue().getNullableSingleValue()))));
     }
 
     /**
@@ -107,7 +108,7 @@ public static <T> Optional<Map<T, NullableValue>> extractFixedValues(TupleDomain
     public static <T> TupleDomain<T> fromFixedValues(Map<T, NullableValue> fixedValues)
     {
         return TupleDomain.withColumnDomains(fixedValues.entrySet().stream()
-                .collect(toMap(
+                .collect(toLinkedMap(
                         Map.Entry::getKey,
                         entry -> {
                             Type type = entry.getValue().getType();
@@ -124,7 +125,7 @@ public static <T> TupleDomain<T> fromColumnDomains(@JsonProperty("columnDomains"
             return none();
         }
         return withColumnDomains(columnDomains.get().stream()
-                .collect(toMap(ColumnDomain::getColumn, ColumnDomain::getDomain)));
+                .collect(toLinkedMap(ColumnDomain::getColumn, ColumnDomain::getDomain)));
     }
 
     @JsonProperty
@@ -145,7 +146,7 @@ private static <T> Map<T, Domain> normalizeAndCopy(Map<T, Domain> domains)
     {
         return domains.entrySet().stream()
                 .filter(entry -> !entry.getValue().isAll())
-                .collect(toMap(Map.Entry::getKey, Map.Entry::getValue));
+                .collect(toLinkedMap(Map.Entry::getKey, Map.Entry::getValue));
     }
 
     /**
@@ -187,7 +188,7 @@ public TupleDomain<T> intersect(TupleDomain<T> other)
             return none();
         }
 
-        Map<T, Domain> intersected = new HashMap<>(this.getDomains().get());
+        Map<T, Domain> intersected = new LinkedHashMap<>(this.getDomains().get());
         for (Map.Entry<T, Domain> entry : other.getDomains().get().entrySet()) {
             Domain intersectionDomain = intersected.get(entry.getKey());
             if (intersectionDomain == null) {
@@ -266,7 +267,7 @@ public static <T> TupleDomain<T> columnWiseUnion(List<TupleDomain<T>> tupleDomai
         }
 
         // group domains by column (only for common columns)
-        Map<T, List<Domain>> domainsByColumn = new HashMap<>(tupleDomains.size());
+        Map<T, List<Domain>> domainsByColumn = new LinkedHashMap<>(tupleDomains.size());
 
         for (TupleDomain<T> domain : tupleDomains) {
             if (!domain.isNone()) {
@@ -284,7 +285,7 @@ public static <T> TupleDomain<T> columnWiseUnion(List<TupleDomain<T>> tupleDomai
         }
 
         // finally, do the column-wise union
-        Map<T, Domain> result = new HashMap<>(domainsByColumn.size());
+        Map<T, Domain> result = new LinkedHashMap<>(domainsByColumn.size());
         for (Map.Entry<T, List<Domain>> entry : domainsByColumn.entrySet()) {
             result.put(entry.getKey(), Domain.union(entry.getValue()));
         }
@@ -351,7 +352,7 @@ else if (isNone()) {
         }
         else {
             buffer.append(domains.get().entrySet().stream()
-                    .collect(toMap(Map.Entry::getKey, entry -> entry.getValue().toString(session))));
+                    .collect(toLinkedMap(Map.Entry::getKey, entry -> entry.getValue().toString(session))));
         }
         return buffer.toString();
     }
@@ -362,7 +363,7 @@ public <U> TupleDomain<U> transform(Function<T, U> function)
             return TupleDomain.none();
         }
 
-        HashMap<U, Domain> result = new HashMap<>(domains.get().size());
+        HashMap<U, Domain> result = new LinkedHashMap<>(domains.get().size());
         for (Map.Entry<T, Domain> entry : domains.get().entrySet()) {
             U key = function.apply(entry.getKey());
 
@@ -387,11 +388,20 @@ public TupleDomain<T> simplify()
         }
 
         Map<T, Domain> simplified = domains.get().entrySet().stream()
-                .collect(Collectors.toMap(Map.Entry::getKey, e -> e.getValue().simplify()));
+                .collect(toLinkedMap(Map.Entry::getKey, e -> e.getValue().simplify()));
 
         return TupleDomain.withColumnDomains(simplified);
     }
 
+    private static <T, K, U> Collector<T, ?, Map<K, U>> toLinkedMap(Function<? super T, ? extends K> keyMapper, Function<? super T, ? extends U> valueMapper)
+    {
+        return toMap(
+                keyMapper,
+                valueMapper,
+                (u, v) -> { throw new IllegalStateException(String.format("Duplicate key %s", u)); },
+                LinkedHashMap::new);
+    }
+
     // Available for Jackson serialization only!
     public static class ColumnDomain<C>
     {

presto-spi/src/main/java/com/facebook/presto/spi/statistics/TableStatistics.java

@@ -16,7 +16,7 @@
 
 import com.facebook.presto.spi.ColumnHandle;
 
-import java.util.HashMap;
+import java.util.LinkedHashMap;
 import java.util.Map;
 import java.util.Objects;
 
@@ -92,7 +92,7 @@ public static Builder builder()
     public static final class Builder
     {
         private Estimate rowCount = Estimate.unknown();
-        private Map<ColumnHandle, ColumnStatistics> columnStatisticsMap = new HashMap<>();
+        private Map<ColumnHandle, ColumnStatistics> columnStatisticsMap = new LinkedHashMap<>();
 
         public Builder setRowCount(Estimate rowCount)
         {

presto-spi/src/main/java/com/facebook/presto/spi/statistics/TableStatisticsMetadata.java

@@ -14,7 +14,7 @@
 package com.facebook.presto.spi.statistics;
 
 import java.util.ArrayList;
-import java.util.HashSet;
+import java.util.LinkedHashSet;
 import java.util.List;
 import java.util.Set;
 
@@ -42,8 +42,8 @@ public TableStatisticsMetadata(
             Set<TableStatisticType> tableStatistics,
             List<String> groupingColumns)
     {
-        this.columnStatistics = unmodifiableSet(new HashSet<>(requireNonNull(columnStatistics, "columnStatistics is null")));
-        this.tableStatistics = unmodifiableSet(new HashSet<>(requireNonNull(tableStatistics, "tableStatistics is null")));
+        this.columnStatistics = unmodifiableSet(new LinkedHashSet<>(requireNonNull(columnStatistics, "columnStatistics is null")));
+        this.tableStatistics = unmodifiableSet(new LinkedHashSet<>(requireNonNull(tableStatistics, "tableStatistics is null")));
         this.groupingColumns = unmodifiableList(new ArrayList<>(requireNonNull(groupingColumns, "groupingColumns is null")));
     }