-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/33779/#review82390
-----------------------------------------------------------


Suggestion: can you make the change to distribution/src/resources/sqlline 
(mentioned 
[here](https://github.com/sudheeshkatkam/drill/commit/7d31db0c3958065a2b61b61d0b0fe45cf56ed83a#diff-038a53b6ec34c26bde1682faf16d79ba))
 to add color to error messages? There were cases in the past where error 
messages were printed directly to the console, from places other than 
DrillResultSet. These errors were not reported to the client. In such cases, 
usually the query hangs and it had to be cancelled. The message was shown to 
the user in embedded mode. But in distributed mode, nothing is reported to the 
user. This is a simple visual help to find those bugs at least in embedded 
mode. It also looks pretty.

- Sudheesh Katkam


On May 2, 2015, 6:12 a.m., Daniel Barclay wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/33779/
> -----------------------------------------------------------
> 
> (Updated May 2, 2015, 6:12 a.m.)
> 
> 
> Review request for drill, abdelhakim deneche, Mehant Baid, and Parth Chandra.
> 
> 
> Bugs: DRILL-2932
>     https://issues.apache.org/jira/browse/DRILL-2932
> 
> 
> Repository: drill-git
> 
> 
> Description
> -------
> 
> Main:
> - Removed the System.out.println(...) from submissionFailed(...).
> - Created unit test for execute...(...) exceptions.
> - Put UserException's message text in SQLException's message (didn't just 
> wrap).
> - Added undoing of extra wrapping by AvaticaStatement.execute...(...).
> - Refined related exception handling (handled cases separately, narrowed 
> throws declarations and catches).
>     
> JDBC cleanup:
> - Renamed ex -> executionFailureException
> - Added getNext() method doc. comment.
> - Removed some obsolete alignment spaces.
>     
> Added DRILL-2933 (SchemaChangeException) "review this" TODOs.
>   - DrillCursor;
>   - MergingRecordBatch, ParquetResultListener, PrintingResultsListener, 
> QueryWrapper, RecordBatchLoader, UnorderedReceiverBatch;
>   - TestDrillbitResilience, TestTextColumn, BaseTestQuery, DrillTestWrapper;
> 
> 
> Diffs
> -----
> 
>   
> exec/java-exec/src/main/java/org/apache/drill/exec/client/PrintingResultsListener.java
>  64e7266 
>   
> exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/mergereceiver/MergingRecordBatch.java
>  40cbc89 
>   
> exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/unorderedreceiver/UnorderedReceiverBatch.java
>  094865e 
>   
> exec/java-exec/src/main/java/org/apache/drill/exec/record/RecordBatchLoader.java
>  088630c 
>   
> exec/java-exec/src/main/java/org/apache/drill/exec/rpc/user/QueryResultHandler.java
>  3beae89 
>   
> exec/java-exec/src/main/java/org/apache/drill/exec/server/rest/QueryWrapper.java
>  aa43aa9 
>   exec/java-exec/src/test/java/org/apache/drill/BaseTestQuery.java b02051b 
>   exec/java-exec/src/test/java/org/apache/drill/DrillTestWrapper.java d05c896 
>   
> exec/java-exec/src/test/java/org/apache/drill/exec/server/TestDrillbitResilience.java
>  2698701 
>   
> exec/java-exec/src/test/java/org/apache/drill/exec/store/parquet/ParquetResultListener.java
>  0e80f91 
>   
> exec/java-exec/src/test/java/org/apache/drill/exec/store/text/TestTextColumn.java
>  3c1a38a 
>   exec/jdbc/src/main/java/org/apache/drill/jdbc/DrillCursor.java 41b89ce 
>   exec/jdbc/src/main/java/org/apache/drill/jdbc/DrillStatement.java ba265e6 
>   exec/jdbc/src/main/java/org/apache/drill/jdbc/impl/DrillResultSetImpl.java 
> 484a5e5 
>   
> exec/jdbc/src/test/java/org/apache/drill/jdbc/test/TestExecutionExceptionsToClient.java
>  PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/33779/diff/
> 
> 
> Testing
> -------
> 
> Ran SQLLine to confirm change from System.out plus poor SQLException to good 
> SQLException.
> 
> Ran new specific unit test.
> 
> Ran regular tests; no new failures.
> 
> 
> Thanks,
> 
> Daniel Barclay
> 
>

Reply via email to