Re: Tags class using wrong length?

[Chia-Ping Tsai]

Sorry for the typo. I mean that "bytes[offset + TAG_LENGTH_SIZE]" is correct.

BTW, the name of "upcoming HBase book" you mentioned is "HBase: The Definitive 
Guide"?

On 2017-08-07 14:13, Lars George  wrote: 
> Gotcha, sorry for the noise. I documented properly in the upcoming HBase 
> book. :)
> 
> Sent from my iPhone
> 
> > On 7. Aug 2017, at 07:02, ramkrishna vasudevan 
> >  wrote:
> > 
> > I think the layout of tags is missing now in the javadoc. May be it got
> > missed or moved to some other place?
> > I remember we had a layout explaining the tag structure then this code is
> > much easier to read this code.
> > 
> > As Chia-Ping said  is the
> > layout.
> > So from the KeyValue lay out we extract the tag part which in itself has a
> > tag length to represent the complete set of tags.
> > 
> > From the tags offset and tags length from the KV we extract individual tags
> > in that KV.
> > 
> > For eg
> > See TagUtil#asList
> > 
> > {code}
> > List tags = new ArrayList<>();
> >int pos = offset;
> >while (pos < offset + length) {
> >  int tagLen = Bytes.readAsInt(b, pos, TAG_LENGTH_SIZE);
> >  tags.add(new ArrayBackedTag(b, pos, tagLen + TAG_LENGTH_SIZE));
> >  pos += TAG_LENGTH_SIZE + tagLen;
> >}
> >return tags;
> > {code}
> > 
> > Regards
> > Ram
> > 
> > 
> > 
> > 
> >> On Mon, Aug 7, 2017 at 3:25 AM, Ted Yu  wrote:
> >> 
> >> The byte following the tag length (a short) is the tag type.
> >> 
> >> The current code is correct.
> >> 
> >> On Sun, Aug 6, 2017 at 5:40 AM, Chia-Ping Tsai 
> >> wrote:
> >> 
> >>> According to the following code:
> >>>  public ArrayBackedTag(byte tagType, byte[] tag) {
> >>>int tagLength = tag.length + TYPE_LENGTH_SIZE;
> >>>if (tagLength > MAX_TAG_LENGTH) {
> >>>  throw new IllegalArgumentException(
> >>>  "Invalid tag data being passed. Its length can not exceed " +
> >>> MAX_TAG_LENGTH);
> >>>}
> >>>length = TAG_LENGTH_SIZE + tagLength;
> >>>bytes = new byte[length];
> >>>int pos = Bytes.putAsShort(bytes, 0, tagLength);
> >>>pos = Bytes.putByte(bytes, pos, tagType);
> >>>Bytes.putBytes(bytes, pos, tag, 0, tag.length);
> >>>this.type = tagType;
> >>>  }
> >>> The layout of the byte array should be:
> >>> |tag legnth (2 bytes)|tag type(1 byte)|tag|
> >>> 
> >>> It seems to me that the "bytes[offset + TYPE_LENGTH_SIZE]" is correct.
> >>> 
>  On 2017-08-06 16:35, Lars George  wrote:
>  Hi,
>  
>  I found this reading through tags in 1.3, but checked in trunk as
>  well. There is this code:
>  
>   public ArrayBackedTag(byte[] bytes, int offset, int length) {
> if (length > MAX_TAG_LENGTH) {
>   throw new IllegalArgumentException(
>   "Invalid tag data being passed. Its length can not exceed "
>  + MAX_TAG_LENGTH);
> }
> this.bytes = bytes;
> this.offset = offset;
> this.length = length;
> this.type = bytes[offset + TAG_LENGTH_SIZE];
>   }
>  
>  I am concerned about the last line of the code, using the wrong
> >> constant?
>  
>   public final static int TYPE_LENGTH_SIZE = Bytes.SIZEOF_BYTE;
>   public final static int TAG_LENGTH_SIZE = Bytes.SIZEOF_SHORT;
>  
>  Should this not read
>  
> this.type = bytes[offset + TYPE_LENGTH_SIZE];
>  
>  Would this not read the type from the wrong place in the array?
>  
>  Cheers,
>  Lars
>  
> >>> 
> >> 
> 

Re: Tags class using wrong length?

[Lars George]

Gotcha, sorry for the noise. I documented properly in the upcoming HBase book. 
:)

Sent from my iPhone

> On 7. Aug 2017, at 07:02, ramkrishna vasudevan 
>  wrote:
> 
> I think the layout of tags is missing now in the javadoc. May be it got
> missed or moved to some other place?
> I remember we had a layout explaining the tag structure then this code is
> much easier to read this code.
> 
> As Chia-Ping said  is the
> layout.
> So from the KeyValue lay out we extract the tag part which in itself has a
> tag length to represent the complete set of tags.
> 
> From the tags offset and tags length from the KV we extract individual tags
> in that KV.
> 
> For eg
> See TagUtil#asList
> 
> {code}
> List tags = new ArrayList<>();
>int pos = offset;
>while (pos < offset + length) {
>  int tagLen = Bytes.readAsInt(b, pos, TAG_LENGTH_SIZE);
>  tags.add(new ArrayBackedTag(b, pos, tagLen + TAG_LENGTH_SIZE));
>  pos += TAG_LENGTH_SIZE + tagLen;
>}
>return tags;
> {code}
> 
> Regards
> Ram
> 
> 
> 
> 
>> On Mon, Aug 7, 2017 at 3:25 AM, Ted Yu  wrote:
>> 
>> The byte following the tag length (a short) is the tag type.
>> 
>> The current code is correct.
>> 
>> On Sun, Aug 6, 2017 at 5:40 AM, Chia-Ping Tsai 
>> wrote:
>> 
>>> According to the following code:
>>>  public ArrayBackedTag(byte tagType, byte[] tag) {
>>>int tagLength = tag.length + TYPE_LENGTH_SIZE;
>>>if (tagLength > MAX_TAG_LENGTH) {
>>>  throw new IllegalArgumentException(
>>>  "Invalid tag data being passed. Its length can not exceed " +
>>> MAX_TAG_LENGTH);
>>>}
>>>length = TAG_LENGTH_SIZE + tagLength;
>>>bytes = new byte[length];
>>>int pos = Bytes.putAsShort(bytes, 0, tagLength);
>>>pos = Bytes.putByte(bytes, pos, tagType);
>>>Bytes.putBytes(bytes, pos, tag, 0, tag.length);
>>>this.type = tagType;
>>>  }
>>> The layout of the byte array should be:
>>> |tag legnth (2 bytes)|tag type(1 byte)|tag|
>>> 
>>> It seems to me that the "bytes[offset + TYPE_LENGTH_SIZE]" is correct.
>>> 
 On 2017-08-06 16:35, Lars George  wrote:
 Hi,
 
 I found this reading through tags in 1.3, but checked in trunk as
 well. There is this code:
 
  public ArrayBackedTag(byte[] bytes, int offset, int length) {
if (length > MAX_TAG_LENGTH) {
  throw new IllegalArgumentException(
  "Invalid tag data being passed. Its length can not exceed "
 + MAX_TAG_LENGTH);
}
this.bytes = bytes;
this.offset = offset;
this.length = length;
this.type = bytes[offset + TAG_LENGTH_SIZE];
  }
 
 I am concerned about the last line of the code, using the wrong
>> constant?
 
  public final static int TYPE_LENGTH_SIZE = Bytes.SIZEOF_BYTE;
  public final static int TAG_LENGTH_SIZE = Bytes.SIZEOF_SHORT;
 
 Should this not read
 
this.type = bytes[offset + TYPE_LENGTH_SIZE];
 
 Would this not read the type from the wrong place in the array?
 
 Cheers,
 Lars
 
>>> 
>> 

Re: Tags class using wrong length?

[ramkrishna vasudevan]

I think the layout of tags is missing now in the javadoc. May be it got
missed or moved to some other place?
I remember we had a layout explaining the tag structure then this code is
much easier to read this code.

As Chia-Ping said  is the
layout.
So from the KeyValue lay out we extract the tag part which in itself has a
tag length to represent the complete set of tags.

>From the tags offset and tags length from the KV we extract individual tags
in that KV.

For eg
See TagUtil#asList

{code}
 List tags = new ArrayList<>();
int pos = offset;
while (pos < offset + length) {
  int tagLen = Bytes.readAsInt(b, pos, TAG_LENGTH_SIZE);
  tags.add(new ArrayBackedTag(b, pos, tagLen + TAG_LENGTH_SIZE));
  pos += TAG_LENGTH_SIZE + tagLen;
}
return tags;
{code}

Regards
Ram




On Mon, Aug 7, 2017 at 3:25 AM, Ted Yu  wrote:

> The byte following the tag length (a short) is the tag type.
>
> The current code is correct.
>
> On Sun, Aug 6, 2017 at 5:40 AM, Chia-Ping Tsai 
> wrote:
>
> > According to the following code:
> >   public ArrayBackedTag(byte tagType, byte[] tag) {
> > int tagLength = tag.length + TYPE_LENGTH_SIZE;
> > if (tagLength > MAX_TAG_LENGTH) {
> >   throw new IllegalArgumentException(
> >   "Invalid tag data being passed. Its length can not exceed " +
> > MAX_TAG_LENGTH);
> > }
> > length = TAG_LENGTH_SIZE + tagLength;
> > bytes = new byte[length];
> > int pos = Bytes.putAsShort(bytes, 0, tagLength);
> > pos = Bytes.putByte(bytes, pos, tagType);
> > Bytes.putBytes(bytes, pos, tag, 0, tag.length);
> > this.type = tagType;
> >   }
> > The layout of the byte array should be:
> > |tag legnth (2 bytes)|tag type(1 byte)|tag|
> >
> > It seems to me that the "bytes[offset + TYPE_LENGTH_SIZE]" is correct.
> >
> > On 2017-08-06 16:35, Lars George  wrote:
> > > Hi,
> > >
> > > I found this reading through tags in 1.3, but checked in trunk as
> > > well. There is this code:
> > >
> > >   public ArrayBackedTag(byte[] bytes, int offset, int length) {
> > > if (length > MAX_TAG_LENGTH) {
> > >   throw new IllegalArgumentException(
> > >   "Invalid tag data being passed. Its length can not exceed "
> > > + MAX_TAG_LENGTH);
> > > }
> > > this.bytes = bytes;
> > > this.offset = offset;
> > > this.length = length;
> > > this.type = bytes[offset + TAG_LENGTH_SIZE];
> > >   }
> > >
> > > I am concerned about the last line of the code, using the wrong
> constant?
> > >
> > >   public final static int TYPE_LENGTH_SIZE = Bytes.SIZEOF_BYTE;
> > >   public final static int TAG_LENGTH_SIZE = Bytes.SIZEOF_SHORT;
> > >
> > > Should this not read
> > >
> > > this.type = bytes[offset + TYPE_LENGTH_SIZE];
> > >
> > > Would this not read the type from the wrong place in the array?
> > >
> > > Cheers,
> > > Lars
> > >
> >
>

Re: Tags class using wrong length?

[Ted Yu]

The byte following the tag length (a short) is the tag type.

The current code is correct.

On Sun, Aug 6, 2017 at 5:40 AM, Chia-Ping Tsai  wrote:

> According to the following code:
>   public ArrayBackedTag(byte tagType, byte[] tag) {
> int tagLength = tag.length + TYPE_LENGTH_SIZE;
> if (tagLength > MAX_TAG_LENGTH) {
>   throw new IllegalArgumentException(
>   "Invalid tag data being passed. Its length can not exceed " +
> MAX_TAG_LENGTH);
> }
> length = TAG_LENGTH_SIZE + tagLength;
> bytes = new byte[length];
> int pos = Bytes.putAsShort(bytes, 0, tagLength);
> pos = Bytes.putByte(bytes, pos, tagType);
> Bytes.putBytes(bytes, pos, tag, 0, tag.length);
> this.type = tagType;
>   }
> The layout of the byte array should be:
> |tag legnth (2 bytes)|tag type(1 byte)|tag|
>
> It seems to me that the "bytes[offset + TYPE_LENGTH_SIZE]" is correct.
>
> On 2017-08-06 16:35, Lars George  wrote:
> > Hi,
> >
> > I found this reading through tags in 1.3, but checked in trunk as
> > well. There is this code:
> >
> >   public ArrayBackedTag(byte[] bytes, int offset, int length) {
> > if (length > MAX_TAG_LENGTH) {
> >   throw new IllegalArgumentException(
> >   "Invalid tag data being passed. Its length can not exceed "
> > + MAX_TAG_LENGTH);
> > }
> > this.bytes = bytes;
> > this.offset = offset;
> > this.length = length;
> > this.type = bytes[offset + TAG_LENGTH_SIZE];
> >   }
> >
> > I am concerned about the last line of the code, using the wrong constant?
> >
> >   public final static int TYPE_LENGTH_SIZE = Bytes.SIZEOF_BYTE;
> >   public final static int TAG_LENGTH_SIZE = Bytes.SIZEOF_SHORT;
> >
> > Should this not read
> >
> > this.type = bytes[offset + TYPE_LENGTH_SIZE];
> >
> > Would this not read the type from the wrong place in the array?
> >
> > Cheers,
> > Lars
> >
>

Re: Tags class using wrong length?

[Lars George]

You mean I am right and it should be the other constant being used?
Sorry, just wanting to confirm.


On Sun, Aug 6, 2017 at 2:40 PM, Chia-Ping Tsai  wrote:
> According to the following code:
>   public ArrayBackedTag(byte tagType, byte[] tag) {
> int tagLength = tag.length + TYPE_LENGTH_SIZE;
> if (tagLength > MAX_TAG_LENGTH) {
>   throw new IllegalArgumentException(
>   "Invalid tag data being passed. Its length can not exceed " + 
> MAX_TAG_LENGTH);
> }
> length = TAG_LENGTH_SIZE + tagLength;
> bytes = new byte[length];
> int pos = Bytes.putAsShort(bytes, 0, tagLength);
> pos = Bytes.putByte(bytes, pos, tagType);
> Bytes.putBytes(bytes, pos, tag, 0, tag.length);
> this.type = tagType;
>   }
> The layout of the byte array should be:
> |tag legnth (2 bytes)|tag type(1 byte)|tag|
>
> It seems to me that the "bytes[offset + TYPE_LENGTH_SIZE]" is correct.
>
> On 2017-08-06 16:35, Lars George  wrote:
>> Hi,
>>
>> I found this reading through tags in 1.3, but checked in trunk as
>> well. There is this code:
>>
>>   public ArrayBackedTag(byte[] bytes, int offset, int length) {
>> if (length > MAX_TAG_LENGTH) {
>>   throw new IllegalArgumentException(
>>   "Invalid tag data being passed. Its length can not exceed "
>> + MAX_TAG_LENGTH);
>> }
>> this.bytes = bytes;
>> this.offset = offset;
>> this.length = length;
>> this.type = bytes[offset + TAG_LENGTH_SIZE];
>>   }
>>
>> I am concerned about the last line of the code, using the wrong constant?
>>
>>   public final static int TYPE_LENGTH_SIZE = Bytes.SIZEOF_BYTE;
>>   public final static int TAG_LENGTH_SIZE = Bytes.SIZEOF_SHORT;
>>
>> Should this not read
>>
>> this.type = bytes[offset + TYPE_LENGTH_SIZE];
>>
>> Would this not read the type from the wrong place in the array?
>>
>> Cheers,
>> Lars
>>

Re: Tags class using wrong length?

[Chia-Ping Tsai]

According to the following code:
  public ArrayBackedTag(byte tagType, byte[] tag) {
int tagLength = tag.length + TYPE_LENGTH_SIZE;
if (tagLength > MAX_TAG_LENGTH) {
  throw new IllegalArgumentException(
  "Invalid tag data being passed. Its length can not exceed " + 
MAX_TAG_LENGTH);
}
length = TAG_LENGTH_SIZE + tagLength;
bytes = new byte[length];
int pos = Bytes.putAsShort(bytes, 0, tagLength);
pos = Bytes.putByte(bytes, pos, tagType);
Bytes.putBytes(bytes, pos, tag, 0, tag.length);
this.type = tagType;
  }
The layout of the byte array should be:
|tag legnth (2 bytes)|tag type(1 byte)|tag|

It seems to me that the "bytes[offset + TYPE_LENGTH_SIZE]" is correct.

On 2017-08-06 16:35, Lars George  wrote: 
> Hi,
> 
> I found this reading through tags in 1.3, but checked in trunk as
> well. There is this code:
> 
>   public ArrayBackedTag(byte[] bytes, int offset, int length) {
> if (length > MAX_TAG_LENGTH) {
>   throw new IllegalArgumentException(
>   "Invalid tag data being passed. Its length can not exceed "
> + MAX_TAG_LENGTH);
> }
> this.bytes = bytes;
> this.offset = offset;
> this.length = length;
> this.type = bytes[offset + TAG_LENGTH_SIZE];
>   }
> 
> I am concerned about the last line of the code, using the wrong constant?
> 
>   public final static int TYPE_LENGTH_SIZE = Bytes.SIZEOF_BYTE;
>   public final static int TAG_LENGTH_SIZE = Bytes.SIZEOF_SHORT;
> 
> Should this not read
> 
> this.type = bytes[offset + TYPE_LENGTH_SIZE];
> 
> Would this not read the type from the wrong place in the array?
> 
> Cheers,
> Lars
> 

Tags class using wrong length?

[Lars George]

Hi,

I found this reading through tags in 1.3, but checked in trunk as
well. There is this code:

  public ArrayBackedTag(byte[] bytes, int offset, int length) {
if (length > MAX_TAG_LENGTH) {
  throw new IllegalArgumentException(
  "Invalid tag data being passed. Its length can not exceed "
+ MAX_TAG_LENGTH);
}
this.bytes = bytes;
this.offset = offset;
this.length = length;
this.type = bytes[offset + TAG_LENGTH_SIZE];
  }

I am concerned about the last line of the code, using the wrong constant?

  public final static int TYPE_LENGTH_SIZE = Bytes.SIZEOF_BYTE;
  public final static int TAG_LENGTH_SIZE = Bytes.SIZEOF_SHORT;

Should this not read

this.type = bytes[offset + TYPE_LENGTH_SIZE];

Would this not read the type from the wrong place in the array?

Cheers,
Lars