I went ahead a put up a PR with the change and some tests. I'm currently preparing a release, and as the change is so small, and fixes a bug in a supported interface, I think it makes sense to include it. [1]
[1] - https://github.com/apache/drill/pull/353 On Mon, Feb 1, 2016 at 4:58 PM, Jason Altekruse <altekruseja...@gmail.com> wrote: > Hey Devender, > > I have created a small test case that reproduces this issue by modifying > one of our existing JDBC tests. Would you like to apply this change to your > branch and add similar tests for all of the data types? [1] > > You can see that the previous test cases covered checking the return > values from getString() for all of the types, but there was no case that > checked for null values in these cases. > > [1] - > https://github.com/jaltekruse/incubator-drill/commit/2e1e6f5c3eaf241a0c330abc19982b9d1d40a1c6 > - Jason > > On Mon, Feb 1, 2016 at 12:42 PM, Jason Altekruse <altekruseja...@gmail.com > > wrote: > >> I just wanted you get get credit for your contribution, do you want to >> just open a github PR with your small change? That or you can just update >> the reivewboard to add your patch from git. >> >> On Mon, Feb 1, 2016 at 12:21 PM, Devender Yadav <dev....@gmail.com> >> wrote: >> >>> To clarify issue I will add some records from hive table that causes >>> null pointer tomorrow. >>> On 02-Feb-2016 1:36 AM, "Devender Yadav" <dev....@gmail.com> wrote: >>> >>>> Thanks for prompt reply. >>>> >>>> It's very much possible to have a null value in the data. Imagine a >>>> hive table with millions of records. The probability of finding a null >>>> value is very high. >>>> >>>> I am not familiar with Drill's source code or design patterns or >>>> tradition to handle null. I did not have experience in contributing to open >>>> source. >>>> >>>> In that patch review request. I just added null check: >>>> >>>> >>>> @Override >>>> public String getString(int rowOffset) throws InvalidAccessException >>>> { return getObject(rowOffset)==null? >>>> null:getObject(rowOffset).toString(); } >>>> >>>> instead of >>>> >>>> @Override >>>> public String getString(int rowOffset) throws InvalidAccessException >>>> { return getObject(rowOffset).toString(); } >>>> >>>> >>>> We are also using Spark SQL for query federation. Spark SQL shows NULL >>>> in the query output for null values. So, I also tried on Drill & got the >>>> null pointer. >>>> >>>> >>>> >>>> >>>> >>>> >>>> >>>> >>>> >>>> >>>> Regards, >>>> Devender >>>> >>>> On Tue, Feb 2, 2016 at 1:25 AM, Jason Altekruse < >>>> altekruseja...@gmail.com> wrote: >>>> >>>>> Sorry for losing track of this, we do fully support the JDBC driver. I >>>>> did take a look at the review request you had opened and forgot to comment >>>>> on it, I don't think there is actually a patch associated with the review. >>>>> >>>>> I'm not very familiar with the JDBC driver code myself, but it looks >>>>> like this should only happen if you are requesting a string from a >>>>> non-varchar field, as the NullableVarCharAccessor method that extends >>>>> AbstractSqlAccessor overrides this method properly handling the null case. >>>>> >>>>> I am just trying to understand the issue and why we didn't have a test >>>>> case for it. >>>>> >>>>> On Mon, Feb 1, 2016 at 11:30 AM, Devender Yadav <dev....@gmail.com> >>>>> wrote: >>>>> >>>>>> Hi, >>>>>> >>>>>> This null pointer seems a blocker to me. I raised an issue. Asked on >>>>>> dev & user both groups. This is a one-liner fix. I added patch also. I >>>>>> find >>>>>> fewer people using Drill JDBC. So, nobody cares about that issue. >>>>>> >>>>>> >>>>>> >>>>>> On Fri, Jan 8, 2016 at 2:46 PM, Devender Yadav <dev....@gmail.com> >>>>>> wrote: >>>>>> >>>>>>> @Jason Altekruse >>>>>>> >>>>>>> Thanks for the reply. >>>>>>> >>>>>>> This is a very silly bug & critical for JDBC users. Please look into >>>>>>> it once. >>>>>>> >>>>>>> Regards, >>>>>>> Devender >>>>>>> >>>>>>> On Tue, Jan 5, 2016 at 2:03 PM, Devender Yadav <dev....@gmail.com> >>>>>>> wrote: >>>>>>> >>>>>>>> I am testing Drill JDBC. While fetching results, I got null >>>>>>>> pointer at >>>>>>>> >>>>>>>> >>>>>>>> org.apache.drill.exec.vector.accessor.AbstractSqlAccessor.getString(AbstractSqlAccessor.jav >>>>>>>> a:101) >>>>>>>> >>>>>>>> Below mentioned method is throwing null pointer becaue >>>>>>>> getObject(rowOffset) returns null for null values & null.toString() >>>>>>>> is throwing *null pointer*. >>>>>>>> >>>>>>>> @Override >>>>>>>> public String getString(int rowOffset) throws InvalidAccessException >>>>>>>> { return getObject(rowOffset).toString(); } >>>>>>>> >>>>>>>> >>>>>>>> I raised an *issue*( >>>>>>>> https://issues.apache.org/jira/browse/DRILL-4128) on Drill JIRA. >>>>>>>> Regards, >>>>>>>> Devender >>>>>>>> >>>>>>> >>>>>>> >>>>>> >>>>> >>>> >> >
