Skip to content

Commit

Permalink
Fix ValuesMatcher
Browse files Browse the repository at this point in the history
- Fix StringLiteral generation by creating string instead of using
toString value of Slice, which only includes base/address/length.
- Use Set instead of List for expected rows. The row sequence does not
impact correctness.
  • Loading branch information
shixuan-fan committed Oct 9, 2020
1 parent 8ab725f commit 05f3875
Showing 1 changed file with 7 additions and 4 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -27,12 +27,14 @@
import com.facebook.presto.sql.tree.StringLiteral;
import com.facebook.presto.sql.tree.SymbolReference;
import com.google.common.collect.ImmutableMap;
import com.google.common.collect.ImmutableSet;
import com.google.common.collect.Maps;
import io.airlift.slice.Slice;

import java.util.List;
import java.util.Map;
import java.util.Optional;
import java.util.Set;

import static com.facebook.presto.sql.planner.assertions.MatchResult.NO_MATCH;
import static com.facebook.presto.sql.planner.assertions.MatchResult.match;
Expand All @@ -41,14 +43,15 @@
import static com.google.common.base.MoreObjects.toStringHelper;
import static com.google.common.base.Preconditions.checkState;
import static com.google.common.collect.ImmutableList.toImmutableList;
import static com.google.common.collect.ImmutableSet.toImmutableSet;
import static java.util.Objects.requireNonNull;

public class ValuesMatcher
implements Matcher
{
private final Map<String, Integer> outputSymbolAliases;
private final Optional<Integer> expectedOutputSymbolCount;
private final Optional<List<List<Expression>>> expectedRows;
private final Optional<Set<List<Expression>>> expectedRows;

public ValuesMatcher(
Map<String, Integer> outputSymbolAliases,
Expand All @@ -57,7 +60,7 @@ public ValuesMatcher(
{
this.outputSymbolAliases = ImmutableMap.copyOf(outputSymbolAliases);
this.expectedOutputSymbolCount = requireNonNull(expectedOutputSymbolCount, "expectedOutputSymbolCount is null");
this.expectedRows = requireNonNull(expectedRows, "expectedRows is null");
this.expectedRows = requireNonNull(expectedRows, "expectedRows is null").map(ImmutableSet::copyOf);
}

@Override
Expand Down Expand Up @@ -91,12 +94,12 @@ public MatchResult detailMatches(PlanNode node, StatsProvider stats, Session ses
return new DoubleLiteral(String.valueOf(expression.getValue()));
}
if (expression.getType().getJavaType() == Slice.class) {
return new StringLiteral(String.valueOf(expression.getValue()));
return new StringLiteral(((Slice) expression.getValue()).toStringUtf8());
}
return new GenericLiteral(expression.getType().toString(), String.valueOf(expression.getValue()));
})
.collect(toImmutableList()))
.collect(toImmutableList())))
.collect(toImmutableSet())))
.orElse(true)) {
return NO_MATCH;
}
Expand Down

0 comments on commit 05f3875

Please sign in to comment.