[jira] [Comment Edited] (PHOENIX-952) Support ANY and ALL built-ins for ARRAYs

["James Taylor (JIRA)" <ji...@apache.org>]

    [ 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)