diff options
6 files changed, 56 insertions, 42 deletions
diff --git a/common/src/main/java/org/apache/drill/common/exceptions/UserException.java b/common/src/main/java/org/apache/drill/common/exceptions/UserException.java index 87b3fd4e1..dd4fd36b8 100644 --- a/common/src/main/java/org/apache/drill/common/exceptions/UserException.java +++ b/common/src/main/java/org/apache/drill/common/exceptions/UserException.java @@ -77,6 +77,14 @@ public class UserException extends DrillRuntimeException { * <p>The cause message will be used unless {@link Builder#message(String, Object...)} is called. * <p>If the wrapped exception is, or wraps, a user exception it will be returned by {@link Builder#build(Logger)} * instead of creating a new exception. Any added context will be added to the user exception as well. + * <p> + * This exception, previously deprecated, has been repurposed to indicate unspecified + * errors. In particular, the case in which a lower level bit of code throws an + * exception other than UserException. The catching code then only knows "something went + * wrong", but not enough information to categorize the error. + * <p> + * System errors also indicate illegal internal states, missing functionality, and other + * code-related errors -- all of which "should never occur." * * @see org.apache.drill.exec.proto.UserBitShared.DrillPBError.ErrorType#SYSTEM * @@ -84,10 +92,8 @@ public class UserException extends DrillRuntimeException { * returned by the builder instead of creating a new user exception * @return user exception builder * - * @deprecated This method should never need to be used explicitly, unless you are passing the exception to the - * Rpc layer or UserResultListener.submitFailed() */ - @Deprecated + public static Builder systemError(final Throwable cause) { return new Builder(DrillPBError.ErrorType.SYSTEM, cause); } diff --git a/exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/ScanBatch.java b/exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/ScanBatch.java index 5a9af3953..42180699a 100644 --- a/exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/ScanBatch.java +++ b/exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/ScanBatch.java @@ -86,31 +86,32 @@ public class ScanBatch implements CloseableRecordBatch { public ScanBatch(PhysicalOperator subScanConfig, FragmentContext context, OperatorContext oContext, Iterator<RecordReader> readers, - List<Map<String, String>> implicitColumns) throws ExecutionSetupException { + List<Map<String, String>> implicitColumns) { this.context = context; this.readers = readers; if (!readers.hasNext()) { - throw new ExecutionSetupException("A scan batch must contain at least one reader."); + throw UserException.systemError( + new ExecutionSetupException("A scan batch must contain at least one reader.")) + .build(logger); } currentReader = readers.next(); this.oContext = oContext; allocator = oContext.getAllocator(); mutator = new Mutator(oContext, allocator, container); - boolean setup = false; try { oContext.getStats().startProcessing(); currentReader.setup(oContext, mutator); - setup = true; - } finally { - // if we had an exception during setup, make sure to release existing data. - if (!setup) { - try { - currentReader.close(); - } catch(final Exception e) { - throw new ExecutionSetupException(e); - } + } catch (ExecutionSetupException e) { + try { + currentReader.close(); + } catch(final Exception e2) { + logger.error("Close failed for reader " + currentReader.getClass().getSimpleName(), e2); } + throw UserException.systemError(e) + .addContext("Setup failed for", currentReader.getClass().getSimpleName()) + .build(logger); + } finally { oContext.getStats().stopProcessing(); } this.implicitColumns = implicitColumns.iterator(); @@ -173,9 +174,8 @@ public class ScanBatch implements CloseableRecordBatch { currentReader.allocate(mutator.fieldVectorMap()); } catch (OutOfMemoryException e) { - logger.debug("Caught Out of Memory Exception", e); clearFieldVectorMap(); - return IterOutcome.OUT_OF_MEMORY; + throw UserException.memoryError(e).build(logger); } while ((recordCount = currentReader.next()) == 0) { try { @@ -213,17 +213,16 @@ public class ScanBatch implements CloseableRecordBatch { try { currentReader.allocate(mutator.fieldVectorMap()); } catch (OutOfMemoryException e) { - logger.debug("Caught OutOfMemoryException"); clearFieldVectorMap(); - return IterOutcome.OUT_OF_MEMORY; + throw UserException.memoryError(e).build(logger); } addImplicitVectors(); } catch (ExecutionSetupException e) { - this.context.fail(e); releaseAssets(); - return IterOutcome.STOP; + throw UserException.systemError(e).build(logger); } } + // At this point, the current reader has read 1 or more rows. hasReadNonEmptyFile = true; @@ -245,18 +244,15 @@ public class ScanBatch implements CloseableRecordBatch { return IterOutcome.OK; } } catch (OutOfMemoryException ex) { - context.fail(UserException.memoryError(ex).build(logger)); - return IterOutcome.STOP; + throw UserException.memoryError(ex).build(logger); } catch (Exception ex) { - logger.debug("Failed to read the batch. Stopping...", ex); - context.fail(ex); - return IterOutcome.STOP; + throw UserException.systemError(ex).build(logger); } finally { oContext.getStats().stopProcessing(); } } - private void addImplicitVectors() throws ExecutionSetupException { + private void addImplicitVectors() { try { if (implicitVectors != null) { for (ValueVector v : implicitVectors.values()) { @@ -274,7 +270,10 @@ public class ScanBatch implements CloseableRecordBatch { } } } catch(SchemaChangeException e) { - throw new ExecutionSetupException(e); + // No exception should be thrown here. + throw UserException.systemError(e) + .addContext("Failure while allocating implicit vectors") + .build(logger); } } @@ -324,7 +323,7 @@ public class ScanBatch implements CloseableRecordBatch { * this scan batch. Made visible so that tests can create this mutator * without also needing a ScanBatch instance. (This class is really independent * of the ScanBatch, but resides here for historical reasons. This is, - * in turn, the only use of the genereated vector readers in the vector + * in turn, the only use of the generated vector readers in the vector * package.) */ diff --git a/protocol/src/main/java/org/apache/drill/common/types/TypeProtos.java b/protocol/src/main/java/org/apache/drill/common/types/TypeProtos.java index ff5698a91..1fa4848de 100644 --- a/protocol/src/main/java/org/apache/drill/common/types/TypeProtos.java +++ b/protocol/src/main/java/org/apache/drill/common/types/TypeProtos.java @@ -170,7 +170,7 @@ public final class TypeProtos { * <code>FLOAT4 = 18;</code> * * <pre> - * 4 byte ieee 754 + * 4 byte ieee 754 * </pre> */ FLOAT4(17, 18), @@ -463,7 +463,7 @@ public final class TypeProtos { * <code>FLOAT4 = 18;</code> * * <pre> - * 4 byte ieee 754 + * 4 byte ieee 754 * </pre> */ public static final int FLOAT4_VALUE = 18; diff --git a/protocol/src/main/java/org/apache/drill/exec/proto/UserBitShared.java b/protocol/src/main/java/org/apache/drill/exec/proto/UserBitShared.java index d28a13d6d..e4261df95 100644 --- a/protocol/src/main/java/org/apache/drill/exec/proto/UserBitShared.java +++ b/protocol/src/main/java/org/apache/drill/exec/proto/UserBitShared.java @@ -2178,6 +2178,10 @@ public final class UserBitShared { * * <pre> * equivalent to SQLNonTransientException. + * - unexpected internal state + * - uncategorized operation + * general user action is to contact the Drill team for + * assistance * </pre> */ SYSTEM(8, 8), @@ -2186,8 +2190,8 @@ public final class UserBitShared { * * <pre> * equivalent to SQLFeatureNotSupportedException - * - type change - * - schema change + * - unimplemented feature, option, or execution path + * - schema change in operator that does not support it * </pre> */ UNSUPPORTED_OPERATION(9, 9), @@ -2286,6 +2290,10 @@ public final class UserBitShared { * * <pre> * equivalent to SQLNonTransientException. + * - unexpected internal state + * - uncategorized operation + * general user action is to contact the Drill team for + * assistance * </pre> */ public static final int SYSTEM_VALUE = 8; @@ -2294,8 +2302,8 @@ public final class UserBitShared { * * <pre> * equivalent to SQLFeatureNotSupportedException - * - type change - * - schema change + * - unimplemented feature, option, or execution path + * - schema change in operator that does not support it * </pre> */ public static final int UNSUPPORTED_OPERATION_VALUE = 9; diff --git a/protocol/src/main/protobuf/Types.proto b/protocol/src/main/protobuf/Types.proto index 71fa4acd9..b2b29f085 100644 --- a/protocol/src/main/protobuf/Types.proto +++ b/protocol/src/main/protobuf/Types.proto @@ -24,7 +24,7 @@ option optimize_for = SPEED; enum MinorType { LATE = 0; // late binding type MAP = 1; // an empty map column. Useful for conceptual setup. Children listed within here - + TINYINT = 3; // single byte signed integer SMALLINT = 4; // two byte signed integer INT = 5; // four byte signed integer @@ -40,7 +40,7 @@ enum MinorType { TIMESTAMPTZ = 15; // unix epoch time in millis TIMESTAMP = 16; // TBD INTERVAL = 17; // TBD - FLOAT4 = 18; // 4 byte ieee 754 + FLOAT4 = 18; // 4 byte ieee 754 FLOAT8 = 19; // 8 byte ieee 754 BIT = 20; // single bit value (boolean) FIXEDCHAR = 21; // utf8 fixed length string, padded with spaces @@ -77,11 +77,8 @@ message MajorType { repeated MinorType sub_type = 7; // used by Union type } - - enum DataMode { OPTIONAL = 0; // nullable REQUIRED = 1; // non-nullable REPEATED = 2; // single, repeated-field } - diff --git a/protocol/src/main/protobuf/UserBitShared.proto b/protocol/src/main/protobuf/UserBitShared.proto index b09171146..65f9698c1 100644 --- a/protocol/src/main/protobuf/UserBitShared.proto +++ b/protocol/src/main/protobuf/UserBitShared.proto @@ -74,11 +74,15 @@ message DrillPBError{ */ RESOURCE = 7; /* equivalent to SQLNonTransientException. + * - unexpected internal state + * - uncategorized operation + * general user action is to contact the Drill team for + * assistance */ SYSTEM = 8; /* equivalent to SQLFeatureNotSupportedException - * - type change - * - schema change + * - unimplemented feature, option, or execution path + * - schema change in operator that does not support it */ UNSUPPORTED_OPERATION = 9; /* SQL validation exception |