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

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_r243724103
  
    --- Diff: 
phoenix-core/src/main/java/org/apache/phoenix/compile/OrderByCompiler.java ---
    @@ -88,7 +88,8 @@ public static OrderBy compile(StatementContext context,
                                       Integer offset,
                                       RowProjector rowProjector,
                                       TupleProjector tupleProjector,
    -                                  boolean isInRowKeyOrder) throws 
SQLException {
    +                                  boolean isInRowKeyOrder,
    +                                  Expression whereExpression) throws 
SQLException {
    --- End diff --
    
    We need the whereExpression because in 
OrderPreservingTracker.IsConstantVisitor, we need whereExpression to help us to 
check the columnExpression is constant or not. Just take the same sql as 
example:  
    
    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
    
    We need to check if "CASE WHEN coalesce(a.ak1,1) > coalesce(a.av2,2) THEN 
coalesce(a.ak1,1) ELSE coalesce(a.av2,2) END,a.av1" is constant in 
OrderPreservingTracker.hasEqualityConstraints method, so we move forward to 
check if the a.ak1 and a.av2 are constant in  
OrderPreservingTracker.IsConstantVisitor, and we must visit the where clause 
"where a.ak1 = 0.0 and a.av2 = length(substr('abc',1,1))" to check the a.ak1 
and a.av2.
    
    BTW. There is no whereExpression in current code of OrderByCompiler because 
for PHOENIX-3451 which was completed by James and me before, 
RowKeyColumnExpression is only supported to check in 
OrderPreservingTracker.hasEqualityConstraints, and two FIXME tags were left in 
this method at at time:
    
     private boolean hasEqualityConstraints(int startPos, int endPos) {
            ScanRanges ranges = context.getScanRanges();
            // If a GROUP BY is being done, then the rows are ordered according 
to the GROUP BY key,
            // not by the original row key order of the table (see 
PHOENIX-3451).
            // We check each GROUP BY expression to see if it only references 
columns that are
            // matched by equality constraints, in which case the expression 
itself would be constant.
            // FIXME: this only recognizes row key columns that are held 
constant, not all columns.
            // FIXME: we should optimize out any GROUP BY or ORDER BY 
expression which is deemed to
            // be held constant based on the WHERE clause.
            if (!groupBy.isEmpty()) {
    
    In this patch, I added whereExpression to try to fix this problem to 
support non-pk columns also.


> 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