[
https://issues.apache.org/jira/browse/PHOENIX-2160?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=14715911#comment-14715911
]
James Taylor commented on PHOENIX-2160:
---------------------------------------
Thanks for the patch, [~Dumindux]. I think we should kind of reverse the logic
for this optimization in SelectClauseVisitor. Instead of optimizing array
element references and then removing them if the array is referenced, let's
leave the original ArrayIndexFunction in place, but do the reference counting
you're doing now so that we know after the main loop in compile() whether or
not we can do the optimization. That way we won't need to keep the old and new
Expression as you've done in ArrayIndexExpression, but instead we'll generate
the new expression after the main compile loop. I think the only thing we'll
need in the SelectClauseVisitor is your Map<ArrayIndexFunction, Integer>
structure.
So after the loop in ProjectionCompiler.compile(), we'll have the following
logic:
- Walk through the Map and remove any entry with a reference count > 0. The
expressions remaining are the ones we can optimize.
- In this same loop, generate arrayKVFuncs and arrayKVFuncs.
- After this loop, call the function to push the info into the scan and
generate the arrayIndexesBitSet and arrayIndexesSchema (but make them local
variables as they're currently static which is a big no-no).
{code}
if(arrayKVFuncs.size() > 0 && arrayKVRefs.size() > 0) {
serailizeArrayIndexInformationAndSetInScan(context,
arrayKVFuncs, arrayKVRefs);
KeyValueSchemaBuilder builder = new KeyValueSchemaBuilder(0);
for (Expression expression : arrayKVRefs) {
builder.addField(expression);
}
KeyValueSchema kvSchema = builder.build();
ValueBitSet arrayIndexesBitSet =
ValueBitSet.newInstance(kvSchema);
builder = new KeyValueSchemaBuilder(0);
for (Expression expression : arrayKVFuncs) {
builder.addField(expression);
}
KeyValueSchema arrayIndexesSchema = builder.build();
}
{code}
- At the bottom of this if statement, add in code to create the new
ArrayIndexExpression, passing through kvSchema and arrayIndexesSchema in the
constructor. You'll want to track the old -> new mapping in a new
Map<ArrayFunctionExpression, ArrayIndexExpression> (see below on why).
- Use a visitor to walk the tree for each projectedColumns expression and
replaces any references to the the old ArrayFunctionExpression with the new
ArrayIndexExpression. The easiest way to do this is to refactor
CloneExpressionVisitor slightly by making an abstract method like
isCloneNode(Expression node, List<Expression> children). This will replace the
Determinism.PER_INVOCATION.compareTo(node.getDeterminism()) > 0 checks. Then
create a new concrete subclass called CloneNonDeterministicExpressionVisitor
that implements isCloneNode() as return
Determinism.PER_INVOCATION.compareTo(node.getDeterminism()) > 0. Use this new
subclass in place of existing usages of CloneExpressionVisitor. Then, you'll
create your own new concrete class called ReplaceArrayFunctionExpressionVisitor
which will implement isCloneNode() something like the following (which will
clone a tree with children that have changed or been replaced).
{code}
return !children.equals(node.getChildren());
{code}
You'll also override visitLeave(ScalarFunction) as that's where you'll want to
check your replacement map (which you'll pass through in a constructor):
{code}
Expression replacement = replacementMap.containsKey(node);
if (replacement != null) {
return replacement;
}
return super.visitLeave(node,children);
{code}
Then you'll use this by instantiating a new
ReplaceArrayFunctionExpressionVisitor(replacementMap) and doing a loop to
replace each projectedColumns element with the return value of
projectedColumn.accept(visitor).
One more minor item - I think you need to check that
expression.getDataType().isArrayType() is true before incrementing your
reference count here. Maybe add a reference to a non array column in your tests
to improve coverage a bit.
{code}
+ @Override
+ public Expression visit(ColumnParseNode node) throws SQLException {
+ Expression expression = super.visit(node);
+ arrayExpressionCounts.put(expression,
arrayExpressionCounts.containsKey(expression) ?
(arrayExpressionCounts.get(expression) + 1) : 1);
+ return expression;
+ }
{code}
> Projection of specific array index does not work
> ------------------------------------------------
>
> Key: PHOENIX-2160
> URL: https://issues.apache.org/jira/browse/PHOENIX-2160
> Project: Phoenix
> Issue Type: Bug
> Reporter: ramkrishna.s.vasudevan
> Assignee: ramkrishna.s.vasudevan
> Attachments: PHOENIX-2160.patch, PHOENIX-2160_v2.patch,
> PHOENIX-2160_v3.patch, PHOENIX-2160_v4.patch, PHOENIX-2160_v5.patch
>
>
> PHOENIX-10 that allowed projection of specific array index does not work now.
> Was looking into the code for some thing and found this issue. Let me know if
> am missing something.
--
This message was sent by Atlassian JIRA
(v6.3.4#6332)
