drill-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Jason Altekruse <altekruseja...@gmail.com>
Subject Re: null pointer at AbstractSqlAccessor.getString()
Date Tue, 02 Feb 2016 02:45:29 GMT
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 <altekrusejason@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 <altekrusejason@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.emr@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.emr@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 <
>>>> altekrusejason@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.emr@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.emr@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.emr@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
>>>>>>>>
>>>>>>>
>>>>>>>
>>>>>>
>>>>>
>>>>
>>
>

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