diff --git a/presto-common/src/main/java/com/facebook/presto/common/ErrorCode.java b/presto-common/src/main/java/com/facebook/presto/common/ErrorCode.java index cb810d5d543ff..f6803c686f0c4 100644 --- a/presto-common/src/main/java/com/facebook/presto/common/ErrorCode.java +++ b/presto-common/src/main/java/com/facebook/presto/common/ErrorCode.java @@ -30,6 +30,7 @@ public final class ErrorCode private final String name; private final ErrorType type; private final boolean retriable; + private final boolean catchableByTry; @JsonCreator @ThriftConstructor @@ -37,7 +38,8 @@ public ErrorCode( @JsonProperty("code") int code, @JsonProperty("name") String name, @JsonProperty("type") ErrorType type, - @JsonProperty("retriable") boolean retriable) + @JsonProperty("retriable") boolean retriable, + @JsonProperty("catchableByTry") boolean catchableByTry) { if (code < 0) { throw new IllegalArgumentException("code is negative"); @@ -46,11 +48,17 @@ public ErrorCode( this.name = requireNonNull(name, "name is null"); this.type = requireNonNull(type, "type is null"); this.retriable = retriable; + this.catchableByTry = catchableByTry; } public ErrorCode(int code, String name, ErrorType type) { - this(code, name, type, false); + this(code, name, type, false, false); + } + + public ErrorCode(int code, String name, ErrorType type, boolean retriable) + { + this(code, name, type, retriable, false); } @JsonProperty @@ -81,6 +89,13 @@ public boolean isRetriable() return retriable; } + @JsonProperty + @ThriftField(5) + public boolean isCatchableByTry() + { + return catchableByTry; + } + @Override public String toString() { diff --git a/presto-common/src/test/java/com/facebook/presto/common/TestErrorCode.java b/presto-common/src/test/java/com/facebook/presto/common/TestErrorCode.java new file mode 100644 index 0000000000000..9f256c8721e2e --- /dev/null +++ b/presto-common/src/test/java/com/facebook/presto/common/TestErrorCode.java @@ -0,0 +1,93 @@ +/* + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package com.facebook.presto.common; + +import org.testng.annotations.Test; + +import static com.facebook.presto.common.ErrorType.INTERNAL_ERROR; +import static com.facebook.presto.common.ErrorType.USER_ERROR; +import static org.testng.Assert.assertEquals; +import static org.testng.Assert.assertFalse; +import static org.testng.Assert.assertTrue; + +public class TestErrorCode +{ + @Test + public void testCatchableByTryFlag() + { + // Error code with catchableByTry = true + ErrorCode catchable = new ErrorCode(1, "CATCHABLE_ERROR", USER_ERROR, false, true); + assertTrue(catchable.isCatchableByTry()); + assertFalse(catchable.isRetriable()); + + // Error code with catchableByTry = false + ErrorCode notCatchable = new ErrorCode(2, "NOT_CATCHABLE_ERROR", USER_ERROR, false, false); + assertFalse(notCatchable.isCatchableByTry()); + } + + @Test + public void testBackwardCompatibleConstructorDefaultsToNotCatchable() + { + // 3-parameter constructor should default catchableByTry to false + ErrorCode threeParam = new ErrorCode(1, "TEST_ERROR", USER_ERROR); + assertFalse(threeParam.isCatchableByTry()); + assertFalse(threeParam.isRetriable()); + + // 4-parameter constructor should default catchableByTry to false + ErrorCode fourParam = new ErrorCode(2, "TEST_ERROR_2", USER_ERROR, true); + assertFalse(fourParam.isCatchableByTry()); + assertTrue(fourParam.isRetriable()); + } + + @Test + public void testErrorCodeProperties() + { + ErrorCode errorCode = new ErrorCode(123, "TEST_ERROR", INTERNAL_ERROR, true, true); + + assertEquals(errorCode.getCode(), 123); + assertEquals(errorCode.getName(), "TEST_ERROR"); + assertEquals(errorCode.getType(), INTERNAL_ERROR); + assertTrue(errorCode.isRetriable()); + assertTrue(errorCode.isCatchableByTry()); + } + + @Test + public void testEquality() + { + // ErrorCode equality is based on code only (existing behavior) + ErrorCode error1 = new ErrorCode(1, "ERROR_A", USER_ERROR, false, true); + ErrorCode error2 = new ErrorCode(1, "ERROR_B", INTERNAL_ERROR, true, false); + + assertEquals(error1, error2); + assertEquals(error1.hashCode(), error2.hashCode()); + } + + @Test(expectedExceptions = IllegalArgumentException.class) + public void testNegativeCodeThrows() + { + new ErrorCode(-1, "NEGATIVE_CODE", USER_ERROR, false, false); + } + + @Test(expectedExceptions = NullPointerException.class) + public void testNullNameThrows() + { + new ErrorCode(1, null, USER_ERROR, false, false); + } + + @Test(expectedExceptions = NullPointerException.class) + public void testNullTypeThrows() + { + new ErrorCode(1, "TEST", null, false, false); + } +} diff --git a/presto-main-base/src/main/java/com/facebook/presto/operator/scalar/TryFunction.java b/presto-main-base/src/main/java/com/facebook/presto/operator/scalar/TryFunction.java index 3fe0d689e9536..9aebc92850fd3 100644 --- a/presto-main-base/src/main/java/com/facebook/presto/operator/scalar/TryFunction.java +++ b/presto-main-base/src/main/java/com/facebook/presto/operator/scalar/TryFunction.java @@ -26,10 +26,6 @@ import java.util.function.Supplier; -import static com.facebook.presto.spi.StandardErrorCode.DIVISION_BY_ZERO; -import static com.facebook.presto.spi.StandardErrorCode.INVALID_CAST_ARGUMENT; -import static com.facebook.presto.spi.StandardErrorCode.INVALID_FUNCTION_ARGUMENT; -import static com.facebook.presto.spi.StandardErrorCode.NUMERIC_VALUE_OUT_OF_RANGE; import static com.facebook.presto.spi.function.SqlFunctionVisibility.HIDDEN; @Description("internal try function for desugaring TRY") @@ -162,11 +158,7 @@ public static T evaluate(Supplier supplier, T defaultValue) private static void propagateIfUnhandled(PrestoException e) throws PrestoException { - int errorCode = e.getErrorCode().getCode(); - if (errorCode == DIVISION_BY_ZERO.toErrorCode().getCode() - || errorCode == INVALID_CAST_ARGUMENT.toErrorCode().getCode() - || errorCode == INVALID_FUNCTION_ARGUMENT.toErrorCode().getCode() - || errorCode == NUMERIC_VALUE_OUT_OF_RANGE.toErrorCode().getCode()) { + if (e.getErrorCode().isCatchableByTry()) { return; } diff --git a/presto-spi/src/main/java/com/facebook/presto/spi/StandardErrorCode.java b/presto-spi/src/main/java/com/facebook/presto/spi/StandardErrorCode.java index 426aab311608d..198c503ab2ddd 100644 --- a/presto-spi/src/main/java/com/facebook/presto/spi/StandardErrorCode.java +++ b/presto-spi/src/main/java/com/facebook/presto/spi/StandardErrorCode.java @@ -30,9 +30,9 @@ public enum StandardErrorCode PERMISSION_DENIED(0x0000_0004, USER_ERROR), NOT_FOUND(0x0000_0005, USER_ERROR), FUNCTION_NOT_FOUND(0x0000_0006, USER_ERROR), - INVALID_FUNCTION_ARGUMENT(0x0000_0007, USER_ERROR), // caught by TRY - DIVISION_BY_ZERO(0x0000_0008, USER_ERROR), // caught by TRY - INVALID_CAST_ARGUMENT(0x0000_0009, USER_ERROR), // caught by TRY + INVALID_FUNCTION_ARGUMENT(0x0000_0007, USER_ERROR, false, true), // caught by TRY + DIVISION_BY_ZERO(0x0000_0008, USER_ERROR, false, true), // caught by TRY + INVALID_CAST_ARGUMENT(0x0000_0009, USER_ERROR, false, true), // caught by TRY OPERATOR_NOT_FOUND(0x0000_000A, USER_ERROR), INVALID_VIEW(0x0000_000B, USER_ERROR), ALREADY_EXISTS(0x0000_000C, USER_ERROR), @@ -42,7 +42,7 @@ public enum StandardErrorCode CONSTRAINT_VIOLATION(0x0000_0010, USER_ERROR), TRANSACTION_CONFLICT(0x0000_0011, USER_ERROR), INVALID_TABLE_PROPERTY(0x0000_0012, USER_ERROR), - NUMERIC_VALUE_OUT_OF_RANGE(0x0000_0013, USER_ERROR), // caught by TRY + NUMERIC_VALUE_OUT_OF_RANGE(0x0000_0013, USER_ERROR, false, true), // caught by TRY UNKNOWN_TRANSACTION(0x0000_0014, USER_ERROR), NOT_IN_TRANSACTION(0x0000_0015, USER_ERROR), TRANSACTION_ALREADY_ABORTED(0x0000_0016, USER_ERROR), @@ -163,12 +163,17 @@ public enum StandardErrorCode StandardErrorCode(int code, ErrorType type) { - this(code, type, false); + this(code, type, false, false); } StandardErrorCode(int code, ErrorType type, boolean retriable) { - errorCode = new ErrorCode(code, name(), type, retriable); + this(code, type, retriable, false); + } + + StandardErrorCode(int code, ErrorType type, boolean retriable, boolean catchableByTry) + { + errorCode = new ErrorCode(code, name(), type, retriable, catchableByTry); } @Override diff --git a/presto-spi/src/test/java/com/facebook/presto/spi/TestStandardErrorCode.java b/presto-spi/src/test/java/com/facebook/presto/spi/TestStandardErrorCode.java index f0403b080398a..eebeb62c5673c 100644 --- a/presto-spi/src/test/java/com/facebook/presto/spi/TestStandardErrorCode.java +++ b/presto-spi/src/test/java/com/facebook/presto/spi/TestStandardErrorCode.java @@ -21,10 +21,17 @@ import static com.facebook.airlift.testing.Assertions.assertGreaterThan; import static com.facebook.airlift.testing.Assertions.assertLessThan; +import static com.facebook.presto.spi.StandardErrorCode.DIVISION_BY_ZERO; import static com.facebook.presto.spi.StandardErrorCode.GENERIC_INSUFFICIENT_RESOURCES; import static com.facebook.presto.spi.StandardErrorCode.GENERIC_INTERNAL_ERROR; +import static com.facebook.presto.spi.StandardErrorCode.GENERIC_USER_ERROR; +import static com.facebook.presto.spi.StandardErrorCode.INVALID_CAST_ARGUMENT; +import static com.facebook.presto.spi.StandardErrorCode.INVALID_FUNCTION_ARGUMENT; +import static com.facebook.presto.spi.StandardErrorCode.NUMERIC_VALUE_OUT_OF_RANGE; +import static com.facebook.presto.spi.StandardErrorCode.SYNTAX_ERROR; import static java.util.Arrays.asList; import static org.testng.Assert.assertEquals; +import static org.testng.Assert.assertFalse; import static org.testng.Assert.assertTrue; public class TestStandardErrorCode @@ -72,4 +79,24 @@ private static int code(StandardErrorCode error) { return error.toErrorCode().getCode(); } + + @Test + public void testCatchableByTryErrors() + { + // These errors should be caught by TRY() and return NULL + assertTrue(DIVISION_BY_ZERO.toErrorCode().isCatchableByTry()); + assertTrue(INVALID_CAST_ARGUMENT.toErrorCode().isCatchableByTry()); + assertTrue(INVALID_FUNCTION_ARGUMENT.toErrorCode().isCatchableByTry()); + assertTrue(NUMERIC_VALUE_OUT_OF_RANGE.toErrorCode().isCatchableByTry()); + } + + @Test + public void testNonCatchableByTryErrors() + { + // These errors should NOT be caught by TRY() - they should propagate + assertFalse(GENERIC_INTERNAL_ERROR.toErrorCode().isCatchableByTry()); + assertFalse(GENERIC_USER_ERROR.toErrorCode().isCatchableByTry()); + assertFalse(SYNTAX_ERROR.toErrorCode().isCatchableByTry()); + assertFalse(GENERIC_INSUFFICIENT_RESOURCES.toErrorCode().isCatchableByTry()); + } }