Output verifier events for queries being skipped by pre-filters

presto-verifier/src/main/java/com/facebook/presto/verifier/event/VerifierQueryEvent.java

@@ -24,6 +24,7 @@
 import java.util.List;
 import java.util.Optional;
 
+import static com.facebook.presto.verifier.event.VerifierQueryEvent.EventStatus.SKIPPED;
 import static java.util.Objects.requireNonNull;
 
 @Immutable
@@ -66,8 +67,8 @@ public VerifierQueryEvent(
             Optional<SkippedReason> skippedReason,
             Optional<DeterminismAnalysis> determinismAnalysis,
             Optional<String> resolveMessage,
-            QueryInfo controlQueryInfo,
-            QueryInfo testQueryInfo,
+            Optional<QueryInfo> controlQueryInfo,
+            Optional<QueryInfo> testQueryInfo,
             Optional<String> errorCode,
             Optional<String> errorMessage,
             Optional<QueryFailure> finalQueryFailure,
@@ -81,14 +82,36 @@ public VerifierQueryEvent(
         this.deterministic = determinismAnalysis.filter(d -> !d.isUnknown()).map(DeterminismAnalysis::isDeterministic).orElse(null);
         this.determinismAnalysis = determinismAnalysis.map(DeterminismAnalysis::name).orElse(null);
         this.resolveMessage = resolveMessage.orElse(null);
-        this.controlQueryInfo = requireNonNull(controlQueryInfo, "controlQueryInfo is null");
-        this.testQueryInfo = requireNonNull(testQueryInfo, "testQueryInfo is null");
+        this.controlQueryInfo = controlQueryInfo.orElse(null);
+        this.testQueryInfo = testQueryInfo.orElse(null);
         this.errorCode = errorCode.orElse(null);
         this.errorMessage = errorMessage.orElse(null);
         this.finalQueryFailure = finalQueryFailure.orElse(null);
         this.queryFailures = ImmutableList.copyOf(queryFailures);
     }
 
+    public static VerifierQueryEvent skipped(
+            String suite,
+            String testId,
+            String name,
+            SkippedReason skippedReason)
+    {
+        return new VerifierQueryEvent(
+                suite,
+                testId,
+                name,
+                SKIPPED,
+                Optional.of(skippedReason),
+                Optional.empty(),
+                Optional.empty(),
+                Optional.empty(),
+                Optional.empty(),
+                Optional.empty(),
+                Optional.empty(),
+                Optional.empty(),
+                ImmutableList.of());
+    }
+
     @EventField
     public String getSuite()
     {

presto-verifier/src/main/java/com/facebook/presto/verifier/framework/AbstractVerification.java

@@ -270,20 +270,20 @@ else if (skippedReason.isPresent()) {
                 skippedReason,
                 determinismAnalysis,
                 resolveMessage,
-                buildQueryInfo(
+                Optional.of(buildQueryInfo(
                         sourceQuery.getControlConfiguration(),
                         sourceQuery.getControlQuery(),
                         verificationResult.map(VerificationResult::getControlChecksumQueryId),
                         verificationResult.map(VerificationResult::getControlChecksumQuery),
                         control,
-                        controlStats),
-                buildQueryInfo(
+                        controlStats)),
+                Optional.of(buildQueryInfo(
                         sourceQuery.getTestConfiguration(),
                         sourceQuery.getTestQuery(),
                         verificationResult.map(VerificationResult::getTestChecksumQueryId),
                         verificationResult.map(VerificationResult::getTestChecksumQuery),
                         test,
-                        testStats),
+                        testStats)),
                 errorCode,
                 Optional.ofNullable(errorMessage),
                 queryException.map(QueryException::toQueryFailure),

presto-verifier/src/main/java/com/facebook/presto/verifier/framework/SkippedReason.java

@@ -15,6 +15,11 @@
 
 public enum SkippedReason
 {
+    SYNTAX_ERROR,
+    UNSUPPORTED_QUERY_TYPE,
+    MISMATCHED_QUERY_TYPE,
+    CUSTOM_FILTER,
+
     CONTROL_QUERY_FAILED,
     CONTROL_SETUP_QUERY_FAILED,
     CONTROL_QUERY_TIMED_OUT,

presto-verifier/src/main/java/com/facebook/presto/verifier/framework/VerificationManager.java

@@ -32,7 +32,9 @@
 
 import java.io.Closeable;
 import java.io.IOException;
+import java.util.ArrayList;
 import java.util.EnumMap;
+import java.util.Iterator;
 import java.util.List;
 import java.util.Map;
 import java.util.Optional;
@@ -50,6 +52,10 @@
 import static com.facebook.presto.verifier.event.VerifierQueryEvent.EventStatus.SKIPPED;
 import static com.facebook.presto.verifier.event.VerifierQueryEvent.EventStatus.SUCCEEDED;
 import static com.facebook.presto.verifier.framework.QueryType.Category.DATA_PRODUCING;
+import static com.facebook.presto.verifier.framework.SkippedReason.CUSTOM_FILTER;
+import static com.facebook.presto.verifier.framework.SkippedReason.MISMATCHED_QUERY_TYPE;
+import static com.facebook.presto.verifier.framework.SkippedReason.SYNTAX_ERROR;
+import static com.facebook.presto.verifier.framework.SkippedReason.UNSUPPORTED_QUERY_TYPE;
 import static com.facebook.presto.verifier.framework.VerifierUtil.PARSING_OPTIONS;
 import static com.facebook.presto.verifier.prestoaction.PrestoExceptionClassifier.shouldResubmit;
 import static com.google.common.collect.ImmutableList.toImmutableList;
@@ -71,6 +77,8 @@ public class VerificationManager
     private final QueryConfigurationOverrides controlOverrides;
     private final QueryConfigurationOverrides testOverrides;
 
+    private final String testId;
+
     private final Optional<Set<String>> whitelist;
     private final Optional<Set<String>> blacklist;
     private final int maxConcurrency;
@@ -104,6 +112,8 @@ public VerificationManager(
         this.controlOverrides = requireNonNull(controlOverrides, "controlOverrides is null");
         this.testOverrides = requireNonNull(testOverrides, "testOverride is null");
 
+        this.testId = requireNonNull(config.getTestId(), "testId is null");
+
         this.whitelist = requireNonNull(config.getWhitelist(), "whitelist is null");
         this.blacklist = requireNonNull(config.getBlacklist(), "blacklist is null");
         this.maxConcurrency = config.getMaxConcurrency();
@@ -219,14 +229,23 @@ private List<SourceQuery> filterQueryType(List<SourceQuery> sourceQueries)
             try {
                 QueryType controlQueryType = QueryType.of(sqlParser.createStatement(sourceQuery.getControlQuery(), PARSING_OPTIONS));
                 QueryType testQueryType = QueryType.of(sqlParser.createStatement(sourceQuery.getTestQuery(), PARSING_OPTIONS));
-                if (controlQueryType == testQueryType && controlQueryType.getCategory() == DATA_PRODUCING) {
+
+                if (controlQueryType != testQueryType) {
+                    postEvent(VerifierQueryEvent.skipped(sourceQuery.getSuite(), testId, sourceQuery.getName(), MISMATCHED_QUERY_TYPE));
+                }
+                else if (controlQueryType.getCategory() != DATA_PRODUCING) {
+                    postEvent(VerifierQueryEvent.skipped(sourceQuery.getSuite(), testId, sourceQuery.getName(), UNSUPPORTED_QUERY_TYPE));
+                }
+                else {
                     selected.add(sourceQuery);
                 }
             }
             catch (ParsingException e) {
                 log.warn("Failed to parse query: %s", sourceQuery.getName());
+                postEvent(VerifierQueryEvent.skipped(sourceQuery.getSuite(), testId, sourceQuery.getName(), SYNTAX_ERROR));
             }
             catch (UnsupportedQueryTypeException ignored) {
+                postEvent(VerifierQueryEvent.skipped(sourceQuery.getSuite(), testId, sourceQuery.getName(), UNSUPPORTED_QUERY_TYPE));
             }
         }
         List<SourceQuery> selectQueries = selected.build();
@@ -241,10 +260,16 @@ private List<SourceQuery> applyCustomFilters(List<SourceQuery> sourceQueries)
         }
 
         log.info("Applying custom query filters");
+        sourceQueries = new ArrayList<>(sourceQueries);
         for (Predicate<SourceQuery> filter : customQueryFilters) {
-            sourceQueries = sourceQueries.stream()
-                    .filter(filter::test)
-                    .collect(toImmutableList());
+            Iterator<SourceQuery> iterator = sourceQueries.iterator();
+            while (iterator.hasNext()) {
+                SourceQuery sourceQuery = iterator.next();
+                if (!filter.test(sourceQuery)) {
+                    iterator.remove();
+                    postEvent(VerifierQueryEvent.skipped(sourceQuery.getSuite(), testId, sourceQuery.getName(), CUSTOM_FILTER));
+                }
+            }
             log.info("Applying custom filter %s... Remaining queries: %s", filter.getClass().getSimpleName(), sourceQueries.size());
         }
         return sourceQueries;
@@ -280,9 +305,7 @@ private void reportProgressUntilFinished()
                 }
                 else {
                     statusCount.compute(EventStatus.valueOf(event.get().getStatus()), (status, count) -> count == null ? 1 : count + 1);
-                    for (EventClient eventClient : eventClients) {
-                        eventClient.post(event.get());
-                    }
+                    postEvent(event.get());
                 }
 
                 double progress = ((double) completed) / queriesSubmitted.get() * 100;
@@ -306,4 +329,11 @@ private void reportProgressUntilFinished()
             }
         }
     }
+
+    private void postEvent(VerifierQueryEvent event)
+    {
+        for (EventClient eventClient : eventClients) {
+            eventClient.post(event);
+        }
+    }
 }

presto-verifier/src/test/java/com/facebook/presto/verifier/framework/TestVerificationManager.java

@@ -13,6 +13,7 @@
  */
 package com.facebook.presto.verifier.framework;
 
+import com.facebook.airlift.event.client.AbstractEventClient;
 import com.facebook.airlift.json.JsonCodec;
 import com.facebook.presto.jdbc.QueryStats;
 import com.facebook.presto.spi.ErrorCodeSupplier;
@@ -25,6 +26,7 @@
 import com.facebook.presto.verifier.checksum.FloatingPointColumnValidator;
 import com.facebook.presto.verifier.checksum.OrderableArrayColumnValidator;
 import com.facebook.presto.verifier.checksum.SimpleColumnValidator;
+import com.facebook.presto.verifier.event.VerifierQueryEvent;
 import com.facebook.presto.verifier.prestoaction.PrestoAction;
 import com.facebook.presto.verifier.prestoaction.PrestoResourceClient;
 import com.facebook.presto.verifier.resolver.FailureResolverConfig;
@@ -33,17 +35,24 @@
 import com.google.common.collect.ImmutableList;
 import com.google.common.collect.ImmutableMap;
 import com.google.common.collect.ImmutableSet;
+import org.testng.annotations.BeforeMethod;
 import org.testng.annotations.Test;
 
+import java.util.ArrayList;
 import java.util.List;
 import java.util.Optional;
 
 import static com.facebook.presto.hive.HiveErrorCode.HIVE_PARTITION_DROPPED_DURING_QUERY;
 import static com.facebook.presto.spi.StandardErrorCode.GENERIC_INTERNAL_ERROR;
 import static com.facebook.presto.sql.parser.IdentifierSymbol.AT_SIGN;
 import static com.facebook.presto.sql.parser.IdentifierSymbol.COLON;
+import static com.facebook.presto.verifier.event.VerifierQueryEvent.EventStatus.SKIPPED;
 import static com.facebook.presto.verifier.framework.ClusterType.CONTROL;
 import static com.facebook.presto.verifier.framework.ClusterType.TEST;
+import static com.facebook.presto.verifier.framework.SkippedReason.MISMATCHED_QUERY_TYPE;
+import static com.facebook.presto.verifier.framework.SkippedReason.SYNTAX_ERROR;
+import static com.facebook.presto.verifier.framework.SkippedReason.UNSUPPORTED_QUERY_TYPE;
+import static com.google.common.base.Preconditions.checkArgument;
 import static java.util.Objects.requireNonNull;
 import static org.testng.Assert.assertEquals;
 
@@ -85,6 +94,24 @@ public <V> V getJsonResponse(String path, JsonCodec<V> responseCodec)
         }
     }
 
+    private static class MockEventClient
+            extends AbstractEventClient
+    {
+        private final List<VerifierQueryEvent> events = new ArrayList<>();
+
+        @Override
+        protected <T> void postEvent(T event)
+        {
+            checkArgument(event instanceof VerifierQueryEvent);
+            this.events.add((VerifierQueryEvent) event);
+        }
+
+        public List<VerifierQueryEvent> getEvents()
+        {
+            return events;
+        }
+    }
+
     private static final String SUITE = "test-suite";
     private static final String NAME = "test-query";
     private static final QualifiedName TABLE_PREFIX = QualifiedName.of("tmp_verifier");
@@ -99,6 +126,14 @@ public <V> V getJsonResponse(String path, JsonCodec<V> responseCodec)
             QUERY_CONFIGURATION);
     private static final VerifierConfig VERIFIER_CONFIG = new VerifierConfig().setTestId("test");
 
+    private MockEventClient eventClient;
+
+    @BeforeMethod
+    public void setup()
+    {
+        this.eventClient = new MockEventClient();
+    }
+
     @Test
     public void testFailureRequeued()
     {
@@ -126,7 +161,47 @@ public void testFailureRequeueDisabled()
         assertEquals(manager.getQueriesSubmitted().get(), 1);
     }
 
-    private static VerificationManager getVerificationManager(List<SourceQuery> sourceQueries, PrestoAction prestoAction, VerifierConfig verifierConfig)
+    @Test
+    public void testFilters()
+    {
+        List<SourceQuery> queries = ImmutableList.of(
+                createSourceQuery("q1", "CREATE TABLE t1 (x int)", "CREATE TABLE t1 (x int)"),
+                createSourceQuery("q2", "CREATE TABLE t1 (x int)", "CREATE TABLE t1 (x int)"),
+                createSourceQuery("q3", "CREATE TABLE t1 (x int)", "CREATE TABLE t1 (x int)"),
+                createSourceQuery("q4", "SHOW FUNCTIONS", "SHOW FUNCTIONS"),
+                createSourceQuery("q5", "SELECT * FROM t1", "INSERT INTO t2 SELECT * FROM t1"),
+                createSourceQuery("q6", "SELECT * FROM t1", "SELECT FROM t1"));
+        VerificationManager manager = getVerificationManager(
+                queries,
+                new MockPrestoAction(GENERIC_INTERNAL_ERROR),
+                new VerifierConfig()
+                        .setTestId("test")
+                        .setWhitelist("q2,q3,q4,q5,q6")
+                        .setBlacklist("q2"));
+        manager.start();
+        assertEquals(manager.getQueriesSubmitted().get(), 0);
+
+        List<VerifierQueryEvent> events = eventClient.getEvents();
+        assertEquals(events.size(), 4);
+        assertSkippedEvent(events.get(0), "q3", UNSUPPORTED_QUERY_TYPE);
+        assertSkippedEvent(events.get(1), "q4", UNSUPPORTED_QUERY_TYPE);
+        assertSkippedEvent(events.get(2), "q5", MISMATCHED_QUERY_TYPE);
+        assertSkippedEvent(events.get(3), "q6", SYNTAX_ERROR);
+    }
+
+    private static SourceQuery createSourceQuery(String name, String controlQuery, String testQuery)
+    {
+        return new SourceQuery(SUITE, name, controlQuery, testQuery, QUERY_CONFIGURATION, QUERY_CONFIGURATION);
+    }
+
+    private static void assertSkippedEvent(VerifierQueryEvent event, String name, SkippedReason skippedReason)
+    {
+        assertEquals(event.getName(), name);
+        assertEquals(event.getStatus(), SKIPPED.name());
+        assertEquals(event.getSkippedReason(), skippedReason.name());
+    }
+
+    private VerificationManager getVerificationManager(List<SourceQuery> sourceQueries, PrestoAction prestoAction, VerifierConfig verifierConfig)
     {
         return new VerificationManager(
                 () -> sourceQueries,
@@ -141,7 +216,7 @@ private static VerificationManager getVerificationManager(List<SourceQuery> sour
                         new TypeRegistry(),
                         new FailureResolverConfig().setEnabled(false)),
                 SQL_PARSER,
-                ImmutableSet.of(),
+                ImmutableSet.of(eventClient),
                 ImmutableList.of(),
                 new QueryConfigurationOverridesConfig(),
                 new QueryConfigurationOverridesConfig(),