[
https://issues.apache.org/jira/browse/PHOENIX-3586?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=15826574#comment-15826574
]
James Taylor edited comment on PHOENIX-3586 at 1/17/17 7:38 PM:
----------------------------------------------------------------
Thanks for the patch, [~tdsilva]. Here's some feedback:
- Can you comment on why the test changes were necessary (I'm sure they are,
but it'd be good to just explain that a bit)?
- Which tests are you running with both encoded and non encoded? What about
tests for different # of bytes for the qualifier? Does Samarth already have
tests for that?
- Should we have a {{none}} option for COLUMN_ENCODED_BYTES as an alias for 0
similar to the way we did for UPDATE_CACHE_FREQUENCY with {{always}} and
{{never}}?
- Do we need to write the storage scheme enum here in read/write, or can we
just include it in the data? Seems like we're writing it twice. Also, what does
WritableUtils.writeEnum do? We've typically just written the ordinal of an enum
in the past.
{code}
@Override
public void readFields(DataInput input) throws IOException {
super.readFields(input);
- this.positionInArray = WritableUtils.readVInt(input);
+ this.decodedColumnQualifier = WritableUtils.readVInt(input);
this.encodingScheme = WritableUtils.readEnum(input,
QualifierEncodingScheme.class);
+ this.immutableStorageScheme = WritableUtils.readEnum(input,
ImmutableStorageScheme.class);
setKeyValueExpression();
}
@Override
public void write(DataOutput output) throws IOException {
super.write(output);
- WritableUtils.writeVInt(output, positionInArray);
+ WritableUtils.writeVInt(output, decodedColumnQualifier);
WritableUtils.writeEnum(output, encodingScheme);
+ WritableUtils.writeEnum(output, immutableStorageScheme);
}
{code}
- Any b/w compat considerations for the above serialization changes?
- More docs for this interface, please:
{code}
+
+ interface ColumnValueEncoderDecoder {
+ void appendValue(byte[] bytes, int offset, int length);
+ // used to represent absence of a column value
+ void appendAbsentValue();
+ byte[] encode();
+ boolean decode(ImmutableBytesWritable ptr, int index);
+ void init(int numElements);
+ }
{code}
- Don't forget to add the add column code for IMMUTABLE_STORAGE_SCHEME for
backward compatibility in ConnectionQueryServicesImpl.
was (Author: jamestaylor):
Thanks for the patch, [~tdsilva]. Here's some feedback:
- Can you comment on why the test changes were necessary (I'm sure they are,
but it'd be good to just explain that a bit)?
- Which tests are you running with both encoded and non encoded? What about
tests for different # of bytes for the qualifier? Does Samarth already have
tests for that?
- Should we have a {{none}} option for COLUMN_ENCODED_BYTES as an alias for 0
similar to the way we did for UPDATE_CACHE_FREQUENCY with {{always}} and
{{never}}?
- Do we need to write the storage scheme enum here in read/write, or can we
just include it in the data? Seems like we're writing it twice. Also, what does
WritableUtils.writeEnum do? We've typically just written the ordinal of an enum
in the past.
{code}
@Override
public void readFields(DataInput input) throws IOException {
super.readFields(input);
- this.positionInArray = WritableUtils.readVInt(input);
+ this.decodedColumnQualifier = WritableUtils.readVInt(input);
this.encodingScheme = WritableUtils.readEnum(input,
QualifierEncodingScheme.class);
+ this.immutableStorageScheme = WritableUtils.readEnum(input,
ImmutableStorageScheme.class);
setKeyValueExpression();
}
@Override
public void write(DataOutput output) throws IOException {
super.write(output);
- WritableUtils.writeVInt(output, positionInArray);
+ WritableUtils.writeVInt(output, decodedColumnQualifier);
WritableUtils.writeEnum(output, encodingScheme);
+ WritableUtils.writeEnum(output, immutableStorageScheme);
}
{code}
- Any b/w compat considerations for the above serialization changes?
- More docs for this interface, please:
{code}
+
+ interface ColumnValueEncoderDecoder {
+ void appendValue(byte[] bytes, int offset, int length);
+ // used to represent absence of a column value
+ void appendAbsentValue();
+ byte[] encode();
+ boolean decode(ImmutableBytesWritable ptr, int index);
+ void init(int numElements);
+ }
{code}
> Add StorageScheme table property to allow users to specify their custom
> storage schemes
> ---------------------------------------------------------------------------------------
>
> Key: PHOENIX-3586
> URL: https://issues.apache.org/jira/browse/PHOENIX-3586
> Project: Phoenix
> Issue Type: Sub-task
> Reporter: Thomas D'Silva
> Assignee: Thomas D'Silva
> Attachments: PHOENIX-3586.patch
>
>
--
This message was sent by Atlassian JIRA
(v6.3.4#6332)
