You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@drill.apache.org by MitchelLabonte <gi...@git.apache.org> on 2017/12/12 15:05:47 UTC
[GitHub] drill pull request #1068: DRILL-6020 Fix NullPointerException when querying ...
GitHub user MitchelLabonte opened a pull request:
https://github.com/apache/drill/pull/1068
DRILL-6020 Fix NullPointerException when querying JSON untyped path w…
…ith Union setting on
You can merge this pull request into a Git repository by running:
$ git pull https://github.com/MitchelLabonte/drill DRILL-6020
Alternatively you can review and apply these changes as the patch at:
https://github.com/apache/drill/pull/1068.patch
To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:
This closes #1068
----
commit ccd6af90fc7a4f1fbd8204d84617bd12ff3207d6
Author: mitchel <mi...@hotmail.com>
Date: 2017-12-12T14:57:55Z
DRILL-6020 Fix NullPointerException when querying JSON untyped path with Union setting on
----
---
[GitHub] drill issue #1068: DRILL-6020 Fix NullPointerException when querying JSON un...
Posted by MitchelLabonte <gi...@git.apache.org>.
Github user MitchelLabonte commented on the issue:
https://github.com/apache/drill/pull/1068
@vvysotskyi No problem, yes all tests pass now.
---
[GitHub] drill issue #1068: DRILL-6020 Fix NullPointerException when querying JSON un...
Posted by vvysotskyi <gi...@git.apache.org>.
Github user vvysotskyi commented on the issue:
https://github.com/apache/drill/pull/1068
@MitchelLabonte, thanks for the pull request, +1
---
[GitHub] drill pull request #1068: DRILL-6020 Fix NullPointerException when querying ...
Posted by asfgit <gi...@git.apache.org>.
Github user asfgit closed the pull request at:
https://github.com/apache/drill/pull/1068
---
[GitHub] drill issue #1068: DRILL-6020 Fix NullPointerException when querying JSON un...
Posted by vvysotskyi <gi...@git.apache.org>.
Github user vvysotskyi commented on the issue:
https://github.com/apache/drill/pull/1068
@MitchelLabonte, I think this NPE is just a consequence of the bug that should be fixed. Please investigate why Drill is trying to use child `PathSegment` when a value has VarChar type.
---
[GitHub] drill issue #1068: DRILL-6020 Fix NullPointerException when querying JSON un...
Posted by MitchelLabonte <gi...@git.apache.org>.
Github user MitchelLabonte commented on the issue:
https://github.com/apache/drill/pull/1068
@vvysotskyi
This is happening because the type is cached as a json object from the previous row. The fix is similar to the getFieldIdIfMatches() method so it looks like this is intended behaviour.
As you can see in the unit test, the results are what is expected after the fix. I am not sure what else could be done.
---
[GitHub] drill issue #1068: DRILL-6020 Fix NullPointerException when querying JSON un...
Posted by MitchelLabonte <gi...@git.apache.org>.
Github user MitchelLabonte commented on the issue:
https://github.com/apache/drill/pull/1068
> But one more point: with this change error is not thrown, but only nulls are returned. I think we should also fix this issue is the scope of this pull request.
@vvysotskyi I am not sure what to do to fix this. Any suggestion?
---
[GitHub] drill issue #1068: DRILL-6020 Fix NullPointerException when querying JSON un...
Posted by vvysotskyi <gi...@git.apache.org>.
Github user vvysotskyi commented on the issue:
https://github.com/apache/drill/pull/1068
Sorry, I read your removed comment and made a suggestion that this test fails.
Did you run all unit tests to see that this change does not break anything?
---
[GitHub] drill pull request #1068: DRILL-6020 Fix NullPointerException when querying ...
Posted by vvysotskyi <gi...@git.apache.org>.
Github user vvysotskyi commented on a diff in the pull request:
https://github.com/apache/drill/pull/1068#discussion_r156621897
--- Diff: exec/java-exec/src/main/java/org/apache/drill/exec/vector/complex/FieldIdUtil.java ---
@@ -131,7 +131,7 @@ public static TypedFieldId getFieldIdIfMatches(ValueVector vector, TypedFieldId.
} else if(v instanceof ListVector) {
ListVector list = (ListVector) v;
return getFieldIdIfMatches(list, builder, addToBreadCrumb, seg.getChild());
- } else if (v instanceof UnionVector) {
+ } else if (v instanceof UnionVector && !seg.isLastPath()) {
--- End diff --
Good catch!
I think we should add check for nulls into method `getFieldIdIfMatchesUnion()` as it was done for `getFieldIdIfMatches()`. Also please add a unit test for this change. You may use [testFieldWithDots()](https://github.com/apache/drill/blob/acc5ed927e1fa4011ac1c3724d15197484b9f45b/exec/java-exec/src/test/java/org/apache/drill/exec/vector/complex/writer/TestJsonReader.java#L705) as an example.
But one more point: with this change error is not thrown, but only nulls are returned. I think we should also fix this issue on the borders of this pull request.
---