DRILL-6049: Misc. hygiene and code cleanup changes

close #1085

common/src/main/java/org/apache/drill/common/exceptions/UserException.java

@@ -83,21 +83,15 @@ public static Builder memoryError() {
    * <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
    *
    * @param cause exception we want the user exception to wrap. If cause is, or wrap, a user exception it will be
    *              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()
    */
 
   public static Builder systemError(final Throwable cause) {
@@ -364,6 +358,47 @@ public static Builder unsupportedError(final Throwable cause) {
     return new Builder(DrillPBError.ErrorType.UNSUPPORTED_OPERATION, cause);
   }
 
+  /**
+   * Wraps an error that arises from execution due to issues in the query, in
+   * the environment and so on -- anything other than "this should never occur"
+   * type checks.
+   * @param cause exception we want the user exception to wrap. If cause is, or wrap, a user exception it will be
+   *              returned by the builder instead of creating a new user exception
+   * @return user exception builder
+   */
+
+  public static Builder executionError(final Throwable cause) {
+    return new Builder(DrillPBError.ErrorType.EXECUTION_ERROR, cause);
+  }
+
+  /**
+   * Indicates an internal validation failed or similar unexpected error. Indicates
+   * the problem is likely within Drill itself rather than due to the environment,
+   * query, etc.
+   * @param cause exception we want the user exception to wrap. If cause is, or wrap, a user exception it will be
+   *              returned by the builder instead of creating a new user exception
+   * @return user exception builder
+   */
+
+  public static Builder internalError(final Throwable cause) {
+    return new Builder(DrillPBError.ErrorType.INTERNAL_ERROR, cause);
+  }
+
+  /**
+   * Indicates an unspecified error: code caught the exception, but does not have
+   * visibility into the cause well enough to pick one of the more specific
+   * error types. In practice, using this exception indicates that error handling
+   * should be moved closer to the source of the exception so we can provide the
+   * user with a better explanation than "something went wrong."
+   * @param cause exception we want the user exception to wrap. If cause is, or wrap, a user exception it will be
+   *              returned by the builder instead of creating a new user exception
+   * @return user exception builder
+   */
+  public static Builder unspecifiedError(final Throwable cause) {
+    return new Builder(DrillPBError.ErrorType.UNSPECIFIED_ERROR, cause);
+  }
+
+
   /**
    * Builder class for DrillUserException. You can wrap an existing exception, in this case it will first check if
    * this exception is, or wraps, a DrillUserException. If it does then the builder will use the user exception as it is
@@ -402,6 +437,14 @@ private Builder(final DrillPBError.ErrorType errorType, final Throwable cause) {
       }
     }
 
+    private Builder(UserException uex) {
+      this.uex = uex;
+      cause = uex.getCause();
+      errorType = uex.errorType;
+      context = uex.context;
+      message = uex.getOriginalMessage();
+    }
+
     /**
      * sets or replaces the error message.
      * <p>This will be ignored if this builder is wrapping a user exception
@@ -415,7 +458,11 @@ private Builder(final DrillPBError.ErrorType errorType, final Throwable cause) {
     public Builder message(final String format, final Object... args) {
       // we can't replace the message of a user exception
       if (uex == null && format != null) {
-        this.message = String.format(format, args);
+        if (args.length == 0) {
+          message = format;
+        } else {
+          message = String.format(format, args);
+        }
       }
       return this;
     }
@@ -636,6 +683,10 @@ private UserException(final Builder builder) {
     this.context = builder.context;
   }
 
+  public Builder rebuild() {
+    return new Builder(this);
+  }
+
   /**
    * generates the message that will be displayed to the client without the stack trace.
    *

common/src/main/java/org/apache/drill/common/types/Types.java

@@ -20,6 +20,9 @@
 import static org.apache.drill.common.types.TypeProtos.DataMode.REPEATED;
 
 import java.sql.ResultSetMetaData;
+import java.util.ArrayList;
+import java.util.Collections;
+import java.util.List;
 
 import org.apache.drill.common.exceptions.DrillRuntimeException;
 import org.apache.drill.common.types.TypeProtos.DataMode;
@@ -54,9 +57,9 @@ public static boolean isComplex(final MajorType type) {
     case LIST:
     case MAP:
       return true;
+    default:
+      return false;
     }
-
-    return false;
   }
 
   public static boolean isRepeated(final MajorType type) {
@@ -460,17 +463,19 @@ public static Comparability getComparability(final MajorType type) {
 
   public static boolean softEquals(final MajorType a, final MajorType b, final boolean allowNullSwap) {
     if (a.getMinorType() != b.getMinorType()) {
-        return false;
+      return false;
     }
-    if(allowNullSwap) {
+    if (allowNullSwap) {
       switch (a.getMode()) {
       case OPTIONAL:
       case REQUIRED:
         switch (b.getMode()) {
         case OPTIONAL:
         case REQUIRED:
           return true;
+        default:
         }
+      default:
       }
     }
     return a.getMode() == b.getMode();
@@ -728,4 +733,59 @@ public static boolean isLaterType(MajorType type) {
     return type.getMinorType() == MinorType.LATE;
   }
 
+  public static boolean isEquivalent(MajorType type1, MajorType type2) {
+
+    // Requires full type equality, including fields such as precision and scale.
+    // But, unset fields are equivalent to 0. Can't use the protobuf-provided
+    // isEquals() which treats set and unset fields as different.
+
+    if (type1.getMinorType() != type2.getMinorType() ||
+        type1.getMode() != type2.getMode() ||
+        type1.getScale() != type2.getScale() ||
+        type1.getPrecision() != type2.getPrecision()) {
+      return false;
+    }
+
+    // Subtypes are only for unions and are seldom used.
+
+    if (type1.getMinorType() != MinorType.UNION) {
+      return true;
+    }
+
+    List<MinorType> subtypes1 = type1.getSubTypeList();
+    List<MinorType> subtypes2 = type2.getSubTypeList();
+    if (subtypes1 == subtypes2) { // Only occurs if both are null
+      return true;
+    }
+    if (subtypes1 == null || subtypes2 == null) {
+      return false;
+    }
+    if (subtypes1.size() != subtypes2.size()) {
+      return false;
+    }
+
+    // Now it gets slow because subtype lists are not ordered.
+
+    List<MinorType> copy1 = new ArrayList<>();
+    List<MinorType> copy2 = new ArrayList<>();
+    copy1.addAll(subtypes1);
+    copy2.addAll(subtypes2);
+    Collections.sort(copy1);
+    Collections.sort(copy2);
+    return copy1.equals(copy2);
+  }
+
+  /**
+   * The union vector is a map of types. The following method provides
+   * the standard name to use in the type map. It replaces the many
+   * ad-hoc appearances of this code in each reference to the map.
+   *
+   * @param type Drill data type
+   * @return string key to use for this type in a union vector type
+   * map
+   */
+
+  public static String typeKey(MinorType type) {
+    return type.name().toLowerCase();
+  }
 }

common/src/test/java/org/apache/drill/test/DrillTest.java

@@ -29,6 +29,7 @@
 import org.junit.Before;
 import org.junit.BeforeClass;
 import org.junit.Rule;
+import org.junit.rules.DisableOnDebug;
 import org.junit.rules.ExpectedException;
 import org.junit.rules.TestName;
 import org.junit.rules.TestRule;
@@ -40,6 +41,7 @@
 import com.fasterxml.jackson.databind.ObjectMapper;
 
 public class DrillTest {
+
   protected static final ObjectMapper objectMapper;
   static {
     System.setProperty("line.separator", "\n");
@@ -54,8 +56,7 @@ public class DrillTest {
   static MemWatcher memWatcher;
   static String className;
 
-  @Rule public final TestRule TIMEOUT = TestTools.getTimeoutRule(100_000);
-
+  @Rule public final TestRule TIMEOUT = new DisableOnDebug(TestTools.getTimeoutRule(100_000));
   @Rule public final TestLogReporter logOutcome = LOG_OUTCOME;
 
   @Rule public final TestRule REPEAT_RULE = TestTools.getRepeatRule(false);

contrib/format-maprdb/src/main/java/org/apache/drill/exec/store/mapr/db/json/MaprDBJsonRecordReader.java

@@ -129,13 +129,13 @@ public MaprDBJsonRecordReader(MapRDBSubScanSpec subScanSpec,
   protected Collection<SchemaPath> transformColumns(Collection<SchemaPath> columns) {
     Set<SchemaPath> transformed = Sets.newLinkedHashSet();
     if (disablePushdown) {
-      transformed.add(Utilities.STAR_COLUMN);
+      transformed.add(SchemaPath.STAR_COLUMN);
       includeId = true;
       return transformed;
     }
 
     if (isStarQuery()) {
-      transformed.add(Utilities.STAR_COLUMN);
+      transformed.add(SchemaPath.STAR_COLUMN);
       includeId = true;
       if (isSkipQuery()) {
     	// `SELECT COUNT(*)` query

contrib/storage-kafka/src/main/java/org/apache/drill/exec/store/kafka/KafkaRecordReader.java

@@ -85,7 +85,7 @@ protected Collection<SchemaPath> transformColumns(Collection<SchemaPath> project
         transformed.add(column);
       }
     } else {
-      transformed.add(Utilities.STAR_COLUMN);
+      transformed.add(SchemaPath.STAR_COLUMN);
     }
     return transformed;
   }

contrib/storage-mongo/src/main/java/org/apache/drill/exec/store/mongo/MongoRecordReader.java

@@ -35,7 +35,6 @@
 import org.apache.drill.exec.physical.impl.OutputMutator;
 import org.apache.drill.exec.store.AbstractRecordReader;
 import org.apache.drill.exec.store.bson.BsonRecordReader;
-import org.apache.drill.exec.util.Utilities;
 import org.apache.drill.exec.vector.BaseValueVector;
 import org.apache.drill.exec.vector.complex.fn.JsonReader;
 import org.apache.drill.exec.vector.complex.impl.VectorContainerWriter;
@@ -113,7 +112,7 @@ protected Collection<SchemaPath> transformColumns(Collection<SchemaPath> project
     } else {
       // Tale all the fields including the _id
       this.fields.remove(DrillMongoConstants.ID);
-      transformed.add(Utilities.STAR_COLUMN);
+      transformed.add(SchemaPath.STAR_COLUMN);
     }
     return transformed;
   }

exec/java-exec/src/main/codegen/templates/CastDateDate.java

@@ -39,7 +39,6 @@
 import org.joda.time.MutableDateTime;
 import org.joda.time.DateTimeZone;
 import org.joda.time.DateMidnight;
-import org.apache.drill.exec.expr.fn.impl.DateUtility;
 
 /*
  * This class is generated using freemarker and the ${.template_name} template.

exec/java-exec/src/main/codegen/templates/CastIntervalInterval.java

@@ -41,7 +41,6 @@
 import org.joda.time.MutableDateTime;
 import org.joda.time.DateTimeZone;
 import org.joda.time.DateMidnight;
-import org.apache.drill.exec.expr.fn.impl.DateUtility;
 
 /*
  * This class is generated using freemarker and the ${.template_name} template.
@@ -83,7 +82,6 @@ public void eval() {
 import org.joda.time.MutableDateTime;
 import org.joda.time.DateTimeZone;
 import org.joda.time.DateMidnight;
-import org.apache.drill.exec.expr.fn.impl.DateUtility;
 
 @SuppressWarnings("unused")
 @FunctionTemplate(name = "cast${type.to?upper_case}", scope = FunctionTemplate.FunctionScope.SIMPLE, nulls=NullHandling.NULL_IF_NULL)

exec/java-exec/src/main/codegen/templates/CastIntervalVarChar.java

@@ -44,7 +44,6 @@
 import org.joda.time.MutableDateTime;
 import org.joda.time.DateTimeZone;
 import org.joda.time.DateMidnight;
-import org.apache.drill.exec.expr.fn.impl.DateUtility;
 
 /*
  * This class is generated using freemarker and the ${.template_name} template.
@@ -67,19 +66,19 @@ public void setup() {
 
   public void eval() {
 
-      int years  = (in.months / org.apache.drill.exec.expr.fn.impl.DateUtility.yearsToMonths);
-      int months = (in.months % org.apache.drill.exec.expr.fn.impl.DateUtility.yearsToMonths);
+      int years  = (in.months / org.apache.drill.exec.vector.DateUtilities.yearsToMonths);
+      int months = (in.months % org.apache.drill.exec.vector.DateUtilities.yearsToMonths);
 
       long millis = in.milliseconds;
 
-      long hours  = millis / (org.apache.drill.exec.expr.fn.impl.DateUtility.hoursToMillis);
-      millis     = millis % (org.apache.drill.exec.expr.fn.impl.DateUtility.hoursToMillis);
+      long hours  = millis / (org.apache.drill.exec.vector.DateUtilities.hoursToMillis);
+      millis     = millis % (org.apache.drill.exec.vector.DateUtilities.hoursToMillis);
 
-      long minutes = millis / (org.apache.drill.exec.expr.fn.impl.DateUtility.minutesToMillis);
-      millis      = millis % (org.apache.drill.exec.expr.fn.impl.DateUtility.minutesToMillis);
+      long minutes = millis / (org.apache.drill.exec.vector.DateUtilities.minutesToMillis);
+      millis      = millis % (org.apache.drill.exec.vector.DateUtilities.minutesToMillis);
 
-      long seconds = millis / (org.apache.drill.exec.expr.fn.impl.DateUtility.secondsToMillis);
-      millis      = millis % (org.apache.drill.exec.expr.fn.impl.DateUtility.secondsToMillis);
+      long seconds = millis / (org.apache.drill.exec.vector.DateUtilities.secondsToMillis);
+      millis      = millis % (org.apache.drill.exec.vector.DateUtilities.secondsToMillis);
 
       String yearString = (Math.abs(years) == 1) ? " year " : " years ";
       String monthString = (Math.abs(months) == 1) ? " month " : " months ";
@@ -124,7 +123,6 @@ public void eval() {
 import org.joda.time.MutableDateTime;
 import org.joda.time.DateTimeZone;
 import org.joda.time.DateMidnight;
-import org.apache.drill.exec.expr.fn.impl.DateUtility;
 
 @SuppressWarnings("unused")
 @FunctionTemplate(name = "cast${type.to?upper_case}",
@@ -143,8 +141,8 @@ public void setup() {
   }
 
   public void eval() {
-      int years  = (in.value / org.apache.drill.exec.expr.fn.impl.DateUtility.yearsToMonths);
-      int months = (in.value % org.apache.drill.exec.expr.fn.impl.DateUtility.yearsToMonths);
+      int years  = (in.value / org.apache.drill.exec.vector.DateUtilities.yearsToMonths);
+      int months = (in.value % org.apache.drill.exec.vector.DateUtilities.yearsToMonths);
 
       String yearString = (Math.abs(years) == 1) ? " year " : " years ";
       String monthString = (Math.abs(months) == 1) ? " month " : " months ";
@@ -184,7 +182,6 @@ public void eval() {
 import org.joda.time.MutableDateTime;
 import org.joda.time.DateTimeZone;
 import org.joda.time.DateMidnight;
-import org.apache.drill.exec.expr.fn.impl.DateUtility;
 import javax.inject.Inject;
 import io.netty.buffer.DrillBuf;
 
@@ -208,14 +205,14 @@ public void setup() {
   public void eval() {
       long millis = in.milliseconds;
 
-      long hours  = millis / (org.apache.drill.exec.expr.fn.impl.DateUtility.hoursToMillis);
-      millis     = millis % (org.apache.drill.exec.expr.fn.impl.DateUtility.hoursToMillis);
+      long hours  = millis / (org.apache.drill.exec.vector.DateUtilities.hoursToMillis);
+      millis     = millis % (org.apache.drill.exec.vector.DateUtilities.hoursToMillis);
 
-      long minutes = millis / (org.apache.drill.exec.expr.fn.impl.DateUtility.minutesToMillis);
-      millis      = millis % (org.apache.drill.exec.expr.fn.impl.DateUtility.minutesToMillis);
+      long minutes = millis / (org.apache.drill.exec.vector.DateUtilities.minutesToMillis);
+      millis      = millis % (org.apache.drill.exec.vector.DateUtilities.minutesToMillis);
 
-      long seconds = millis / (org.apache.drill.exec.expr.fn.impl.DateUtility.secondsToMillis);
-      millis      = millis % (org.apache.drill.exec.expr.fn.impl.DateUtility.secondsToMillis);
+      long seconds = millis / (org.apache.drill.exec.vector.DateUtilities.secondsToMillis);
+      millis      = millis % (org.apache.drill.exec.vector.DateUtilities.secondsToMillis);
 
       String dayString = (Math.abs(in.days) == 1) ? " day " : " days ";