[
https://issues.apache.org/jira/browse/PHOENIX-952?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=14010014#comment-14010014
]
James Taylor edited comment on PHOENIX-952 at 5/27/14 10:37 PM:
----------------------------------------------------------------
Here's the refactoring I was talking about for the boolean_expression rule:
{code}
comparison_op returns [CompareOp ret]
: EQ { $ret = CompareOp.EQUAL; }
| LT { $ret = CompareOp.LESS; }
| GT { $ret = CompareOp.GREATER; }
| LT EQ { $ret = CompareOp.LESS_OR_EQUAL; }
| GT EQ { $ret = CompareOp.GREATER_OR_EQUAL; }
| (NOEQ1 | NOEQ2) { $ret = CompareOp.NOT_EQUAL; }
;
boolean_expression returns [ParseNode ret]
: l=value_expression ((op=comparison_op (r=value_expression | ((all=ALL |
any=ANY) LPAREN r=value_expression RPAREN)) {$ret = all != null ? wrapInAll(op,
l, r) : any != null ? wrapInAny(op, l, r) : factory.comparison(op,l,r); } )
| (IS n=NOT? NULL {$ret = factory.isNull(l,n!=null); } )
| ( n=NOT? ((LIKE r=value_expression {$ret =
factory.like(l,r,n!=null); } )
| (EXISTS LPAREN r=subquery_expression RPAREN
{$ret = factory.exists(l,r,n!=null);} )
| (BETWEEN r1=value_expression AND
r2=value_expression {$ret = factory.between(l,r1,r2,n!=null); } )
| ((IN ((r=bind_expression {$ret =
factory.inList(Arrays.asList(l,r),n!=null);} )
| (LPAREN r=subquery_expression RPAREN {$ret =
factory.in(l,r,n!=null);} )
| (LPAREN v=one_or_more_expressions RPAREN
{List<ParseNode> il = new ArrayList<ParseNode>(v.size() + 1); il.add(l);
il.addAll(v); $ret = factory.inList(il,n!=null);})
)))
))
| { $ret = l; } )
;
{code}
So you don't need to change the method signatures for factory.equal,
factory.gt, etc. Instead, get the CompareOp through the new rule and use
factory.comparison(op,lhs,rhs). In the case that ANY or ALL are present, just
implement those functions in @parser::members section at the beginning of the
PhoenixSQL.g file. The wrapInAny method would instantiate a parse node
hierarchy like this:
{code}
ArrayAnyParseNode(rhs /*array*/, ComparisonParseNode(lhs, ArrayElemRef(rhs,1)))
{code}
Where the ArrayElemRef parse node is just a placeholder for a reference to an
element from the array and would be created through a call like this:
{code}
factory.arrayElemRef(Arrays.<ParseNode>asList(rhs/*array*/,factory.literal(1));
{code}
The ArrayElemRef parse node will get replaced by ExpressionCompiler at compile
time (during the new visitLeave(ArrayAnyParseNode) call) with an inline
expression created and controlled by the outer ArrayAnyExpression that simply
loops through the array elements. By creating it like this, though, you'll get
the type checking you want, as the compilation will check that the rhs and an
element of the array are of comparable types. This way, you're not having to
duplicate any ComparisonExpression logic. Instead, the ANY and ALL expression
wraps the ComparisonExpression and is just a looping construct. IMHO, this is
the cleanest way to implement this.
was (Author: jamestaylor):
Here's the refactoring I was talking about for the boolean_expression rule:
{code}
comparison_op returns [CompareOp ret]
: EQ { $ret = CompareOp.EQUAL; }
| LT { $ret = CompareOp.LESS; }
| GT { $ret = CompareOp.GREATER; }
| LT EQ { $ret = CompareOp.LESS_OR_EQUAL; }
| GT EQ { $ret = CompareOp.GREATER_OR_EQUAL; }
| (NOEQ1 | NOEQ2) { $ret = CompareOp.NOT_EQUAL; }
;
boolean_expression returns [ParseNode ret]
: l=value_expression ((op=comparison_op (r=value_expression | ((all=ALL |
any=ANY) LPAREN r=value_expression RPAREN)) {$ret = all != null ? wrapInAll(op,
l, r) : any != null ? wrapInAny(op, l, r) : factory.comparison(op,l,r); } )
| (IS n=NOT? NULL {$ret = factory.isNull(l,n!=null); } )
| ( n=NOT? ((LIKE r=value_expression {$ret =
factory.like(l,r,n!=null); } )
| (EXISTS LPAREN r=subquery_expression RPAREN
{$ret = factory.exists(l,r,n!=null);} )
| (BETWEEN r1=value_expression AND
r2=value_expression {$ret = factory.between(l,r1,r2,n!=null); } )
| ((IN ((r=bind_expression {$ret =
factory.inList(Arrays.asList(l,r),n!=null);} )
| (LPAREN r=subquery_expression RPAREN {$ret =
factory.in(l,r,n!=null);} )
| (LPAREN v=one_or_more_expressions RPAREN
{List<ParseNode> il = new ArrayList<ParseNode>(v.size() + 1); il.add(l);
il.addAll(v); $ret = factory.inList(il,n!=null);})
)))
))
| { $ret = l; } )
;
{code}
So you don't need to change the method signatures for factory.equal,
factory.gt, etc. Instead, get the CompareOp through the new rule and use
factory.comparison(op,lhs,rhs) instead. In the case that ANY or ALL are
present, just implement those functions in @parser::members section at the
beginning of the PhoenixSQL.g file. The wrapInAny method would instantiate a
parse node hierarchy like this:
{code}
ArrayAnyParseNode(rhs /*array*/, ComparisonParseNode(ArrayElemRef, lhs))
{code}
Where the ArrayElemRef parse node is just a placeholder and would be created
through a call like this:
{code}
factory.arrayElemRef(Arrays.<ParseNode>asList(rhs/*array*/,factory.literal(1));
{code}
The ArrayElemRef parse node will get replaced by ExpressionCompiler at compile
time with an inline expression created and controlled by the outer
ArrayAnyExpression that simply loops through the array elements. This way,
you're not having to duplicate any ComparisonExpression logic. Instead, the ANY
and ALL expression wraps the ComparisonExpression and is just a looping
construct. IMHO, this is the cleanest way to implement this.
> Support ANY and ALL built-ins for ARRAYs
> ----------------------------------------
>
> Key: PHOENIX-952
> URL: https://issues.apache.org/jira/browse/PHOENIX-952
> Project: Phoenix
> Issue Type: Bug
> Affects Versions: 3.0.0, 4.0.0
> Reporter: James Taylor
> Assignee: ramkrishna.s.vasudevan
> Fix For: 3.1, 4.1
>
> Attachments: Phoenix-932_1.patch, Phoenix-932_2.patch,
> Phoenix-952_4.patch
>
>
> There's currently no good way to search array elements. We should support the
> ANY and ALL built-ins for our ARRAY type like Postgres does:
> http://www.postgresql.org/docs/9.1/static/arrays.html#ARRAYS-SEARCHING
--
This message was sent by Atlassian JIRA
(v6.2#6252)
