drill-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Daniel Barclay" <dbarc...@maprtech.com>
Subject Re: Review Request 33779: DRILL-2932: Fix: Error text reported via System.out.println rather than thrown SQLException's message
Date Tue, 05 May 2015 03:08:51 GMT


> On May 4, 2015, 4:49 p.m., Sudheesh Katkam wrote:
> > 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.
> 
> abdelhakim deneche wrote:
>     +1

Let's do that possible improvement separately from this bug fix.

(Also, if we want to find errors where code writes output directly to stdout/stderr, it would
be more effective to search/filter for them directly rather than waiting for occasional occurrences
of them.)


- Daniel


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


On May 5, 2015, 3:08 a.m., Daniel Barclay wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/33779/
> -----------------------------------------------------------
> 
> (Updated May 5, 2015, 3:08 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
> -------
> 
> DRILL-2932: Fix:  Error reported via System.out rather than exception message
>     
> Main:
> - Removed the System.out.println(...) from submissionFailed(...).
> - Put UserException's message text in SQLException's message (didn't just wrap).
> - Added undoing of extra wrapping by AvaticaStatement.execute...(...).
> - Created unit test for execute...(...) exceptions.
> - 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 "review this" TODOs re DRILL-2933 (probably-obsolete SchemaChangeException):
> - 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
c36b0d3 
>   exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/unorderedreceiver/UnorderedReceiverBatch.java
09cb7ad 
>   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 d6e6d08 
>   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
> 
>


Mime
  • Unnamed multipart/alternative (inline, None, 0 bytes)
View raw message