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

    https://github.com/apache/incubator-trafodion/pull/795#discussion_r85605485
  
    --- Diff: core/sql/optimizer/OptPhysRelExpr.cpp ---
    @@ -11814,10 +11814,23 @@ void 
SortGroupBy::addArrangementAndOrderRequirements(
       {
         // Shouldn't/Can't add a sort order type requirement
         // if we are in DP2
    -    if (rg.getStartRequirements()->executeInDP2())
    -      rg.addArrangement(groupExpr(),NO_SOT);
    -    else
    -      rg.addArrangement(groupExpr(),ESP_SOT);
    +
    +   if( NOT extraOrderExpr().isEmpty())
    +   {
    +     ValueIdList groupExprCpy(groupExpr());
    +     for (ValueId vid = extraOrderExpr().init(); 
extraOrderExpr().next(vid);
    --- End diff --
    
    At this point does extraOrderExpr contain (studentname, test_score) or just 
(test_score). This statement uses the query in TEST002 that is part of this PR. 
I thought it would contain only test_score, if yes, then how we skip adding the 
arrangement requirement with GroupExpr? 
    
    Also, is there a check somewhere that if such a clause is used we will 
always come though sort groupby and not hash groupby?
    
    Do we also know that partial groupby rule and shortcut groupby rule will be 
prevented from firing? I assume we cannot get everything to work with partial 
groups or shortcut groupby.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

Reply via email to