From 621adb8f8239d6cdf3671981a325a4bc4deea170 Mon Sep 17 00:00:00 2001 From: Leiqing Cai Date: Wed, 11 Dec 2019 21:04:10 -0800 Subject: [PATCH] Skip verification when checksum query fail to compile When the target table has too many floating point columns, checksum query ends up with too many columns with window functions, causing COMPILER_FAILURE error code. This case cannot be properly handled by the Verifier and thus should be skipped for now. --- .../verifier/framework/AbstractVerification.java | 16 ++++++++++++++-- .../presto/verifier/framework/SkippedReason.java | 4 +++- .../verifier/framework/TestDataVerification.java | 4 +++- 3 files changed, 20 insertions(+), 4 deletions(-) diff --git a/presto-verifier/src/main/java/com/facebook/presto/verifier/framework/AbstractVerification.java b/presto-verifier/src/main/java/com/facebook/presto/verifier/framework/AbstractVerification.java index a790a1a5a0de8..3133e88301537 100644 --- a/presto-verifier/src/main/java/com/facebook/presto/verifier/framework/AbstractVerification.java +++ b/presto-verifier/src/main/java/com/facebook/presto/verifier/framework/AbstractVerification.java @@ -33,6 +33,7 @@ import java.util.List; import java.util.Optional; +import static com.facebook.presto.spi.StandardErrorCode.COMPILER_ERROR; import static com.facebook.presto.spi.StandardErrorCode.EXCEEDED_TIME_LIMIT; import static com.facebook.presto.verifier.event.VerifierQueryEvent.EventStatus.FAILED; import static com.facebook.presto.verifier.event.VerifierQueryEvent.EventStatus.FAILED_RESOLVED; @@ -40,6 +41,7 @@ import static com.facebook.presto.verifier.event.VerifierQueryEvent.EventStatus.SUCCEEDED; 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.QueryStage.CHECKSUM; import static com.facebook.presto.verifier.framework.QueryStage.CONTROL_MAIN; import static com.facebook.presto.verifier.framework.QueryStage.DETERMINISM_ANALYSIS; import static com.facebook.presto.verifier.framework.QueryStage.TEST_MAIN; @@ -51,6 +53,7 @@ import static com.facebook.presto.verifier.framework.SkippedReason.CONTROL_SETUP_QUERY_FAILED; import static com.facebook.presto.verifier.framework.SkippedReason.FAILED_BEFORE_CONTROL_QUERY; import static com.facebook.presto.verifier.framework.SkippedReason.NON_DETERMINISTIC; +import static com.facebook.presto.verifier.framework.SkippedReason.VERIFIER_LIMITATION; import static com.google.common.base.Preconditions.checkState; import static com.google.common.base.Throwables.getStackTraceAsString; import static com.google.common.collect.ImmutableList.toImmutableList; @@ -239,7 +242,7 @@ private VerifierQueryEvent buildEvent( } EventStatus status; - Optional skippedReason = getSkippedReason(controlState, determinismAnalysis); + Optional skippedReason = getSkippedReason(controlState, determinismAnalysis, queryException); Optional resolveMessage = Optional.empty(); if (succeeded) { status = SUCCEEDED; @@ -336,7 +339,10 @@ protected static List formatSqls(List statements) .collect(toImmutableList()); } - private static Optional getSkippedReason(QueryState controlState, Optional determinismAnalysis) + private static Optional getSkippedReason( + QueryState controlState, + Optional determinismAnalysis, + Optional queryException) { switch (controlState) { case FAILED: @@ -351,6 +357,12 @@ private static Optional getSkippedReason(QueryState controlState, if (determinismAnalysis.isPresent() && determinismAnalysis.get().isNonDeterministic()) { return Optional.of(NON_DETERMINISTIC); } + if (queryException.isPresent() && + queryException.get().getQueryStage().equals(CHECKSUM) && + queryException.get().getPrestoErrorCode().isPresent() && + queryException.get().getPrestoErrorCode().get().equals(COMPILER_ERROR)) { + return Optional.of(VERIFIER_LIMITATION); + } return Optional.empty(); } diff --git a/presto-verifier/src/main/java/com/facebook/presto/verifier/framework/SkippedReason.java b/presto-verifier/src/main/java/com/facebook/presto/verifier/framework/SkippedReason.java index 4274db5dad924..02ba16c87eb4c 100644 --- a/presto-verifier/src/main/java/com/facebook/presto/verifier/framework/SkippedReason.java +++ b/presto-verifier/src/main/java/com/facebook/presto/verifier/framework/SkippedReason.java @@ -24,5 +24,7 @@ public enum SkippedReason CONTROL_SETUP_QUERY_FAILED, CONTROL_QUERY_TIMED_OUT, FAILED_BEFORE_CONTROL_QUERY, - NON_DETERMINISTIC + NON_DETERMINISTIC, + + VERIFIER_LIMITATION, } diff --git a/presto-verifier/src/test/java/com/facebook/presto/verifier/framework/TestDataVerification.java b/presto-verifier/src/test/java/com/facebook/presto/verifier/framework/TestDataVerification.java index e53412d98008e..b3a362530b243 100644 --- a/presto-verifier/src/test/java/com/facebook/presto/verifier/framework/TestDataVerification.java +++ b/presto-verifier/src/test/java/com/facebook/presto/verifier/framework/TestDataVerification.java @@ -59,6 +59,7 @@ import static com.facebook.presto.verifier.framework.SkippedReason.CONTROL_SETUP_QUERY_FAILED; import static com.facebook.presto.verifier.framework.SkippedReason.FAILED_BEFORE_CONTROL_QUERY; import static com.facebook.presto.verifier.framework.SkippedReason.NON_DETERMINISTIC; +import static com.facebook.presto.verifier.framework.SkippedReason.VERIFIER_LIMITATION; import static com.google.common.collect.ImmutableList.toImmutableList; import static java.lang.String.format; import static java.util.regex.Pattern.DOTALL; @@ -280,7 +281,8 @@ public void testChecksumQueryFailed() Optional event = createVerification(query, query).run(); assertTrue(event.isPresent()); - assertEquals(event.get().getStatus(), FAILED.name()); + assertEquals(event.get().getStatus(), SKIPPED.name()); + assertEquals(event.get().getSkippedReason(), VERIFIER_LIMITATION.name()); assertEquals(event.get().getErrorCode(), "PRESTO(COMPILER_ERROR)"); assertNotNull(event.get().getControlQueryInfo().getChecksumQuery()); assertNotNull(event.get().getControlQueryInfo().getChecksumQueryId());