[ 
https://issues.apache.org/jira/browse/PHOENIX-4820?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16727184#comment-16727184
 ] 

ASF GitHub Bot commented on PHOENIX-4820:
-----------------------------------------

Github user comnetwork commented on a diff in the pull request:

    https://github.com/apache/phoenix/pull/417#discussion_r243723430
  
    --- Diff: 
phoenix-core/src/main/java/org/apache/phoenix/expression/Expression.java ---
    @@ -88,4 +88,10 @@
          * @return
          */
         boolean requiresFinalEvaluation();
    +
    --- End diff --
    
    ExpressionUtil.isConstant(Expression) is not suitable for 
OrderPreservingTracker.IsConstantVisitor, because 
ExpressionUtil.isConstant(Expression) depends on Expression.isStateless() and 
Expression.getDeterminism(). Expression isStateless() is to check if the 
expression is depend on the server state, even for RowKeyColumnExpression , 
isStateless() is false. What we want to check is if a expression is constant 
when all children of it are constants, just consider following sql:
    
    select a.ak3 from
    (select rand() ak1,length(pk2) ak2,length(pk3) ak3,length(v1) 
av1,length(v2) av2 from test order by pk2,pk3 limit 10) a
    where a.ak1 = 0.0 and a.av2 = length(substr('abc',1,1))
    group by a.ak3,CASE WHEN coalesce(a.ak1,1) > coalesce(a.av2,2) THEN 
coalesce(a.ak1,1) ELSE coalesce(a.av2,2) END,a.av1
    order by a.ak3,a.av1
    
    Obviously , because of rand(), the Determinism of expression a.ak1 is 
Determinism.PER_INVOCATION, so for expression "CASE WHEN coalesce(a.ak1,1) > 
coalesce(a.av2,2) THEN coalesce(a.ak1,1) ELSE coalesce(a.av2,2) END", the 
determinism is Determinism.PER_INVOCATION and isStateless is false , but 
because the a.ak1 and a.av2 are both constants in where clause of outer query, 
we can regard "CASE WHEN coalesce(a.ak1,1) > coalesce(a.av2,2) THEN 
coalesce(a.ak1,1) ELSE coalesce(a.av2,2) END" as constant in IsConstantVisitor.
    
    BTW. The value returned by ExpressionUtil.isConstant() is complicated and 
runtime related because of Expression.getDeterminism(). In following 
BaseCompoundExpression.init method, determinism is the combine of its children.
    ` private void init(List<Expression> children) {
            this.children = ImmutableList.copyOf(children);
            boolean isStateless = true;
            boolean isNullable = false;
            boolean requiresFinalEvaluation = false;
            this.determinism = Determinism.ALWAYS;
            for (int i = 0; i < children.size(); i++) {
                Expression child = children.get(i);
                isNullable |= child.isNullable();
                isStateless &= child.isStateless();
                this.determinism = 
this.determinism.combine(child.getDeterminism());
                requiresFinalEvaluation |= child.requiresFinalEvaluation();
            }
            this.isStateless = isStateless;
            this.isNullable = isNullable;
            this.requiresFinalEvaluation = requiresFinalEvaluation;
        }`
     


> Optimize OrderBy for ClientAggregatePlan
> ----------------------------------------
>
>                 Key: PHOENIX-4820
>                 URL: https://issues.apache.org/jira/browse/PHOENIX-4820
>             Project: Phoenix
>          Issue Type: Improvement
>    Affects Versions: 4.14.0
>            Reporter: chenglei
>            Assignee: chenglei
>            Priority: Major
>             Fix For: 4.15.0
>
>         Attachments: PHOENIX-4820-4.x-HBase-1.3.patch
>
>
> Given a table
> {code}
>   create table test ( 
>        pk1 varchar not null , 
>        pk2 varchar not null, 
>        pk3 varchar not null,
>         v1 varchar, 
>         v2 varchar, 
>         CONSTRAINT TEST_PK PRIMARY KEY ( 
>               pk1,
>               pk2,
>               pk3 ))
> {code}
> for following sql :
> {code}
>      select a.ak3 
>      from (select substr(pk1,1,1) ak1,substr(pk2,1,1) ak2,substr(pk3,1,1) 
> ak3,substr(v1,1,1) av1,substr(v2,1,1) av2 from test order by pk2,pk3 limit 
> 10) a  group by a.ak3,a.av1 order by a.ak3,a.av1
> {code}
> Intuitively, the above OrderBy statement {{order by a.ak3,a.av1}} should be 
> compiled out because it match the group by statement, but in fact it is not.
> The problem is caused by the {{QueryCompiler.compileSingleQuery}} and 
> {{QueryCompiler.compileSingleFlatQuery}},for  
> {{QueryCompiler.compileSingleQuery}} method,because the inner query has order 
> by, so in line 520, local variable {{isInRowKeyOrder}} is false:
> {code}
> 519        context.setCurrentTable(tableRef);
> 520        boolean isInRowKeyOrder = innerPlan.getGroupBy() == 
> GroupBy.EMPTY_GROUP_BY && innerPlan.getOrderBy() == OrderBy.EMPTY_ORDER_BY;
> {code}
> In {{QueryCompiler.compileSingleFlatQuery}},when {{OrderByCompiler.compile}} 
> method is invoked, the last parameter {{isInRowKeyOrder}} is false:
> {code}
> 562        OrderBy orderBy = OrderByCompiler.compile(context, select, 
> groupBy, limit, offset, projector,
> 563                groupBy == GroupBy.EMPTY_GROUP_BY ? 
> innerPlanTupleProjector : null, isInRowKeyOrder);
> {code}
> So in following line 156 for  {{OrderByCompiler.compile}},even though the 
> {{tracker.isOrderPreserving}} is true, the OrderBy  statement could not be 
> compiled out.
> {code}
> 156       if (isInRowKeyOrder && tracker.isOrderPreserving()) {
> {code}      
> In my opinion, with GroupBy, in following line 563 for 
> {{QueryCompiler.compileSingleFlatQuery}} method, when we call 
> {{OrderByCompiler.compile}} method, we no need to conside the 
> {{isInRowKeyOrder}},  just like the previous parameter  {{tupleProjector}} 
> does.
> {code}
>  562        OrderBy orderBy = OrderByCompiler.compile(context, select, 
> groupBy, limit, offset, projector,
>  563                groupBy == GroupBy.EMPTY_GROUP_BY ? 
> innerPlanTupleProjector : null, isInRowKeyOrder);
> {code}



--
This message was sent by Atlassian JIRA
(v7.6.3#76005)

Reply via email to