Enable tests to fail

prestodb · May 22, 2024 · 4b085e8 · 4b085e8
1 parent cd3ef2b
commit 4b085e8
Show file tree

Hide file tree

Showing 3 changed files with 53 additions and 29 deletions.
diff --git a/presto-main/src/test/java/com/facebook/presto/sql/planner/TestPlanMatchingFramework.java b/presto-main/src/test/java/com/facebook/presto/sql/planner/TestPlanMatchingFramework.java
@@ -17,10 +17,15 @@
 import com.facebook.presto.spi.plan.OutputNode;
 import com.facebook.presto.spi.plan.TableScanNode;
 import com.facebook.presto.sql.planner.assertions.BasePlanTest;
+import com.facebook.presto.sql.planner.assertions.ExpressionMatcher;
+import com.facebook.presto.sql.planner.assertions.PlanMatchPattern;
 import com.google.common.collect.ImmutableList;
 import com.google.common.collect.ImmutableMap;
 import org.testng.annotations.Test;
 
+import java.util.List;
+import java.util.Map;
+
 import static com.facebook.presto.SystemSessionProperties.JOIN_DISTRIBUTION_TYPE;
 import static com.facebook.presto.SystemSessionProperties.JOIN_REORDERING_STRATEGY;
 import static com.facebook.presto.spi.plan.JoinType.INNER;
@@ -200,47 +205,36 @@ public void testReferenceNonexistentAlias()
                         tableScan("lineitem", ImmutableMap.of("ORDERKEY", "orderkey"))));
     }
 
-    /*
-     * There are so many ways for matches to fail that this is not likely to be generally useful.
-     * Pending better diagnostics, please leave this here, and restrict its use to simple queries
-     * that have few ways to not match a pattern, and functionality that is well-tested with
-     * positive tests.
-     */
-    private void assertFails(Runnable runnable)
-    {
-        try {
-            runnable.run();
-            fail("Plans should not have matched!");
-        }
-        catch (AssertionError e) {
-            //ignored
-        }
-    }
-
     @Test
     public void testStrictOutputExtraSymbols()
     {
-        assertFails(() -> assertMinimallyOptimizedPlan("SELECT orderkey, extendedprice FROM lineitem",
-                strictOutput(ImmutableList.of("ORDERKEY"),
-                        tableScan("lineitem", ImmutableMap.of("ORDERKEY", "orderkey",
-                                "EXTENDEDPRICE", "extendedprice")))));
+        assertMinimallyOptimizedPlanDoesNotMatch(
+            "SELECT orderkey, extendedprice FROM lineitem",
+            strictOutput(ImmutableList.of("ORDERKEY"),
+                tableScan("lineitem", ImmutableMap.of("ORDERKEY", "orderkey",
+                        "EXTENDEDPRICE", "extendedprice"))));
     }
 
     @Test
     public void testStrictTableScanExtraSymbols()
     {
-        assertFails(() -> assertMinimallyOptimizedPlan("SELECT orderkey, extendedprice FROM lineitem",
-                output(ImmutableList.of("ORDERKEY", "EXTENDEDPRICE"),
-                        strictTableScan("lineitem", ImmutableMap.of("ORDERKEY", "orderkey")))));
+        String sql = "SELECT orderkey, extendedprice FROM lineitem";
+        List<String> outputs = ImmutableList.of("ORDERKEY", "EXTENDEDPRICE");
+        PlanMatchPattern source = strictTableScan("lineitem", ImmutableMap.of("ORDERKEY", "orderkey"));
+        PlanMatchPattern output = output(outputs, source);
+        assertMinimallyOptimizedPlanDoesNotMatch(sql, output);
     }
 
     @Test
     public void testStrictProjectExtraSymbols()
     {
-        assertFails(() -> assertMinimallyOptimizedPlan("SELECT discount, orderkey, 1 + orderkey FROM lineitem",
-                output(ImmutableList.of("ORDERKEY", "EXPRESSION"),
-                        strictProject(ImmutableMap.of("EXPRESSION", expression("1 + ORDERKEY"), "ORDERKEY", expression("ORDERKEY")),
-                                tableScan("lineitem", ImmutableMap.of("ORDERKEY", "orderkey"))))));
+        String sql = "SELECT discount, orderkey, 1 + orderkey FROM lineitem";
+        List<String> outputs = ImmutableList.of("ORDERKEY", "EXPRESSION");
+        Map<String, ExpressionMatcher> assignments = ImmutableMap.of("EXPRESSION", expression("1 + ORDERKEY"), "ORDERKEY", expression("ORDERKEY"));
+        PlanMatchPattern source = tableScan("lineitem", ImmutableMap.of("ORDERKEY", "orderkey"));
+        PlanMatchPattern strict = strictProject(assignments, source);
+        PlanMatchPattern output = output(outputs, strict);
+        assertMinimallyOptimizedPlanDoesNotMatch(sql, output);
     }
 
     @Test(expectedExceptions = {IllegalStateException.class}, expectedExceptionsMessageRegExp = ".*already bound to expression.*")

diff --git a/presto-main/src/test/java/com/facebook/presto/sql/planner/assertions/BasePlanTest.java b/presto-main/src/test/java/com/facebook/presto/sql/planner/assertions/BasePlanTest.java
@@ -193,6 +193,20 @@ protected void assertPlan(String sql, Session session, Optimizer.PlanStage stage
         });
     }
 
+    private void assertPlanDoesNotMatch(String sql, Session session, Optimizer.PlanStage stage, PlanMatchPattern pattern, List<PlanOptimizer> optimizers)
+    {
+        queryRunner.inTransaction(session, transactionSession -> {
+            Plan actualPlan = queryRunner.createPlan(
+                    transactionSession,
+                    sql,
+                    optimizers,
+                    stage,
+                    WarningCollector.NOOP);
+            PlanAssert.assertPlanDoesNotMatch(transactionSession, queryRunner.getMetadata(), queryRunner.getStatsCalculator(), actualPlan, pattern);
+            return null;
+        });
+    }
+
     protected void assertDistributedPlan(String sql, PlanMatchPattern pattern)
     {
         assertDistributedPlan(sql, getQueryRunner().getDefaultSession(), pattern);
@@ -218,6 +232,22 @@ protected void assertMinimallyOptimizedPlan(@Language("SQL") String sql, PlanMat
         assertPlan(sql, queryRunner.getDefaultSession(), Optimizer.PlanStage.OPTIMIZED, pattern, optimizers);
     }
 
+
+    protected void assertMinimallyOptimizedPlanDoesNotMatch(@Language("SQL") String sql, PlanMatchPattern pattern)
+    {
+        List<PlanOptimizer> optimizers = ImmutableList.of(
+                new UnaliasSymbolReferences(queryRunner.getMetadata().getFunctionAndTypeManager()),
+                new PruneUnreferencedOutputs(),
+                new IterativeOptimizer(
+                        getMetadata(),
+                        new RuleStatsRecorder(),
+                        queryRunner.getStatsCalculator(),
+                        queryRunner.getCostCalculator(),
+                        ImmutableSet.of(new RemoveRedundantIdentityProjections())));
+
+        assertPlanDoesNotMatch(sql, queryRunner.getDefaultSession(), Optimizer.PlanStage.OPTIMIZED, pattern, optimizers);
+    }
+
     protected void assertPlanWithSession(@Language("SQL") String sql, Session session, boolean forceSingleNode, PlanMatchPattern pattern)
     {
         queryRunner.inTransaction(session, transactionSession -> {

diff --git a/presto-tests/src/test/java/com/facebook/presto/tests/TestMetadataManager.java b/presto-tests/src/test/java/com/facebook/presto/tests/TestMetadataManager.java
@@ -115,7 +115,7 @@ public void testMetadataIsClearedAfterQueryFailed()
             queryRunner.execute(sql);
             fail("expected exception");
         }
-        catch (Throwable t) {
+        catch (RuntimeException t) {
             // query should fail
         }