Skip to content

Commit

Permalink
Use comparator for expressions canonical ordering
Browse files Browse the repository at this point in the history
Ordering is obsolete for Java >= 8, comparator
should be also faster.
  • Loading branch information
Pawel Palucha authored and sopel39 committed Oct 7, 2021
1 parent 2840b38 commit 9705655
Showing 1 changed file with 9 additions and 12 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -14,13 +14,11 @@
package io.trino.sql.planner;

import com.google.common.annotations.VisibleForTesting;
import com.google.common.collect.ComparisonChain;
import com.google.common.collect.ImmutableList;
import com.google.common.collect.ImmutableMap;
import com.google.common.collect.ImmutableSet;
import com.google.common.collect.ImmutableSetMultimap;
import com.google.common.collect.Multimap;
import com.google.common.collect.Ordering;
import io.trino.metadata.Metadata;
import io.trino.sql.tree.ComparisonExpression;
import io.trino.sql.tree.Expression;
Expand All @@ -30,13 +28,15 @@
import java.util.ArrayList;
import java.util.Arrays;
import java.util.Collection;
import java.util.Comparator;
import java.util.HashMap;
import java.util.LinkedHashSet;
import java.util.List;
import java.util.Map;
import java.util.Objects;
import java.util.Set;
import java.util.function.Predicate;
import java.util.function.ToIntFunction;
import java.util.stream.Collectors;
import java.util.stream.Stream;

Expand All @@ -51,19 +51,16 @@
*/
public class EqualityInference
{
// Ordering used to determine Expression preference when determining canonicals
private static final Ordering<Expression> CANONICAL_ORDERING = Ordering.from((expression1, expression2) -> {
// Comparator used to determine Expression preference when determining canonicals
private static final Comparator<Expression> CANONICAL_COMPARATOR = Comparator
// Current cost heuristic:
// 1) Prefer fewer input symbols
// 2) Prefer smaller expression trees
// 3) Sort the expressions alphabetically - creates a stable consistent ordering (extremely useful for unit testing)
// TODO: be more precise in determining the cost of an expression
return ComparisonChain.start()
.compare(SymbolsExtractor.extractAll(expression1).size(), SymbolsExtractor.extractAll(expression2).size())
.compare(SubExpressionExtractor.extract(expression1).count(), SubExpressionExtractor.extract(expression2).count())
.compare(expression1.toString(), expression2.toString())
.result();
});
.comparingInt((ToIntFunction<Expression>) (expression -> SymbolsExtractor.extractAll(expression).size()))
.thenComparingLong(expression -> SubExpressionExtractor.extract(expression).count())
.thenComparing(Expression::toString);

private final Multimap<Expression, Expression> equalitySets; // Indexed by canonical expression
private final Map<Expression, Expression> canonicalMap; // Map each known expression to canonical expression
Expand Down Expand Up @@ -293,7 +290,7 @@ private Expression rewrite(Expression expression, Predicate<Symbol> symbolScope,
*/
private static Expression getCanonical(Stream<Expression> expressions)
{
return expressions.min(CANONICAL_ORDERING).orElse(null);
return expressions.min(CANONICAL_COMPARATOR).orElse(null);
}

/**
Expand Down Expand Up @@ -335,7 +332,7 @@ private static Multimap<Expression, Expression> makeEqualitySets(DisjointSet<Exp
ImmutableSetMultimap.Builder<Expression, Expression> builder = ImmutableSetMultimap.builder();
for (Set<Expression> equalityGroup : equalities.getEquivalentClasses()) {
if (!equalityGroup.isEmpty()) {
builder.putAll(CANONICAL_ORDERING.min(equalityGroup), equalityGroup);
builder.putAll(equalityGroup.stream().min(CANONICAL_COMPARATOR).get(), equalityGroup);
}
}
return builder.build();
Expand Down

0 comments on commit 9705655

Please sign in to comment.