You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@drill.apache.org by Parth Chandra <pc...@maprtech.com> on 2015/04/17 08:14:02 UTC
Review Request 33289: DRILL-2350: Improve exception handling and error
messages in JSON reader.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/33289/
-----------------------------------------------------------
Review request for drill, abdelhakim deneche and Jason Altekruse.
Repository: drill-git
Description
-------
DRILL-2350: Improve exception handling and error messages in JSON reader.
Errors reported from the JSON reader will now look like the following:
Query failed: UNSUPPORTED_OPERATION ERROR: In a list of type FLOAT8, encountered a value of type BIGINT. Drill does not support lists of different types.
File: /Users/pchandra/work/data/test/DRILL-2350.json
Record: 1
Line: 2
Column: 23
Field: loc
[c4b33cca-0a52-4126-a300-2ab91d0ed9d1 on localhost:31010]
Diffs
-----
exec/java-exec/src/main/codegen/includes/vv_imports.ftl 371d8d0
exec/java-exec/src/main/codegen/templates/ListWriters.java 29708d7
exec/java-exec/src/main/java/org/apache/drill/exec/store/easy/json/JSONRecordReader.java b41de31
exec/java-exec/src/main/java/org/apache/drill/exec/store/easy/json/JsonProcessor.java b310818
exec/java-exec/src/main/java/org/apache/drill/exec/store/easy/json/reader/BaseJsonProcessor.java 718bb09
exec/java-exec/src/main/java/org/apache/drill/exec/vector/complex/fn/JsonReader.java c196fd2
Diff: https://reviews.apache.org/r/33289/diff/
Testing
-------
All unit tests.
Thanks,
Parth Chandra
Re: Review Request 33289: DRILL-2350: Improve exception handling and
error messages in JSON reader.
Posted by Jason Altekruse <al...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/33289/#review80558
-----------------------------------------------------------
Ship it!
Ship It!
- Jason Altekruse
On April 17, 2015, 6:14 a.m., Parth Chandra wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/33289/
> -----------------------------------------------------------
>
> (Updated April 17, 2015, 6:14 a.m.)
>
>
> Review request for drill, abdelhakim deneche and Jason Altekruse.
>
>
> Repository: drill-git
>
>
> Description
> -------
>
> DRILL-2350: Improve exception handling and error messages in JSON reader.
>
> Errors reported from the JSON reader will now look like the following:
>
> Query failed: UNSUPPORTED_OPERATION ERROR: In a list of type FLOAT8, encountered a value of type BIGINT. Drill does not support lists of different types.
>
> File: /Users/pchandra/work/data/test/DRILL-2350.json
> Record: 1
> Line: 2
> Column: 23
> Field: loc
>
> [c4b33cca-0a52-4126-a300-2ab91d0ed9d1 on localhost:31010]
>
>
> Diffs
> -----
>
> exec/java-exec/src/main/codegen/includes/vv_imports.ftl 371d8d0
> exec/java-exec/src/main/codegen/templates/ListWriters.java 29708d7
> exec/java-exec/src/main/java/org/apache/drill/exec/store/easy/json/JSONRecordReader.java b41de31
> exec/java-exec/src/main/java/org/apache/drill/exec/store/easy/json/JsonProcessor.java b310818
> exec/java-exec/src/main/java/org/apache/drill/exec/store/easy/json/reader/BaseJsonProcessor.java 718bb09
> exec/java-exec/src/main/java/org/apache/drill/exec/vector/complex/fn/JsonReader.java c196fd2
>
> Diff: https://reviews.apache.org/r/33289/diff/
>
>
> Testing
> -------
>
> All unit tests.
>
>
> Thanks,
>
> Parth Chandra
>
>
Re: Review Request 33289: DRILL-2350: Improve exception handling and
error messages in JSON reader.
Posted by abdelhakim deneche <ad...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/33289/#review80477
-----------------------------------------------------------
LGTM
- abdelhakim deneche
On April 17, 2015, 6:14 a.m., Parth Chandra wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/33289/
> -----------------------------------------------------------
>
> (Updated April 17, 2015, 6:14 a.m.)
>
>
> Review request for drill, abdelhakim deneche and Jason Altekruse.
>
>
> Repository: drill-git
>
>
> Description
> -------
>
> DRILL-2350: Improve exception handling and error messages in JSON reader.
>
> Errors reported from the JSON reader will now look like the following:
>
> Query failed: UNSUPPORTED_OPERATION ERROR: In a list of type FLOAT8, encountered a value of type BIGINT. Drill does not support lists of different types.
>
> File: /Users/pchandra/work/data/test/DRILL-2350.json
> Record: 1
> Line: 2
> Column: 23
> Field: loc
>
> [c4b33cca-0a52-4126-a300-2ab91d0ed9d1 on localhost:31010]
>
>
> Diffs
> -----
>
> exec/java-exec/src/main/codegen/includes/vv_imports.ftl 371d8d0
> exec/java-exec/src/main/codegen/templates/ListWriters.java 29708d7
> exec/java-exec/src/main/java/org/apache/drill/exec/store/easy/json/JSONRecordReader.java b41de31
> exec/java-exec/src/main/java/org/apache/drill/exec/store/easy/json/JsonProcessor.java b310818
> exec/java-exec/src/main/java/org/apache/drill/exec/store/easy/json/reader/BaseJsonProcessor.java 718bb09
> exec/java-exec/src/main/java/org/apache/drill/exec/vector/complex/fn/JsonReader.java c196fd2
>
> Diff: https://reviews.apache.org/r/33289/diff/
>
>
> Testing
> -------
>
> All unit tests.
>
>
> Thanks,
>
> Parth Chandra
>
>
Re: Review Request 33289: DRILL-2350: Improve exception handling and
error messages in JSON reader.
Posted by abdelhakim deneche <ad...@gmail.com>.
> On April 20, 2015, 9:46 p.m., Jason Altekruse wrote:
> > exec/java-exec/src/test/java/org/apache/drill/exec/store/json/TestJsonRecordReader.java, line 81
> > <https://reviews.apache.org/r/33289/diff/2-3/?file=936938#file936938line81>
> >
> > I don't aggree with this suggestion, this test could 'pass' if any error thrown in the system comes back with as an unsupported operation exception type. I think we should be checking messages. If we want to avoid some mangement overhead, the tests and project live together in the same source. You can refer to a static final string from wherever in the source the error message is produced when you go to check it in the test.
True, but testing if the error message contains "UNSUPPORTED OPERATION ERROR" won't be enough either as all unsupported operation errors will contain this string.
- abdelhakim
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/33289/#review80840
-----------------------------------------------------------
On April 20, 2015, 9:31 p.m., Parth Chandra wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/33289/
> -----------------------------------------------------------
>
> (Updated April 20, 2015, 9:31 p.m.)
>
>
> Review request for drill, abdelhakim deneche and Jason Altekruse.
>
>
> Repository: drill-git
>
>
> Description
> -------
>
> DRILL-2350: Improve exception handling and error messages in JSON reader.
>
> Errors reported from the JSON reader will now look like the following:
>
> Query failed: UNSUPPORTED_OPERATION ERROR: In a list of type FLOAT8, encountered a value of type BIGINT. Drill does not support lists of different types.
>
> File: /Users/pchandra/work/data/test/DRILL-2350.json
> Record: 1
> Line: 2
> Column: 23
> Field: loc
>
> [c4b33cca-0a52-4126-a300-2ab91d0ed9d1 on localhost:31010]
>
>
> Diffs
> -----
>
> exec/java-exec/src/main/codegen/includes/vv_imports.ftl 371d8d0
> exec/java-exec/src/main/codegen/templates/ListWriters.java 29708d7
> exec/java-exec/src/main/java/org/apache/drill/exec/store/easy/json/JSONRecordReader.java b41de31
> exec/java-exec/src/main/java/org/apache/drill/exec/store/easy/json/JsonProcessor.java b310818
> exec/java-exec/src/main/java/org/apache/drill/exec/store/easy/json/reader/BaseJsonProcessor.java 718bb09
> exec/java-exec/src/main/java/org/apache/drill/exec/vector/complex/fn/JsonReader.java c196fd2
> exec/java-exec/src/test/java/org/apache/drill/exec/store/json/TestJsonRecordReader.java 8b09e80
> exec/java-exec/src/test/resources/jsoninput/DRILL-2350.json PRE-CREATION
>
> Diff: https://reviews.apache.org/r/33289/diff/
>
>
> Testing
> -------
>
> All unit tests.
>
>
> Thanks,
>
> Parth Chandra
>
>
Re: Review Request 33289: DRILL-2350: Improve exception handling and
error messages in JSON reader.
Posted by Jason Altekruse <al...@gmail.com>.
> On April 20, 2015, 9:46 p.m., Jason Altekruse wrote:
> > exec/java-exec/src/test/java/org/apache/drill/exec/store/json/TestJsonRecordReader.java, line 81
> > <https://reviews.apache.org/r/33289/diff/2-3/?file=936938#file936938line81>
> >
> > I don't aggree with this suggestion, this test could 'pass' if any error thrown in the system comes back with as an unsupported operation exception type. I think we should be checking messages. If we want to avoid some mangement overhead, the tests and project live together in the same source. You can refer to a static final string from wherever in the source the error message is produced when you go to check it in the test.
>
> abdelhakim deneche wrote:
> True, but testing if the error message contains "UNSUPPORTED OPERATION ERROR" won't be enough either as all unsupported operation errors will contain this string.
I should have looked more carefully, I didn't realize the only string he was checking for before was the portion that will be common to unsupported operation exceptions. What I should ahve said is that on top of a check for the type of exception we should at least make an effort to make sure that the correct path we intend to check is being executed. This could be as simple as checking for the string JSON in the error message for at least basic coverage in this case. Obviously it would be better to check for the whole message, but there is the burden then of dealing with cases where error messages are actually format strings with %s, %d, etc. and we would need to have the same String.format statement in the test to produce the full error message. We could possibly look at also implementing a little code to help on the test writing side, possibly looking for the format sequences in the test verification of the messages and giving the option to have them somewhat ignored by subsituting
them with regex expressions for the appropriate type (such as %d substitutued with [0-9]*.[0-9]*)
- Jason
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/33289/#review80840
-----------------------------------------------------------
On April 20, 2015, 9:31 p.m., Parth Chandra wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/33289/
> -----------------------------------------------------------
>
> (Updated April 20, 2015, 9:31 p.m.)
>
>
> Review request for drill, abdelhakim deneche and Jason Altekruse.
>
>
> Repository: drill-git
>
>
> Description
> -------
>
> DRILL-2350: Improve exception handling and error messages in JSON reader.
>
> Errors reported from the JSON reader will now look like the following:
>
> Query failed: UNSUPPORTED_OPERATION ERROR: In a list of type FLOAT8, encountered a value of type BIGINT. Drill does not support lists of different types.
>
> File: /Users/pchandra/work/data/test/DRILL-2350.json
> Record: 1
> Line: 2
> Column: 23
> Field: loc
>
> [c4b33cca-0a52-4126-a300-2ab91d0ed9d1 on localhost:31010]
>
>
> Diffs
> -----
>
> exec/java-exec/src/main/codegen/includes/vv_imports.ftl 371d8d0
> exec/java-exec/src/main/codegen/templates/ListWriters.java 29708d7
> exec/java-exec/src/main/java/org/apache/drill/exec/store/easy/json/JSONRecordReader.java b41de31
> exec/java-exec/src/main/java/org/apache/drill/exec/store/easy/json/JsonProcessor.java b310818
> exec/java-exec/src/main/java/org/apache/drill/exec/store/easy/json/reader/BaseJsonProcessor.java 718bb09
> exec/java-exec/src/main/java/org/apache/drill/exec/vector/complex/fn/JsonReader.java c196fd2
> exec/java-exec/src/test/java/org/apache/drill/exec/store/json/TestJsonRecordReader.java 8b09e80
> exec/java-exec/src/test/resources/jsoninput/DRILL-2350.json PRE-CREATION
>
> Diff: https://reviews.apache.org/r/33289/diff/
>
>
> Testing
> -------
>
> All unit tests.
>
>
> Thanks,
>
> Parth Chandra
>
>
Re: Review Request 33289: DRILL-2350: Improve exception handling and
error messages in JSON reader.
Posted by Jason Altekruse <al...@gmail.com>.
> On April 20, 2015, 9:46 p.m., Jason Altekruse wrote:
> > exec/java-exec/src/test/java/org/apache/drill/exec/store/json/TestJsonRecordReader.java, line 81
> > <https://reviews.apache.org/r/33289/diff/2-3/?file=936938#file936938line81>
> >
> > I don't aggree with this suggestion, this test could 'pass' if any error thrown in the system comes back with as an unsupported operation exception type. I think we should be checking messages. If we want to avoid some mangement overhead, the tests and project live together in the same source. You can refer to a static final string from wherever in the source the error message is produced when you go to check it in the test.
>
> abdelhakim deneche wrote:
> True, but testing if the error message contains "UNSUPPORTED OPERATION ERROR" won't be enough either as all unsupported operation errors will contain this string.
>
> Jason Altekruse wrote:
> I should have looked more carefully, I didn't realize the only string he was checking for before was the portion that will be common to unsupported operation exceptions. What I should ahve said is that on top of a check for the type of exception we should at least make an effort to make sure that the correct path we intend to check is being executed. This could be as simple as checking for the string JSON in the error message for at least basic coverage in this case. Obviously it would be better to check for the whole message, but there is the burden then of dealing with cases where error messages are actually format strings with %s, %d, etc. and we would need to have the same String.format statement in the test to produce the full error message. We could possibly look at also implementing a little code to help on the test writing side, possibly looking for the format sequences in the test verification of the messages and giving the option to have them somewhat ignored by subsi
tuting them with regex expressions for the appropriate type (such as %d substitutued with [0-9]*.[0-9]*)
I'm not the best with regex, but reviewboard pulled out part of my comment, I had a star after both numeric character classes and a backslash before the dot in the middle, this probably does not cover everything, but it is a little more valid that what reviewboard said I posted :p (I should have put a ? on the dot to match it for an integer number or a decimal one)
- Jason
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/33289/#review80840
-----------------------------------------------------------
On April 20, 2015, 9:31 p.m., Parth Chandra wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/33289/
> -----------------------------------------------------------
>
> (Updated April 20, 2015, 9:31 p.m.)
>
>
> Review request for drill, abdelhakim deneche and Jason Altekruse.
>
>
> Repository: drill-git
>
>
> Description
> -------
>
> DRILL-2350: Improve exception handling and error messages in JSON reader.
>
> Errors reported from the JSON reader will now look like the following:
>
> Query failed: UNSUPPORTED_OPERATION ERROR: In a list of type FLOAT8, encountered a value of type BIGINT. Drill does not support lists of different types.
>
> File: /Users/pchandra/work/data/test/DRILL-2350.json
> Record: 1
> Line: 2
> Column: 23
> Field: loc
>
> [c4b33cca-0a52-4126-a300-2ab91d0ed9d1 on localhost:31010]
>
>
> Diffs
> -----
>
> exec/java-exec/src/main/codegen/includes/vv_imports.ftl 371d8d0
> exec/java-exec/src/main/codegen/templates/ListWriters.java 29708d7
> exec/java-exec/src/main/java/org/apache/drill/exec/store/easy/json/JSONRecordReader.java b41de31
> exec/java-exec/src/main/java/org/apache/drill/exec/store/easy/json/JsonProcessor.java b310818
> exec/java-exec/src/main/java/org/apache/drill/exec/store/easy/json/reader/BaseJsonProcessor.java 718bb09
> exec/java-exec/src/main/java/org/apache/drill/exec/vector/complex/fn/JsonReader.java c196fd2
> exec/java-exec/src/test/java/org/apache/drill/exec/store/json/TestJsonRecordReader.java 8b09e80
> exec/java-exec/src/test/resources/jsoninput/DRILL-2350.json PRE-CREATION
>
> Diff: https://reviews.apache.org/r/33289/diff/
>
>
> Testing
> -------
>
> All unit tests.
>
>
> Thanks,
>
> Parth Chandra
>
>
Re: Review Request 33289: DRILL-2350: Improve exception handling and
error messages in JSON reader.
Posted by Jason Altekruse <al...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/33289/#review80840
-----------------------------------------------------------
exec/java-exec/src/test/java/org/apache/drill/exec/store/json/TestJsonRecordReader.java
<https://reviews.apache.org/r/33289/#comment130944>
I don't aggree with this suggestion, this test could 'pass' if any error thrown in the system comes back with as an unsupported operation exception type. I think we should be checking messages. If we want to avoid some mangement overhead, the tests and project live together in the same source. You can refer to a static final string from wherever in the source the error message is produced when you go to check it in the test.
- Jason Altekruse
On April 20, 2015, 9:31 p.m., Parth Chandra wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/33289/
> -----------------------------------------------------------
>
> (Updated April 20, 2015, 9:31 p.m.)
>
>
> Review request for drill, abdelhakim deneche and Jason Altekruse.
>
>
> Repository: drill-git
>
>
> Description
> -------
>
> DRILL-2350: Improve exception handling and error messages in JSON reader.
>
> Errors reported from the JSON reader will now look like the following:
>
> Query failed: UNSUPPORTED_OPERATION ERROR: In a list of type FLOAT8, encountered a value of type BIGINT. Drill does not support lists of different types.
>
> File: /Users/pchandra/work/data/test/DRILL-2350.json
> Record: 1
> Line: 2
> Column: 23
> Field: loc
>
> [c4b33cca-0a52-4126-a300-2ab91d0ed9d1 on localhost:31010]
>
>
> Diffs
> -----
>
> exec/java-exec/src/main/codegen/includes/vv_imports.ftl 371d8d0
> exec/java-exec/src/main/codegen/templates/ListWriters.java 29708d7
> exec/java-exec/src/main/java/org/apache/drill/exec/store/easy/json/JSONRecordReader.java b41de31
> exec/java-exec/src/main/java/org/apache/drill/exec/store/easy/json/JsonProcessor.java b310818
> exec/java-exec/src/main/java/org/apache/drill/exec/store/easy/json/reader/BaseJsonProcessor.java 718bb09
> exec/java-exec/src/main/java/org/apache/drill/exec/vector/complex/fn/JsonReader.java c196fd2
> exec/java-exec/src/test/java/org/apache/drill/exec/store/json/TestJsonRecordReader.java 8b09e80
> exec/java-exec/src/test/resources/jsoninput/DRILL-2350.json PRE-CREATION
>
> Diff: https://reviews.apache.org/r/33289/diff/
>
>
> Testing
> -------
>
> All unit tests.
>
>
> Thanks,
>
> Parth Chandra
>
>
Re: Review Request 33289: DRILL-2350: Improve exception handling and
error messages in JSON reader.
Posted by abdelhakim deneche <ad...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/33289/#review80836
-----------------------------------------------------------
Ship it!
Ship It!
- abdelhakim deneche
On April 20, 2015, 9:31 p.m., Parth Chandra wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/33289/
> -----------------------------------------------------------
>
> (Updated April 20, 2015, 9:31 p.m.)
>
>
> Review request for drill, abdelhakim deneche and Jason Altekruse.
>
>
> Repository: drill-git
>
>
> Description
> -------
>
> DRILL-2350: Improve exception handling and error messages in JSON reader.
>
> Errors reported from the JSON reader will now look like the following:
>
> Query failed: UNSUPPORTED_OPERATION ERROR: In a list of type FLOAT8, encountered a value of type BIGINT. Drill does not support lists of different types.
>
> File: /Users/pchandra/work/data/test/DRILL-2350.json
> Record: 1
> Line: 2
> Column: 23
> Field: loc
>
> [c4b33cca-0a52-4126-a300-2ab91d0ed9d1 on localhost:31010]
>
>
> Diffs
> -----
>
> exec/java-exec/src/main/codegen/includes/vv_imports.ftl 371d8d0
> exec/java-exec/src/main/codegen/templates/ListWriters.java 29708d7
> exec/java-exec/src/main/java/org/apache/drill/exec/store/easy/json/JSONRecordReader.java b41de31
> exec/java-exec/src/main/java/org/apache/drill/exec/store/easy/json/JsonProcessor.java b310818
> exec/java-exec/src/main/java/org/apache/drill/exec/store/easy/json/reader/BaseJsonProcessor.java 718bb09
> exec/java-exec/src/main/java/org/apache/drill/exec/vector/complex/fn/JsonReader.java c196fd2
> exec/java-exec/src/test/java/org/apache/drill/exec/store/json/TestJsonRecordReader.java 8b09e80
> exec/java-exec/src/test/resources/jsoninput/DRILL-2350.json PRE-CREATION
>
> Diff: https://reviews.apache.org/r/33289/diff/
>
>
> Testing
> -------
>
> All unit tests.
>
>
> Thanks,
>
> Parth Chandra
>
>
Re: Review Request 33289: DRILL-2350: Improve exception handling and
error messages in JSON reader.
Posted by Jason Altekruse <al...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/33289/#review80982
-----------------------------------------------------------
Looks like you accidentally uploaded the old patch, I don't see any changes after you moved from the check for "UNSUPPORTED_OPERATION" in the string, to when you were checking the exception type in the PBError object.
- Jason Altekruse
On April 21, 2015, 3:44 a.m., Parth Chandra wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/33289/
> -----------------------------------------------------------
>
> (Updated April 21, 2015, 3:44 a.m.)
>
>
> Review request for drill, abdelhakim deneche and Jason Altekruse.
>
>
> Repository: drill-git
>
>
> Description
> -------
>
> DRILL-2350: Improve exception handling and error messages in JSON reader.
>
> Errors reported from the JSON reader will now look like the following:
>
> Query failed: UNSUPPORTED_OPERATION ERROR: In a list of type FLOAT8, encountered a value of type BIGINT. Drill does not support lists of different types.
>
> File: /Users/pchandra/work/data/test/DRILL-2350.json
> Record: 1
> Line: 2
> Column: 23
> Field: loc
>
> [c4b33cca-0a52-4126-a300-2ab91d0ed9d1 on localhost:31010]
>
>
> Diffs
> -----
>
> exec/java-exec/src/main/codegen/includes/vv_imports.ftl 371d8d0
> exec/java-exec/src/main/codegen/templates/ListWriters.java 29708d7
> exec/java-exec/src/main/java/org/apache/drill/exec/store/easy/json/JSONRecordReader.java b41de31
> exec/java-exec/src/main/java/org/apache/drill/exec/store/easy/json/JsonProcessor.java b310818
> exec/java-exec/src/main/java/org/apache/drill/exec/store/easy/json/reader/BaseJsonProcessor.java 718bb09
> exec/java-exec/src/main/java/org/apache/drill/exec/vector/complex/fn/JsonReader.java c196fd2
> exec/java-exec/src/test/java/org/apache/drill/exec/store/json/TestJsonRecordReader.java 8b09e80
> exec/java-exec/src/test/resources/jsoninput/DRILL-2350.json PRE-CREATION
>
> Diff: https://reviews.apache.org/r/33289/diff/
>
>
> Testing
> -------
>
> All unit tests.
>
>
> Thanks,
>
> Parth Chandra
>
>
Re: Review Request 33289: DRILL-2350: Improve exception handling and
error messages in JSON reader.
Posted by Jason Altekruse <al...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/33289/#review81059
-----------------------------------------------------------
Ship it!
Ship It!
- Jason Altekruse
On April 21, 2015, 7:56 p.m., Parth Chandra wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/33289/
> -----------------------------------------------------------
>
> (Updated April 21, 2015, 7:56 p.m.)
>
>
> Review request for drill, abdelhakim deneche and Jason Altekruse.
>
>
> Repository: drill-git
>
>
> Description
> -------
>
> DRILL-2350: Improve exception handling and error messages in JSON reader.
>
> Errors reported from the JSON reader will now look like the following:
>
> Query failed: UNSUPPORTED_OPERATION ERROR: In a list of type FLOAT8, encountered a value of type BIGINT. Drill does not support lists of different types.
>
> File: /Users/pchandra/work/data/test/DRILL-2350.json
> Record: 1
> Line: 2
> Column: 23
> Field: loc
>
> [c4b33cca-0a52-4126-a300-2ab91d0ed9d1 on localhost:31010]
>
>
> Diffs
> -----
>
> exec/java-exec/src/main/codegen/includes/vv_imports.ftl 371d8d0
> exec/java-exec/src/main/codegen/templates/ListWriters.java 29708d7
> exec/java-exec/src/main/java/org/apache/drill/exec/store/easy/json/JSONRecordReader.java b41de31
> exec/java-exec/src/main/java/org/apache/drill/exec/store/easy/json/JsonProcessor.java b310818
> exec/java-exec/src/main/java/org/apache/drill/exec/store/easy/json/reader/BaseJsonProcessor.java 718bb09
> exec/java-exec/src/main/java/org/apache/drill/exec/vector/complex/fn/JsonReader.java c196fd2
> exec/java-exec/src/test/java/org/apache/drill/exec/store/json/TestJsonRecordReader.java 8b09e80
> exec/java-exec/src/test/resources/jsoninput/DRILL-2350.json PRE-CREATION
>
> Diff: https://reviews.apache.org/r/33289/diff/
>
>
> Testing
> -------
>
> All unit tests.
>
>
> Thanks,
>
> Parth Chandra
>
>
Re: Review Request 33289: DRILL-2350: Improve exception handling and
error messages in JSON reader.
Posted by Parth Chandra <pc...@maprtech.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/33289/
-----------------------------------------------------------
(Updated April 21, 2015, 7:56 p.m.)
Review request for drill, abdelhakim deneche and Jason Altekruse.
Changes
-------
Updated file
Repository: drill-git
Description
-------
DRILL-2350: Improve exception handling and error messages in JSON reader.
Errors reported from the JSON reader will now look like the following:
Query failed: UNSUPPORTED_OPERATION ERROR: In a list of type FLOAT8, encountered a value of type BIGINT. Drill does not support lists of different types.
File: /Users/pchandra/work/data/test/DRILL-2350.json
Record: 1
Line: 2
Column: 23
Field: loc
[c4b33cca-0a52-4126-a300-2ab91d0ed9d1 on localhost:31010]
Diffs (updated)
-----
exec/java-exec/src/main/codegen/includes/vv_imports.ftl 371d8d0
exec/java-exec/src/main/codegen/templates/ListWriters.java 29708d7
exec/java-exec/src/main/java/org/apache/drill/exec/store/easy/json/JSONRecordReader.java b41de31
exec/java-exec/src/main/java/org/apache/drill/exec/store/easy/json/JsonProcessor.java b310818
exec/java-exec/src/main/java/org/apache/drill/exec/store/easy/json/reader/BaseJsonProcessor.java 718bb09
exec/java-exec/src/main/java/org/apache/drill/exec/vector/complex/fn/JsonReader.java c196fd2
exec/java-exec/src/test/java/org/apache/drill/exec/store/json/TestJsonRecordReader.java 8b09e80
exec/java-exec/src/test/resources/jsoninput/DRILL-2350.json PRE-CREATION
Diff: https://reviews.apache.org/r/33289/diff/
Testing
-------
All unit tests.
Thanks,
Parth Chandra
Re: Review Request 33289: DRILL-2350: Improve exception handling and
error messages in JSON reader.
Posted by Parth Chandra <pc...@maprtech.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/33289/
-----------------------------------------------------------
(Updated April 21, 2015, 3:44 a.m.)
Review request for drill, abdelhakim deneche and Jason Altekruse.
Changes
-------
OK. So I've added the additional check for the message as well. But perhaps we should add an interface in the test builder to check for negative cases as well.
Repository: drill-git
Description
-------
DRILL-2350: Improve exception handling and error messages in JSON reader.
Errors reported from the JSON reader will now look like the following:
Query failed: UNSUPPORTED_OPERATION ERROR: In a list of type FLOAT8, encountered a value of type BIGINT. Drill does not support lists of different types.
File: /Users/pchandra/work/data/test/DRILL-2350.json
Record: 1
Line: 2
Column: 23
Field: loc
[c4b33cca-0a52-4126-a300-2ab91d0ed9d1 on localhost:31010]
Diffs (updated)
-----
exec/java-exec/src/main/codegen/includes/vv_imports.ftl 371d8d0
exec/java-exec/src/main/codegen/templates/ListWriters.java 29708d7
exec/java-exec/src/main/java/org/apache/drill/exec/store/easy/json/JSONRecordReader.java b41de31
exec/java-exec/src/main/java/org/apache/drill/exec/store/easy/json/JsonProcessor.java b310818
exec/java-exec/src/main/java/org/apache/drill/exec/store/easy/json/reader/BaseJsonProcessor.java 718bb09
exec/java-exec/src/main/java/org/apache/drill/exec/vector/complex/fn/JsonReader.java c196fd2
exec/java-exec/src/test/java/org/apache/drill/exec/store/json/TestJsonRecordReader.java 8b09e80
exec/java-exec/src/test/resources/jsoninput/DRILL-2350.json PRE-CREATION
Diff: https://reviews.apache.org/r/33289/diff/
Testing
-------
All unit tests.
Thanks,
Parth Chandra
Re: Review Request 33289: DRILL-2350: Improve exception handling and
error messages in JSON reader.
Posted by Parth Chandra <pc...@maprtech.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/33289/
-----------------------------------------------------------
(Updated April 20, 2015, 9:31 p.m.)
Review request for drill, abdelhakim deneche and Jason Altekruse.
Changes
-------
Updated with Hakim's suggestion
Repository: drill-git
Description
-------
DRILL-2350: Improve exception handling and error messages in JSON reader.
Errors reported from the JSON reader will now look like the following:
Query failed: UNSUPPORTED_OPERATION ERROR: In a list of type FLOAT8, encountered a value of type BIGINT. Drill does not support lists of different types.
File: /Users/pchandra/work/data/test/DRILL-2350.json
Record: 1
Line: 2
Column: 23
Field: loc
[c4b33cca-0a52-4126-a300-2ab91d0ed9d1 on localhost:31010]
Diffs (updated)
-----
exec/java-exec/src/main/codegen/includes/vv_imports.ftl 371d8d0
exec/java-exec/src/main/codegen/templates/ListWriters.java 29708d7
exec/java-exec/src/main/java/org/apache/drill/exec/store/easy/json/JSONRecordReader.java b41de31
exec/java-exec/src/main/java/org/apache/drill/exec/store/easy/json/JsonProcessor.java b310818
exec/java-exec/src/main/java/org/apache/drill/exec/store/easy/json/reader/BaseJsonProcessor.java 718bb09
exec/java-exec/src/main/java/org/apache/drill/exec/vector/complex/fn/JsonReader.java c196fd2
exec/java-exec/src/test/java/org/apache/drill/exec/store/json/TestJsonRecordReader.java 8b09e80
exec/java-exec/src/test/resources/jsoninput/DRILL-2350.json PRE-CREATION
Diff: https://reviews.apache.org/r/33289/diff/
Testing
-------
All unit tests.
Thanks,
Parth Chandra
Re: Review Request 33289: DRILL-2350: Improve exception handling and
error messages in JSON reader.
Posted by abdelhakim deneche <ad...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/33289/#review80818
-----------------------------------------------------------
exec/java-exec/src/test/java/org/apache/drill/exec/store/json/TestJsonRecordReader.java
<https://reviews.apache.org/r/33289/#comment130929>
you can also do something like this:
```
} catch(UserException e) {
Assert.assertEquals(DrillPBError.ErrorType.UNSUPPORTED_OPERATION, e.getOrCreatePBError(false).getErrorType());
}
```
This way, even if we decide to change the error message for Unsupported Errors, this test will still succeed.
I should probably add a getErrorType() to UserException.
- abdelhakim deneche
On April 20, 2015, 8:44 p.m., Parth Chandra wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/33289/
> -----------------------------------------------------------
>
> (Updated April 20, 2015, 8:44 p.m.)
>
>
> Review request for drill, abdelhakim deneche and Jason Altekruse.
>
>
> Repository: drill-git
>
>
> Description
> -------
>
> DRILL-2350: Improve exception handling and error messages in JSON reader.
>
> Errors reported from the JSON reader will now look like the following:
>
> Query failed: UNSUPPORTED_OPERATION ERROR: In a list of type FLOAT8, encountered a value of type BIGINT. Drill does not support lists of different types.
>
> File: /Users/pchandra/work/data/test/DRILL-2350.json
> Record: 1
> Line: 2
> Column: 23
> Field: loc
>
> [c4b33cca-0a52-4126-a300-2ab91d0ed9d1 on localhost:31010]
>
>
> Diffs
> -----
>
> exec/java-exec/src/main/codegen/includes/vv_imports.ftl 371d8d0
> exec/java-exec/src/main/codegen/templates/ListWriters.java 29708d7
> exec/java-exec/src/main/java/org/apache/drill/exec/store/easy/json/JSONRecordReader.java b41de31
> exec/java-exec/src/main/java/org/apache/drill/exec/store/easy/json/JsonProcessor.java b310818
> exec/java-exec/src/main/java/org/apache/drill/exec/store/easy/json/reader/BaseJsonProcessor.java 718bb09
> exec/java-exec/src/main/java/org/apache/drill/exec/vector/complex/fn/JsonReader.java c196fd2
> exec/java-exec/src/test/java/org/apache/drill/exec/store/json/TestJsonRecordReader.java 8b09e80
> exec/java-exec/src/test/resources/jsoninput/DRILL-2350.json PRE-CREATION
>
> Diff: https://reviews.apache.org/r/33289/diff/
>
>
> Testing
> -------
>
> All unit tests.
>
>
> Thanks,
>
> Parth Chandra
>
>
Re: Review Request 33289: DRILL-2350: Improve exception handling and
error messages in JSON reader.
Posted by Parth Chandra <pc...@maprtech.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/33289/
-----------------------------------------------------------
(Updated April 20, 2015, 8:44 p.m.)
Review request for drill, abdelhakim deneche and Jason Altekruse.
Repository: drill-git
Description
-------
DRILL-2350: Improve exception handling and error messages in JSON reader.
Errors reported from the JSON reader will now look like the following:
Query failed: UNSUPPORTED_OPERATION ERROR: In a list of type FLOAT8, encountered a value of type BIGINT. Drill does not support lists of different types.
File: /Users/pchandra/work/data/test/DRILL-2350.json
Record: 1
Line: 2
Column: 23
Field: loc
[c4b33cca-0a52-4126-a300-2ab91d0ed9d1 on localhost:31010]
Diffs (updated)
-----
exec/java-exec/src/main/codegen/includes/vv_imports.ftl 371d8d0
exec/java-exec/src/main/codegen/templates/ListWriters.java 29708d7
exec/java-exec/src/main/java/org/apache/drill/exec/store/easy/json/JSONRecordReader.java b41de31
exec/java-exec/src/main/java/org/apache/drill/exec/store/easy/json/JsonProcessor.java b310818
exec/java-exec/src/main/java/org/apache/drill/exec/store/easy/json/reader/BaseJsonProcessor.java 718bb09
exec/java-exec/src/main/java/org/apache/drill/exec/vector/complex/fn/JsonReader.java c196fd2
exec/java-exec/src/test/java/org/apache/drill/exec/store/json/TestJsonRecordReader.java 8b09e80
exec/java-exec/src/test/resources/jsoninput/DRILL-2350.json PRE-CREATION
Diff: https://reviews.apache.org/r/33289/diff/
Testing
-------
All unit tests.
Thanks,
Parth Chandra