Fix ORDER BY with projections, when the order aliases are not in the …

…projections

New executor

core/src/main/grammar/OrientSQL.jjt

@@ -2972,7 +2972,7 @@ OOrderBy OrderBy():
                 }
                 (
                     (
-                        lastIdentifier = Identifier() { lastItem.alias = lastIdentifier.toString(); }
+                        lastIdentifier = Identifier() { lastItem.alias = lastIdentifier.getStringValue(); }
                         [ lastModifier = Modifier()  { lastItem.modifier = lastModifier; } ]
                     )
                     |
@@ -2993,7 +2993,7 @@ OOrderBy OrderBy():
                 }
                 (
                     (
-                        lastIdentifier = Identifier() { lastItem.alias = lastIdentifier.toString(); }
+                        lastIdentifier = Identifier() { lastItem.alias = lastIdentifier.getStringValue(); }
                         [ lastModifier = Modifier()  { lastItem.modifier = lastModifier; } ]
                     )
                     |

core/src/main/java/com/orientechnologies/orient/core/sql/executor/OSelectExecutionPlanner.java

@@ -26,7 +26,8 @@ public class OSelectExecutionPlanner {
 
   private OProjection preAggregateProjection;
   private OProjection aggregateProjection;
-  private OProjection projection = null;
+  private OProjection projection             = null;
+  private OProjection projectionAfterOrderBy = null;
 
   private OLetClause globalLetClause    = null;
   private OLetClause perRecordLetClause = null;
@@ -168,7 +169,7 @@ private void optimizeQuery() {
       flattenedWhereClause = moveFlattededEqualitiesLeft(flattenedWhereClause);
     }
 
-    extractAggregateProjections();
+    splitProjectionsForGroupBy();
     addOrderByProjections();
   }
 
@@ -227,15 +228,81 @@ private List<OAndBlock> moveFlattededEqualitiesLeft(List<OAndBlock> flattenedWhe
     return result;
   }
 
+  /**
+   * creates additional projections for ORDER BY
+   */
   private void addOrderByProjections() {
-    if (orderApplied || orderBy == null || orderBy.getItems().size() == 0 || projection == null || projection.getItems() == null
-        || (projection.getItems().size() == 1 && projection.getItems().get(0).isAll())) {
+    if (orderApplied || expand || unwind != null || orderBy == null || orderBy.getItems().size() == 0 || projection == null
+        || projection.getItems() == null || (projection.getItems().size() == 1 && projection.getItems().get(0).isAll())) {
       return;
     }
-    //TODO
+
+    OOrderBy newOrderBy = orderBy == null ? null : orderBy.copy();
+    List<OProjectionItem> additionalOrderByProjections = calculateAdditionalOrderByProjections(this.projection.getAllAliases(),
+        newOrderBy);
+    if (additionalOrderByProjections.size() > 0) {
+      orderBy = newOrderBy;//the ORDER BY has changed
+    }
+    if (additionalOrderByProjections.size() > 0) {
+      projectionAfterOrderBy = new OProjection(-1);
+      projectionAfterOrderBy.setItems(new ArrayList<>());
+      for (String alias : projection.getAllAliases()) {
+        projectionAfterOrderBy.getItems().add(projectionFromAlias(new OIdentifier(alias)));
+      }
+
+      for (OProjectionItem item : additionalOrderByProjections) {
+        if (preAggregateProjection != null) {
+          preAggregateProjection.getItems().add(item);
+          aggregateProjection.getItems().add(projectionFromAlias(item.getAlias()));
+          projection.getItems().add(projectionFromAlias(item.getAlias()));
+        } else {
+          projection.getItems().add(item);
+        }
+      }
+    }
   }
 
-  private void extractAggregateProjections() {
+  /**
+   * given a list of aliases (present in the existing projections) calculates a list of additional projections to
+   * add to the existing projections to allow ORDER BY calculation.
+   * The sorting clause will be modified with new replaced aliases
+   *
+   * @param allAliases existing aliases in the projection
+   * @param orderBy    sorting clause
+   * @return a list of additional projections to add to the existing projections to allow ORDER BY calculation (empty if nothing has to be added).
+   */
+  private List<OProjectionItem> calculateAdditionalOrderByProjections(Set<String> allAliases, OOrderBy orderBy) {
+    List<OProjectionItem> result = new ArrayList<>();
+    int nextAliasCount = 0;
+    if (orderBy != null && orderBy.getItems() != null || !orderBy.getItems().isEmpty()) {
+      for (OOrderByItem item : orderBy.getItems()) {
+        if (!allAliases.contains(item.getAlias())) {
+          OProjectionItem newProj = new OProjectionItem(-1);
+          if (item.getAlias() != null) {
+            newProj.setExpression(new OExpression(new OIdentifier(item.getAlias()), item.getModifier()));
+          } else if (item.getRecordAttr() != null) {
+            ORecordAttribute attr = new ORecordAttribute(-1);
+            attr.setName(item.getRecordAttr());
+            newProj.setExpression(new OExpression(attr, item.getModifier()));
+          } else if (item.getRid() != null) {
+            OExpression exp = new OExpression(-1);
+            exp.setRid(item.getRid().copy());
+            newProj.setExpression(exp);
+          }
+          OIdentifier newAlias = new OIdentifier("_$$$ORDER_BY_ALIAS$$$_" + (nextAliasCount++));
+          newProj.setAlias(newAlias);
+          item.setAlias(newAlias.getStringValue());
+          result.add(newProj);
+        }
+      }
+    }
+    return result;
+  }
+
+  /**
+   * splits projections in three parts (pre-aggregate, aggregate and final) to efficiently manage aggregations
+   */
+  private void splitProjectionsForGroupBy() {
     if (projection == null) {
       return;
     }
@@ -247,12 +314,14 @@ private void extractAggregateProjections() {
     OProjection postAggregate = new OProjection(-1);
     postAggregate.setItems(new ArrayList<>());
 
-    boolean isAggregate = false;
+    boolean isSplitted = false;
+
+    //split for aggregate projections
     AggregateProjectionSplit result = new AggregateProjectionSplit();
     for (OProjectionItem item : this.projection.getItems()) {
       result.reset();
       if (item.isAggregate()) {
-        isAggregate = true;
+        isSplitted = true;
         OProjectionItem post = item.splitForAggregation(result);
         OIdentifier postAlias = item.getProjectionAlias();
         postAlias.setQuoted(true);
@@ -269,7 +338,9 @@ private void extractAggregateProjections() {
         postAggregate.getItems().add(aggItem);
       }
     }
-    if (isAggregate) {
+
+    //bind split projections to the execution planner
+    if (isSplitted) {
       this.preAggregateProjection = preAggregate;
       if (preAggregateProjection.getItems() == null || preAggregateProjection.getItems().size() == 0) {
         preAggregateProjection = null;
@@ -284,6 +355,12 @@ private void extractAggregateProjections() {
     }
   }
 
+  private OProjectionItem projectionFromAlias(OIdentifier oIdentifier) {
+    OProjectionItem result = new OProjectionItem(-1);
+    result.setExpression(new OExpression(oIdentifier));
+    return result;
+  }
+
   /**
    * if GROUP BY is performed on an expression that is not explicitly in the pre-aggregate projections, then
    * that expression has to be put in the pre-aggregate (only here, in subsequent steps it's removed)
@@ -569,6 +646,9 @@ private void handleOrderBy(OSelectExecutionPlan plan, OOrderBy orderBy, OCommand
     }
     if (!orderApplied && orderBy != null && orderBy.getItems() != null && orderBy.getItems().size() > 0) {
       plan.chain(new OrderByStep(orderBy, maxResults, ctx));
+      if (projectionAfterOrderBy != null) {
+        plan.chain(new ProjectionCalculationStep(projectionAfterOrderBy, ctx));
+      }
     }
   }
 

core/src/main/java/com/orientechnologies/orient/core/sql/parser/OBaseExpression.java

@@ -40,6 +40,21 @@ public OBaseExpression(OIdentifier identifier) {
     this.identifier = new OBaseIdentifier(identifier);
   }
 
+  public OBaseExpression(OIdentifier identifier, OModifier modifier) {
+    this(identifier);
+    if (modifier != null) {
+      this.modifier = modifier;
+    }
+  }
+
+  public OBaseExpression(ORecordAttribute attr, OModifier modifier) {
+    super(-1);
+    this.identifier = new OBaseIdentifier(attr);
+    if(modifier!=null){
+      this.modifier = modifier;
+    }
+  }
+
   /**
    * Accept the visitor.
    **/
@@ -147,14 +162,15 @@ public boolean canExecuteIndexedFunctionWithoutIndex(OFromClause target, OComman
 
   /**
    * tests if current expression is an indexed function AND that function can be used on this target
-   * @param target the query target
-   * @param context the execution context
+   *
+   * @param target   the query target
+   * @param context  the execution context
    * @param operator
    * @param right
    * @return true if current expression is an indexed function AND that function can be used on this target, false otherwise
    */
-  public boolean allowsIndexedFunctionExecutionOnTarget(OFromClause target, OCommandContext context, OBinaryCompareOperator operator,
-      Object right){
+  public boolean allowsIndexedFunctionExecutionOnTarget(OFromClause target, OCommandContext context,
+      OBinaryCompareOperator operator, Object right) {
     if (this.identifier == null) {
       return false;
     }
@@ -165,12 +181,13 @@ public boolean allowsIndexedFunctionExecutionOnTarget(OFromClause target, OComma
    * tests if current expression is an indexed function AND the function has also to be executed after the index search.
    * In some cases, the index search is accurate, so this condition can be excluded from further evaluation. In other cases
    * the result from the index is a superset of the expected result, so the function has to be executed anyway for further filtering
-   * @param target the query target
+   *
+   * @param target  the query target
    * @param context the execution context
    * @return true if current expression is an indexed function AND the function has also to be executed after the index search.
    */
-  public boolean executeIndexedFunctionAfterIndexSearch(OFromClause target, OCommandContext context, OBinaryCompareOperator operator,
-      Object right){
+  public boolean executeIndexedFunctionAfterIndexSearch(OFromClause target, OCommandContext context,
+      OBinaryCompareOperator operator, Object right) {
     if (this.identifier == null) {
       return false;
     }
@@ -252,10 +269,10 @@ public AggregationContext getAggregationContext(OCommandContext ctx) {
   }
 
   public boolean refersToParent() {
-    if(identifier!=null && identifier.refersToParent()){
+    if (identifier != null && identifier.refersToParent()) {
       return true;
     }
-    if(modifier!=null && modifier.refersToParent()){
+    if (modifier != null && modifier.refersToParent()) {
       return true;
     }
     return false;

core/src/main/java/com/orientechnologies/orient/core/sql/parser/OBaseIdentifier.java

@@ -29,6 +29,10 @@ public OBaseIdentifier(OIdentifier identifier) {
     this.suffix = new OSuffixIdentifier(identifier);
   }
 
+  public OBaseIdentifier(ORecordAttribute attr) {
+    this.suffix = new OSuffixIdentifier(attr);
+  }
+
   /**
    * Accept the visitor.
    **/

core/src/main/java/com/orientechnologies/orient/core/sql/parser/OExpression.java

@@ -36,6 +36,15 @@ public OExpression(OIdentifier identifier) {
     mathExpression = new OBaseExpression(identifier);
   }
 
+  public OExpression(OIdentifier identifier, OModifier modifier) {
+
+    mathExpression = new OBaseExpression(identifier, modifier);
+  }
+
+  public OExpression(ORecordAttribute attr, OModifier modifier) {
+    mathExpression = new OBaseExpression(attr, modifier);
+  }
+
   /**
    * Accept the visitor.
    **/
@@ -442,6 +451,12 @@ public boolean refersToParent() {
     return false;
   }
 
+  public ORid getRid() {
+    return rid;
+  }
 
+  public void setRid(ORid rid) {
+    this.rid = rid;
+  }
 }
 /* JavaCC - OriginalChecksum=9c860224b121acdc89522ae97010be01 (do not edit this line) */

core/src/main/java/com/orientechnologies/orient/core/sql/parser/OOrderByItem.java

@@ -133,4 +133,8 @@ public boolean refersToParent() {
     }
     return false;
   }
+
+  public OModifier getModifier() {
+    return modifier;
+  }
 }

core/src/main/java/com/orientechnologies/orient/core/sql/parser/ORecordAttribute.java

@@ -50,5 +50,13 @@ public ORecordAttribute copy() {
   @Override public int hashCode() {
     return name != null ? name.hashCode() : 0;
   }
+
+  public String getName() {
+    return name;
+  }
+
+  public void setName(String name) {
+    this.name = name;
+  }
 }
 /* JavaCC - OriginalChecksum=45ce3cd16399dec7d7ef89f8920d02ae (do not edit this line) */

core/src/main/java/com/orientechnologies/orient/core/sql/parser/OSuffixIdentifier.java

@@ -29,6 +29,10 @@ public OSuffixIdentifier(OIdentifier identifier) {
     this.identifier = identifier;
   }
 
+  public OSuffixIdentifier(ORecordAttribute attr) {
+    this.recordAttribute = attr;
+  }
+
   /**
    * Accept the visitor.
    **/

core/src/main/java/com/orientechnologies/orient/core/sql/parser/OrientSql.java

@@ -9480,7 +9480,7 @@ final public OOrderBy OrderBy() throws ParseException {
         case IDENTIFIER:
         case QUOTED_IDENTIFIER:
           lastIdentifier = Identifier();
-                                                        lastItem.alias = lastIdentifier.toString();
+                                                        lastItem.alias = lastIdentifier.getStringValue();
           switch ((jj_ntk==-1)?jj_ntk():jj_ntk) {
           case LBRACKET:
           case DOT:
@@ -9622,7 +9622,7 @@ final public OOrderBy OrderBy() throws ParseException {
         case IDENTIFIER:
         case QUOTED_IDENTIFIER:
           lastIdentifier = Identifier();
-                                                        lastItem.alias = lastIdentifier.toString();
+                                                        lastItem.alias = lastIdentifier.getStringValue();
           switch ((jj_ntk==-1)?jj_ntk():jj_ntk) {
           case LBRACKET:
           case DOT: