paul-rogers commented on a change in pull request #2076:
URL: https://github.com/apache/drill/pull/2076#discussion_r429685226
##########
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:
Turns out the format is explained two lines above, but repeated it in
the `@return` as requested.
##########
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:
As the comment (tersely) suggests, the error context can be added after
the merge of a different PR which provides the error context. This comment will
show up in a merge conflict and remind me to make that change.
----------------------------------------------------------------
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:
[email protected]
