cgivre commented on a change in pull request #2076:
URL: https://github.com/apache/drill/pull/2076#discussion_r421900398
##########
File path:
exec/vector/src/main/java/org/apache/drill/exec/vector/DateUtilities.java
##########
@@ -195,7 +197,27 @@ public static int timeToMillis(int hours, int minutes, int
seconds, int millis)
* @param localTime Java local time
* @return Drill form of the time
Review comment:
@paul-rogers
Would you mind please adding a brief javadoc to this class explaining what a
Drill form of the time is?
##########
File path:
exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/scan/convert/StandardConversions.java
##########
@@ -166,6 +171,12 @@ public static DirectConverter newInstance(
try {
final Constructor<? extends DirectConverter> ctor =
conversionClass.getDeclaredConstructor(ScalarWriter.class, Map.class);
return ctor.newInstance(baseWriter, properties);
+ } catch (final InvocationTargetException e) {
+ throw UserException.validationError(e.getTargetException())
+ .addContext("Converter setup failed")
+ .addContext("Conversion class" + conversionClass.getName())
+ // .addContext(errorContext) // Add after merge
Review comment:
Should we add this line back in?
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
