diff --git a/presto-main/src/main/java/com/facebook/presto/sql/planner/planPrinter/JsonRenderer.java b/presto-main/src/main/java/com/facebook/presto/sql/planner/planPrinter/JsonRenderer.java index 2bff74077a319..020993619665d 100644 --- a/presto-main/src/main/java/com/facebook/presto/sql/planner/planPrinter/JsonRenderer.java +++ b/presto-main/src/main/java/com/facebook/presto/sql/planner/planPrinter/JsonRenderer.java @@ -72,6 +72,7 @@ public static class JsonRenderedNode private final List children; private final List remoteSources; + @JsonCreator public JsonRenderedNode(String id, String name, String identifier, String details, List children, List remoteSources) { this.id = requireNonNull(id, "id is null"); diff --git a/presto-verifier/src/main/java/com/facebook/presto/verifier/framework/ExplainMatchResult.java b/presto-verifier/src/main/java/com/facebook/presto/verifier/framework/ExplainMatchResult.java index eea8745ad75fd..dae0ea0fbab92 100644 --- a/presto-verifier/src/main/java/com/facebook/presto/verifier/framework/ExplainMatchResult.java +++ b/presto-verifier/src/main/java/com/facebook/presto/verifier/framework/ExplainMatchResult.java @@ -21,7 +21,8 @@ public class ExplainMatchResult public enum MatchType { MATCH, - PLAN_CHANGED, + STRUCTURE_MISMATCH, + DETAILS_MISMATCH, } private final MatchType matchType; diff --git a/presto-verifier/src/main/java/com/facebook/presto/verifier/framework/ExplainVerification.java b/presto-verifier/src/main/java/com/facebook/presto/verifier/framework/ExplainVerification.java index f6424a1b6fa31..ed49e76d17dbd 100644 --- a/presto-verifier/src/main/java/com/facebook/presto/verifier/framework/ExplainVerification.java +++ b/presto-verifier/src/main/java/com/facebook/presto/verifier/framework/ExplainVerification.java @@ -13,7 +13,9 @@ */ package com.facebook.presto.verifier.framework; +import com.facebook.airlift.json.JsonCodec; import com.facebook.presto.sql.parser.SqlParser; +import com.facebook.presto.sql.planner.planPrinter.JsonRenderer.JsonRenderedNode; import com.facebook.presto.sql.tree.Explain; import com.facebook.presto.sql.tree.ExplainFormat; import com.facebook.presto.sql.tree.Statement; @@ -24,15 +26,15 @@ import com.facebook.presto.verifier.prestoaction.SqlExceptionClassifier; import com.google.common.collect.ImmutableList; +import java.util.Objects; import java.util.Optional; +import static com.facebook.airlift.json.JsonCodec.jsonCodec; import static com.facebook.presto.sql.tree.ExplainFormat.Type.JSON; -import static com.facebook.presto.verifier.framework.ClusterType.CONTROL; +import static com.facebook.presto.verifier.framework.ExplainMatchResult.MatchType; +import static com.facebook.presto.verifier.framework.ExplainMatchResult.MatchType.DETAILS_MISMATCH; import static com.facebook.presto.verifier.framework.ExplainMatchResult.MatchType.MATCH; -import static com.facebook.presto.verifier.framework.ExplainMatchResult.MatchType.PLAN_CHANGED; -import static com.facebook.presto.verifier.framework.QueryType.CREATE_TABLE_AS_SELECT; -import static com.facebook.presto.verifier.framework.QueryType.INSERT; -import static com.facebook.presto.verifier.framework.QueryType.QUERY; +import static com.facebook.presto.verifier.framework.ExplainMatchResult.MatchType.STRUCTURE_MISMATCH; import static com.facebook.presto.verifier.framework.VerifierUtil.PARSING_OPTIONS; import static com.google.common.base.Preconditions.checkArgument; import static com.google.common.collect.Iterables.getOnlyElement; @@ -42,6 +44,7 @@ public class ExplainVerification extends AbstractVerification { private static final ResultSetConverter QUERY_PLAN_RESULT_SET_CONVERTER = resultSet -> Optional.of(resultSet.getString("Query Plan")); + private static final JsonCodec PLAN_CODEC = jsonCodec(JsonRenderedNode.class); private final SqlParser sqlParser; public ExplainVerification( @@ -75,9 +78,11 @@ protected ExplainMatchResult verify( { checkArgument(controlQueryResult.isPresent(), "control query plan is missing"); checkArgument(testQueryResult.isPresent(), "test query plan is missing"); - String controlPlan = getOnlyElement(controlQueryResult.get().getResults()); - String testPlan = getOnlyElement(testQueryResult.get().getResults()); - return new ExplainMatchResult(testPlan.equals(controlPlan) ? MATCH : PLAN_CHANGED); + + JsonRenderedNode controlPlan = PLAN_CODEC.fromJson(getOnlyElement(controlQueryResult.get().getResults())); + JsonRenderedNode testPlan = PLAN_CODEC.fromJson(getOnlyElement(testQueryResult.get().getResults())); + + return new ExplainMatchResult(match(controlPlan, testPlan)); } @Override @@ -97,4 +102,26 @@ protected void updateQueryInfo(QueryInfo.Builder queryInfo, Optional queryInfo.setJsonPlan(getOnlyElement(result.getResults()))); } + + private MatchType match(JsonRenderedNode controlPlan, JsonRenderedNode testPlan) + { + if (!Objects.equals(controlPlan.getName(), testPlan.getName()) + || !Objects.equals(controlPlan.getIdentifier(), testPlan.getIdentifier()) + || !Objects.equals(controlPlan.getRemoteSources(), testPlan.getRemoteSources()) + || controlPlan.getChildren().size() != testPlan.getChildren().size()) { + return STRUCTURE_MISMATCH; + } + + boolean detailsMismatched = !Objects.equals(controlPlan.getDetails(), testPlan.getDetails()); + for (int i = 0; i < controlPlan.getChildren().size(); i++) { + MatchType childMatchType = match(controlPlan.getChildren().get(i), testPlan.getChildren().get(i)); + if (childMatchType == STRUCTURE_MISMATCH) { + return STRUCTURE_MISMATCH; + } + else if (childMatchType == DETAILS_MISMATCH) { + detailsMismatched = true; + } + } + return detailsMismatched ? DETAILS_MISMATCH : MATCH; + } } diff --git a/presto-verifier/src/test/java/com/facebook/presto/verifier/framework/TestExplainVerification.java b/presto-verifier/src/test/java/com/facebook/presto/verifier/framework/TestExplainVerification.java index a86ccf773d386..649b28cbe7b6a 100644 --- a/presto-verifier/src/test/java/com/facebook/presto/verifier/framework/TestExplainVerification.java +++ b/presto-verifier/src/test/java/com/facebook/presto/verifier/framework/TestExplainVerification.java @@ -72,14 +72,32 @@ public void testSuccess() } @Test - public void testPlanChanged() + public void testStructureMismatch() + { + queryRunner.execute("CREATE TABLE structure_mismatch (x int, ds varchar)"); + Optional event = runVerification( + "SELECT count(*) FROM structure_mismatch", + "SELECT count(*) FROM structure_mismatch CROSS JOIN structure_mismatch"); + assertTrue(event.isPresent()); + + // Explain verification do not fail in case of plan changes. + assertEvent(event.get(), SUCCEEDED); + assertEquals(event.get().getMatchType(), "STRUCTURE_MISMATCH"); + assertEquals(event.get().getControlQueryInfo().getQuery().trim(), "EXPLAIN (FORMAT JSON)\nSELECT \"count\"(*)\nFROM\n structure_mismatch"); + assertEquals(event.get().getTestQueryInfo().getQuery().trim(), "EXPLAIN (FORMAT JSON)\nSELECT \"count\"(*)\nFROM\n (structure_mismatch\nCROSS JOIN structure_mismatch)"); + assertNotNull(event.get().getControlQueryInfo().getJsonPlan()); + assertNotNull(event.get().getTestQueryInfo().getJsonPlan()); + } + + @Test + public void testDetailsMismatch() { Optional event = runVerification("SELECT 1", "SELECT 2"); assertTrue(event.isPresent()); // Explain verification do not fail in case of plan changes. assertEvent(event.get(), SUCCEEDED); - assertEquals(event.get().getMatchType(), "PLAN_CHANGED"); + assertEquals(event.get().getMatchType(), "DETAILS_MISMATCH"); assertEquals(event.get().getControlQueryInfo().getQuery().trim(), "EXPLAIN (FORMAT JSON)\nSELECT 1"); assertEquals(event.get().getTestQueryInfo().getQuery().trim(), "EXPLAIN (FORMAT JSON)\nSELECT 2"); assertNotNull(event.get().getControlQueryInfo().getJsonPlan());