Accumulo, mail # user - Re: Incorrectly setting TKey causes NPE (to nobody's surprise)

Incorrectly setting TKey causes NPE (to nobody's surprise)

David Medinets 2012-06-26, 12:56

I did this:

TKey tKey = new TKey();
tKey.setRow(row_id.getBytes());
TRange tRange = new TRange();
trange.setStart(tKey);

scan.setRange(tRange);

Iterator iterator = scan.iterator();
iterator.hasNext();

This resulted in an NPE in:

org.apache.accumulo.core.data.Key.rowColumnStringBuilder(Key.java:472)

While I have no real objection to this NPE (my code is clearly
deficient), I wonder if a more cogent error message is possible.
Should there be guard statements somewhere to ensure a valid object?

Re: Incorrectly setting TKey causes NPE (to nobody's surprise)

William Slacum 2012-06-26, 13:03

Is there a reason you're using the thrift classes?

On Tue, Jun 26, 2012 at 5:56 AM, David Medinets
<[EMAIL PROTECTED]> wrote:
> I did this:
>
> TKey tKey = new TKey();
> tKey.setRow(row_id.getBytes());
>
>
> TRange tRange = new TRange();
> trange.setStart(tKey);
>
> scan.setRange(tRange);
>
> Iterator iterator = scan.iterator();
> iterator.hasNext();
>
> This resulted in an NPE in:
>
> org.apache.accumulo.core.data.Key.rowColumnStringBuilder(Key.java:472)
>
> While I have no real objection to this NPE (my code is clearly
> deficient), I wonder if a more cogent error message is possible.
> Should there be guard statements somewhere to ensure a valid object?

Re: Incorrectly setting TKey causes NPE (to nobody's surprise)

Marc P. 2012-06-26, 13:03

Why are you using that accepts the thrift key and range? They're
internal communication objects within accumulo. I haven't looked the
code directly, but they're likely contracted to be set in a different
manner.
On Tue, Jun 26, 2012 at 8:56 AM, David Medinets
<[EMAIL PROTECTED]> wrote:
> I did this:
>
> TKey tKey = new TKey();
> tKey.setRow(row_id.getBytes());
>
>
> TRange tRange = new TRange();
> trange.setStart(tKey);
>
> scan.setRange(tRange);
>
> Iterator iterator = scan.iterator();
> iterator.hasNext();
>
> This resulted in an NPE in:
>
> org.apache.accumulo.core.data.Key.rowColumnStringBuilder(Key.java:472)
>
> While I have no real objection to this NPE (my code is clearly
> deficient), I wonder if a more cogent error message is possible.
> Should there be guard statements somewhere to ensure a valid object?

Re: Incorrectly setting TKey causes NPE (to nobody's surprise)

Marc P. 2012-06-26, 13:09

I realized that Mr Slacum and I addressed the concern of using thrift;
however, perhaps you are doing something internally. Have you tried
setting the stop key on the TRange just for S&Gs?

On Tue, Jun 26, 2012 at 9:03 AM, Marc P. <[EMAIL PROTECTED]> wrote:
> Why are you using that accepts the thrift key and range? They're
> internal communication objects within accumulo. I haven't looked the
> code directly, but they're likely contracted to be set in a different
> manner.
>
>
> On Tue, Jun 26, 2012 at 8:56 AM, David Medinets
> <[EMAIL PROTECTED]> wrote:
>> I did this:
>>
>> TKey tKey = new TKey();
>> tKey.setRow(row_id.getBytes());
>>
>>
>> TRange tRange = new TRange();
>> trange.setStart(tKey);
>>
>> scan.setRange(tRange);
>>
>> Iterator iterator = scan.iterator();
>> iterator.hasNext();
>>
>> This resulted in an NPE in:
>>
>> org.apache.accumulo.core.data.Key.rowColumnStringBuilder(Key.java:472)
>>
>> While I have no real objection to this NPE (my code is clearly
>> deficient), I wonder if a more cogent error message is possible.
>> Should there be guard statements somewhere to ensure a valid object?

Re: Incorrectly setting TKey causes NPE (to nobody's surprise)

David Medinets 2012-06-26, 13:17

I play 'stupid developer' fairly well. I saw something that defines a
key and started to use it. If I set row, cf, cq, and visibility then
the iterator works fine.

Is there any reasons why default values of "" should not be provided
for cf, cq, and visibility?

On Tue, Jun 26, 2012 at 9:09 AM, Marc P. <[EMAIL PROTECTED]> wrote:
> I realized that Mr Slacum and I addressed the concern of using thrift;
> however, perhaps you are doing something internally. Have you tried
> setting the stop key on the TRange just for S&Gs?
>
> On Tue, Jun 26, 2012 at 9:03 AM, Marc P. <[EMAIL PROTECTED]> wrote:
>> Why are you using that accepts the thrift key and range? They're
>> internal communication objects within accumulo. I haven't looked the
>> code directly, but they're likely contracted to be set in a different
>> manner.
>>
>>
>> On Tue, Jun 26, 2012 at 8:56 AM, David Medinets
>> <[EMAIL PROTECTED]> wrote:
>>> I did this:
>>>
>>> TKey tKey = new TKey();
>>> tKey.setRow(row_id.getBytes());
>>>
>>>
>>> TRange tRange = new TRange();
>>> trange.setStart(tKey);
>>>
>>> scan.setRange(tRange);
>>>
>>> Iterator iterator = scan.iterator();
>>> iterator.hasNext();
>>>
>>> This resulted in an NPE in:
>>>
>>> org.apache.accumulo.core.data.Key.rowColumnStringBuilder(Key.java:472)
>>>
>>> While I have no real objection to this NPE (my code is clearly
>>> deficient), I wonder if a more cogent error message is possible.
>>> Should there be guard statements somewhere to ensure a valid object?

Re: Incorrectly setting TKey causes NPE (to nobody's surprise)

Adam Fuchs 2012-06-26, 14:20

The tradeoff would be convenience versus complexity in the API. I would
lean towards having fewer ways to create a Key.

Has this debate played out before?
http://www.wikivs.com/wiki/Python_vs_Ruby#Philosophy

Adam

On Tue, Jun 26, 2012 at 9:17 AM, David Medinets <[EMAIL PROTECTED]>wrote:

> I play 'stupid developer' fairly well. I saw something that defines a
> key and started to use it. If I set row, cf, cq, and visibility then
> the iterator works fine.
>
> Is there any reasons why default values of "" should not be provided
> for cf, cq, and visibility?
>
> On Tue, Jun 26, 2012 at 9:09 AM, Marc P. <[EMAIL PROTECTED]> wrote:
> > I realized that Mr Slacum and I addressed the concern of using thrift;
> > however, perhaps you are doing something internally. Have you tried
> > setting the stop key on the TRange just for S&Gs?
> >
> > On Tue, Jun 26, 2012 at 9:03 AM, Marc P. <[EMAIL PROTECTED]> wrote:
> >> Why are you using that accepts the thrift key and range? They're
> >> internal communication objects within accumulo. I haven't looked the
> >> code directly, but they're likely contracted to be set in a different
> >> manner.
> >>
> >>
> >> On Tue, Jun 26, 2012 at 8:56 AM, David Medinets
> >> <[EMAIL PROTECTED]> wrote:
> >>> I did this:
> >>>
> >>> TKey tKey = new TKey();
> >>> tKey.setRow(row_id.getBytes());
> >>>
> >>>
> >>> TRange tRange = new TRange();
> >>> trange.setStart(tKey);
> >>>
> >>> scan.setRange(tRange);
> >>>
> >>> Iterator iterator = scan.iterator();
> >>> iterator.hasNext();
> >>>
> >>> This resulted in an NPE in:
> >>>
> >>> org.apache.accumulo.core.data.Key.rowColumnStringBuilder(Key.java:472)
> >>>
> >>> While I have no real objection to this NPE (my code is clearly
> >>> deficient), I wonder if a more cogent error message is possible.
> >>> Should there be guard statements somewhere to ensure a valid object?
>

Re: Incorrectly setting TKey causes NPE (to nobody's surprise)

Marc P. 2012-06-26, 14:31

I don't agree with that last promotion. From a usability perspective,
I think it would be better to either require all arguments or allow
them not to be set, instead of throwing an exception. This will create
confusion, especially with people unfamiliar with the stack, as
evidenced by David's question.

On Tue, Jun 26, 2012 at 10:20 AM, Adam Fuchs <[EMAIL PROTECTED]> wrote:
> The tradeoff would be convenience versus complexity in the API. I would lean
> towards having fewer ways to create a Key.
>
> Has this debate played out
> before? http://www.wikivs.com/wiki/Python_vs_Ruby#Philosophy
>
> Adam
>
>
>
> On Tue, Jun 26, 2012 at 9:17 AM, David Medinets <[EMAIL PROTECTED]>
> wrote:
>>
>> I play 'stupid developer' fairly well. I saw something that defines a
>> key and started to use it. If I set row, cf, cq, and visibility then
>> the iterator works fine.
>>
>> Is there any reasons why default values of "" should not be provided
>> for cf, cq, and visibility?
>>
>> On Tue, Jun 26, 2012 at 9:09 AM, Marc P. <[EMAIL PROTECTED]> wrote:
>> > I realized that Mr Slacum and I addressed the concern of using thrift;
>> > however, perhaps you are doing something internally. Have you tried
>> > setting the stop key on the TRange just for S&Gs?
>> >
>> > On Tue, Jun 26, 2012 at 9:03 AM, Marc P. <[EMAIL PROTECTED]> wrote:
>> >> Why are you using that accepts the thrift key and range? They're
>> >> internal communication objects within accumulo. I haven't looked the
>> >> code directly, but they're likely contracted to be set in a different
>> >> manner.
>> >>
>> >>
>> >> On Tue, Jun 26, 2012 at 8:56 AM, David Medinets
>> >> <[EMAIL PROTECTED]> wrote:
>> >>> I did this:
>> >>>
>> >>> TKey tKey = new TKey();
>> >>> tKey.setRow(row_id.getBytes());
>> >>>
>> >>>
>> >>> TRange tRange = new TRange();
>> >>> trange.setStart(tKey);
>> >>>
>> >>> scan.setRange(tRange);
>> >>>
>> >>> Iterator iterator = scan.iterator();
>> >>> iterator.hasNext();
>> >>>
>> >>> This resulted in an NPE in:
>> >>>
>> >>> org.apache.accumulo.core.data.Key.rowColumnStringBuilder(Key.java:472)
>> >>>
>> >>> While I have no real objection to this NPE (my code is clearly
>> >>> deficient), I wonder if a more cogent error message is possible.
>> >>> Should there be guard statements somewhere to ensure a valid object?
>
>

Re: Incorrectly setting TKey causes NPE (to nobody's surprise)

Adam Fuchs 2012-06-26, 15:30

Mark,

You're right, my answer was totally misdirected.

The real reason is that TKey is not very user-friendly is that it is auto
generated from the code in core/src/main/thrift/data.thrift. We don't
intend for these auto-generated classes to be part of the public API. Also,
we don't hand-code them (anymore) to make them convenient because we are
likely to re-generate them when we upgrade Thrift in the future. Bottom
line: don't use classes that are in a *.thrift package in your client code.

Adam

On Tue, Jun 26, 2012 at 10:31 AM, Marc P. <[EMAIL PROTECTED]> wrote:

> I don't agree with that last promotion. From a usability perspective,
> I think it would be better to either require all arguments or allow
> them not to be set, instead of throwing an exception. This will create
> confusion, especially with people unfamiliar with the stack, as
> evidenced by David's question.
>
> On Tue, Jun 26, 2012 at 10:20 AM, Adam Fuchs <[EMAIL PROTECTED]> wrote:
> > The tradeoff would be convenience versus complexity in the API. I would
> lean
> > towards having fewer ways to create a Key.
> >
> > Has this debate played out
> > before? http://www.wikivs.com/wiki/Python_vs_Ruby#Philosophy
> >
> > Adam
> >
> >
> >
> > On Tue, Jun 26, 2012 at 9:17 AM, David Medinets <
> [EMAIL PROTECTED]>
> > wrote:
> >>
> >> I play 'stupid developer' fairly well. I saw something that defines a
> >> key and started to use it. If I set row, cf, cq, and visibility then
> >> the iterator works fine.
> >>
> >> Is there any reasons why default values of "" should not be provided
> >> for cf, cq, and visibility?
> >>
> >> On Tue, Jun 26, 2012 at 9:09 AM, Marc P. <[EMAIL PROTECTED]> wrote:
> >> > I realized that Mr Slacum and I addressed the concern of using thrift;
> >> > however, perhaps you are doing something internally. Have you tried
> >> > setting the stop key on the TRange just for S&Gs?
> >> >
> >> > On Tue, Jun 26, 2012 at 9:03 AM, Marc P. <[EMAIL PROTECTED]>
> wrote:
> >> >> Why are you using that accepts the thrift key and range? They're
> >> >> internal communication objects within accumulo. I haven't looked the
> >> >> code directly, but they're likely contracted to be set in a different
> >> >> manner.
> >> >>
> >> >>
> >> >> On Tue, Jun 26, 2012 at 8:56 AM, David Medinets
> >> >> <[EMAIL PROTECTED]> wrote:
> >> >>> I did this:
> >> >>>
> >> >>> TKey tKey = new TKey();
> >> >>> tKey.setRow(row_id.getBytes());
> >> >>>
> >> >>>
> >> >>> TRange tRange = new TRange();
> >> >>> trange.setStart(tKey);
> >> >>>
> >> >>> scan.setRange(tRange);
> >> >>>
> >> >>> Iterator iterator = scan.iterator();
> >> >>> iterator.hasNext();
> >> >>>
> >> >>> This resulted in an NPE in:
> >> >>>
> >> >>>
> org.apache.accumulo.core.data.Key.rowColumnStringBuilder(Key.java:472)
> >> >>>
> >> >>> While I have no real objection to this NPE (my code is clearly
> >> >>> deficient), I wonder if a more cogent error message is possible.
> >> >>> Should there be guard statements somewhere to ensure a valid object?
> >
> >
>

Re: Incorrectly setting TKey causes NPE (to nobody's surprise)

David Medinets 2012-06-26, 16:20

Adam, your answer makes sense. I replaced the TKey with Key without a problem.

On Tue, Jun 26, 2012 at 11:30 AM, Adam Fuchs <[EMAIL PROTECTED]> wrote:
> Mark,
>
> You're right, my answer was totally misdirected.
>
> The real reason is that TKey is not very user-friendly is that it is auto
> generated from the code in core/src/main/thrift/data.thrift. We don't intend
> for these auto-generated classes to be part of the public API. Also, we
> don't hand-code them (anymore) to make them convenient because we are
> likely to re-generate them when we upgrade Thrift in the future. Bottom
> line: don't use classes that are in a *.thrift package in your client code.
>
> Adam
>
>
>
> On Tue, Jun 26, 2012 at 10:31 AM, Marc P. <[EMAIL PROTECTED]> wrote:
>>
>> I don't agree with that last promotion. From a usability perspective,
>> I think it would be better to either require all arguments or allow
>> them not to be set, instead of throwing an exception. This will create
>> confusion, especially with people unfamiliar with the stack, as
>> evidenced by David's question.
>>
>> On Tue, Jun 26, 2012 at 10:20 AM, Adam Fuchs <[EMAIL PROTECTED]> wrote:
>> > The tradeoff would be convenience versus complexity in the API. I would
>> > lean
>> > towards having fewer ways to create a Key.
>> >
>> > Has this debate played out
>> > before? http://www.wikivs.com/wiki/Python_vs_Ruby#Philosophy
>> >
>> > Adam
>> >
>> >
>> >
>> > On Tue, Jun 26, 2012 at 9:17 AM, David Medinets
>> > <[EMAIL PROTECTED]>
>> > wrote:
>> >>
>> >> I play 'stupid developer' fairly well. I saw something that defines a
>> >> key and started to use it. If I set row, cf, cq, and visibility then
>> >> the iterator works fine.
>> >>
>> >> Is there any reasons why default values of "" should not be provided
>> >> for cf, cq, and visibility?
>> >>
>> >> On Tue, Jun 26, 2012 at 9:09 AM, Marc P. <[EMAIL PROTECTED]> wrote:
>> >> > I realized that Mr Slacum and I addressed the concern of using
>> >> > thrift;
>> >> > however, perhaps you are doing something internally. Have you tried
>> >> > setting the stop key on the TRange just for S&Gs?
>> >> >
>> >> > On Tue, Jun 26, 2012 at 9:03 AM, Marc P. <[EMAIL PROTECTED]>
>> >> > wrote:
>> >> >> Why are you using that accepts the thrift key and range? They're
>> >> >> internal communication objects within accumulo. I haven't looked the
>> >> >> code directly, but they're likely contracted to be set in a
>> >> >> different
>> >> >> manner.
>> >> >>
>> >> >>
>> >> >> On Tue, Jun 26, 2012 at 8:56 AM, David Medinets
>> >> >> <[EMAIL PROTECTED]> wrote:
>> >> >>> I did this:
>> >> >>>
>> >> >>> TKey tKey = new TKey();
>> >> >>> tKey.setRow(row_id.getBytes());
>> >> >>>
>> >> >>>
>> >> >>> TRange tRange = new TRange();
>> >> >>> trange.setStart(tKey);
>> >> >>>
>> >> >>> scan.setRange(tRange);
>> >> >>>
>> >> >>> Iterator iterator = scan.iterator();
>> >> >>> iterator.hasNext();
>> >> >>>
>> >> >>> This resulted in an NPE in:
>> >> >>>
>> >> >>>
>> >> >>> org.apache.accumulo.core.data.Key.rowColumnStringBuilder(Key.java:472)
>> >> >>>
>> >> >>> While I have no real objection to this NPE (my code is clearly
>> >> >>> deficient), I wonder if a more cogent error message is possible.
>> >> >>> Should there be guard statements somewhere to ensure a valid
>> >> >>> object?
>> >
>> >
>
>