[
https://issues.apache.org/jira/browse/PHOENIX-3295?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=15560604#comment-15560604
]
James Taylor commented on PHOENIX-3295:
---------------------------------------
Thanks, [~tdsilva]. Here's some feedback:
- Is the column qualifier name of the single key value the family name? Does
that create any issues for [~samarthjain]'s column encoding scheme and would it
be easier to use a known/static column qualifier name like
QueryConstants.EMPTY_COLUMN_BYTES? More of a question, as I think Samarth can
probably change if need be.
{code}
public ArrayColumnExpression(PDatum column, byte[] cf, int index) {
- super(column);
+ super(column, cf, cf);
{code}
- Is this TODO still relevant?
{code}
+ // TODO: samarth think about the case when the encodedcolumn qualifier
is null. Probably best to add a check here for encodedcolumnname to be true
{code}
- Do we still need this method in ArrayColumnExpression? I can't find a call to
it in your patch. Would be good if this wasn't needed (and if it is need, then
do we need to create a new instance on each call?).
{code}
public KeyValueColumnExpression getKeyValueExpression() {
{code}
- Can we use ImmutableBytesPtr as the key to this Map instead of ByteBuffer?
{code}
+ Map<ByteBuffer, List<Pair<ColumnReference, ColumnReference>>>
familyToColListMap = Maps.newHashMap();
{code}
- Would it be possible for PArrayDataType.positionAtArrayElement() to
differentiate between the absence of a column value and a null value? We'll
need something like that when support for DEFAULT comes in (PHOENIX-476). Maybe
something like serializing a -1 value for the length versus a 0 and having
PArrayDataType.positionAtArrayElement return true or false which would be the
return value of evaluate? Another alternative would be to serialize the default
value for immutable tables (but that'd be kind of wasteful space-wise if the
value is large). For now, please add a TODO and a JIRA too so we don't forget.
{code}
@Override
public boolean evaluate(Tuple tuple, ImmutableBytesWritable ptr) {
- return PArrayDataType.positionAtArrayElement(tuple, ptr, index,
arrayExpression, PVarbinary.INSTANCE, null);
- }
+ if (!super.evaluate(tuple, ptr)) {
+ return false;
+ } else if (ptr.getLength() == 0) {
+ return true;
+ }
- @Override
- public <T> T accept(ExpressionVisitor<T> visitor) {
- return visitor.visit(this);
+ // Given a ptr to the entire array, set ptr to point to a particular
element within that array
+ // given the type of an array element (see comments in
PDataTypeForArray)
+ PArrayDataType.positionAtArrayElement(ptr, index, PVarbinary.INSTANCE,
null);
+ return true;
}
{code}
> Remove ReplaceArrayColumnWithKeyValueColumnExpressionVisitor
> -------------------------------------------------------------
>
> Key: PHOENIX-3295
> URL: https://issues.apache.org/jira/browse/PHOENIX-3295
> Project: Phoenix
> Issue Type: Improvement
> Reporter: Thomas D'Silva
> Assignee: Thomas D'Silva
> Attachments: PHOENIX-3295-v2.patch, PHOENIX-3295.patch
>
>
> ReplaceArrayColumnWithKeyValueColumnExpressionVisitor is only used in one
> place in IndexUtil.generateIndexData because we use a ValueGetter to get the
> value of the data table column using the original data table column
> reference. This is also why ArrayColumnExpression needs to keep track of the
> original key value column expression.
> If we don't replace the array column expression with the original column
> expression when it looks up the column by the qualifier it won't find it.
> {code}
> ValueGetter valueGetter = new ValueGetter() {
>
> @Override
> public byte[] getRowKey() {
> return dataMutation.getRow();
> }
>
> @Override
> public ImmutableBytesWritable
> getLatestValue(ColumnReference ref) {
> // Always return null for our empty key value, as
> this will cause the index
> // maintainer to always treat this Put as a new
> row.
> if (isEmptyKeyValue(table, ref)) {
> return null;
> }
> byte[] family = ref.getFamily();
> byte[] qualifier = ref.getQualifier();
> RowMutationState rowMutationState =
> valuesMap.get(ptr);
> PColumn column = null;
> try {
> column =
> table.getColumnFamily(family).getPColumnForColumnQualifier(qualifier);
> } catch (ColumnNotFoundException e) {
> } catch (ColumnFamilyNotFoundException e) {
> }
> if (rowMutationState!=null && column!=null) {
> byte[] value =
> rowMutationState.getColumnValues().get(column);
> ImmutableBytesPtr ptr = new
> ImmutableBytesPtr();
> ptr.set(value==null ?
> ByteUtil.EMPTY_BYTE_ARRAY : value);
>
> SchemaUtil.padData(table.getName().getString(), column, ptr);
> return ptr;
> }
> return null;
> }
>
> };
> {code}
--
This message was sent by Atlassian JIRA
(v6.3.4#6332)
