diff --git a/core/src/main/grammar/OrientSQL.jjt b/core/src/main/grammar/OrientSQL.jjt index fb72f41cd49..b20f77b92f1 100644 --- a/core/src/main/grammar/OrientSQL.jjt +++ b/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; } ] ) | diff --git a/core/src/main/java/com/orientechnologies/orient/core/sql/executor/OSelectExecutionPlanner.java b/core/src/main/java/com/orientechnologies/orient/core/sql/executor/OSelectExecutionPlanner.java index 216ba378046..db19247852b 100644 --- a/core/src/main/java/com/orientechnologies/orient/core/sql/executor/OSelectExecutionPlanner.java +++ b/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 moveFlattededEqualitiesLeft(List 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 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 calculateAdditionalOrderByProjections(Set allAliases, OOrderBy orderBy) { + List 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)); + } } } diff --git a/core/src/main/java/com/orientechnologies/orient/core/sql/parser/OBaseExpression.java b/core/src/main/java/com/orientechnologies/orient/core/sql/parser/OBaseExpression.java index 1e4d1415a0b..f1d485215bb 100644 --- a/core/src/main/java/com/orientechnologies/orient/core/sql/parser/OBaseExpression.java +++ b/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; diff --git a/core/src/main/java/com/orientechnologies/orient/core/sql/parser/OBaseIdentifier.java b/core/src/main/java/com/orientechnologies/orient/core/sql/parser/OBaseIdentifier.java index 088726014bd..10f672fece8 100644 --- a/core/src/main/java/com/orientechnologies/orient/core/sql/parser/OBaseIdentifier.java +++ b/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. **/ diff --git a/core/src/main/java/com/orientechnologies/orient/core/sql/parser/OExpression.java b/core/src/main/java/com/orientechnologies/orient/core/sql/parser/OExpression.java index e99d4a3fe54..4a9119e3860 100644 --- a/core/src/main/java/com/orientechnologies/orient/core/sql/parser/OExpression.java +++ b/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) */ diff --git a/core/src/main/java/com/orientechnologies/orient/core/sql/parser/OOrderByItem.java b/core/src/main/java/com/orientechnologies/orient/core/sql/parser/OOrderByItem.java index 964128037ba..8c49effbd17 100644 --- a/core/src/main/java/com/orientechnologies/orient/core/sql/parser/OOrderByItem.java +++ b/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; + } } diff --git a/core/src/main/java/com/orientechnologies/orient/core/sql/parser/ORecordAttribute.java b/core/src/main/java/com/orientechnologies/orient/core/sql/parser/ORecordAttribute.java index 0ebf6ba0ba8..79126c66b01 100644 --- a/core/src/main/java/com/orientechnologies/orient/core/sql/parser/ORecordAttribute.java +++ b/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) */ diff --git a/core/src/main/java/com/orientechnologies/orient/core/sql/parser/OSuffixIdentifier.java b/core/src/main/java/com/orientechnologies/orient/core/sql/parser/OSuffixIdentifier.java index 945bd581cc9..280e0f14c54 100644 --- a/core/src/main/java/com/orientechnologies/orient/core/sql/parser/OSuffixIdentifier.java +++ b/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. **/ diff --git a/core/src/main/java/com/orientechnologies/orient/core/sql/parser/OrientSql.java b/core/src/main/java/com/orientechnologies/orient/core/sql/parser/OrientSql.java index 8015257fab3..07d020ac54f 100644 --- a/core/src/main/java/com/orientechnologies/orient/core/sql/parser/OrientSql.java +++ b/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: diff --git a/core/src/test/java/com/orientechnologies/orient/core/sql/executor/OSelectStatementExecutionTest.java b/core/src/test/java/com/orientechnologies/orient/core/sql/executor/OSelectStatementExecutionTest.java index de97b1edf23..fdcdd24ed64 100644 --- a/core/src/test/java/com/orientechnologies/orient/core/sql/executor/OSelectStatementExecutionTest.java +++ b/core/src/test/java/com/orientechnologies/orient/core/sql/executor/OSelectStatementExecutionTest.java @@ -292,6 +292,66 @@ public class OSelectStatementExecutionTest { result.close(); } + @Test public void testSelectOrderWithProjections() { + String className = "testSelectOrderWithProjections"; + db.getMetadata().getSchema().createClass(className); + for (int i = 0; i < 100; i++) { + ODocument doc = db.newInstance(className); + doc.setProperty("name", "name" + i % 10); + doc.setProperty("surname", "surname" + i % 10); + doc.save(); + } + long begin = System.nanoTime(); + OTodoResultSet result = db.query("select name from " + className + " order by surname asc"); + System.out.println("elapsed: " + (System.nanoTime() - begin)); + printExecutionPlan(result); + + String lastName = null; + for (int i = 0; i < 100; i++) { + Assert.assertTrue(result.hasNext()); + OResult item = result.next(); + Assert.assertNotNull(item); + String name = item.getProperty("name"); + Assert.assertNotNull(name); + if (i > 0) { + Assert.assertTrue(name.compareTo(lastName) >= 0); + } + lastName = name; + } + Assert.assertFalse(result.hasNext()); + result.close(); + } + + @Test public void testSelectOrderWithProjections2() { + String className = "testSelectOrderWithProjections2"; + db.getMetadata().getSchema().createClass(className); + for (int i = 0; i < 100; i++) { + ODocument doc = db.newInstance(className); + doc.setProperty("name", "name" + i % 10); + doc.setProperty("surname", "surname" + i % 10); + doc.save(); + } + long begin = System.nanoTime(); + OTodoResultSet result = db.query("select name from " + className + " order by name asc, surname asc"); + System.out.println("elapsed: " + (System.nanoTime() - begin)); + printExecutionPlan(result); + + String lastName = null; + for (int i = 0; i < 100; i++) { + Assert.assertTrue(result.hasNext()); + OResult item = result.next(); + Assert.assertNotNull(item); + String name = item.getProperty("name"); + Assert.assertNotNull(name); + if (i > 0) { + Assert.assertTrue(name.compareTo(lastName) >= 0); + } + lastName = name; + } + Assert.assertFalse(result.hasNext()); + result.close(); + } + @Test public void testSelectFullScanWithFilter1() { String className = "testSelectFullScanWithFilter1"; db.getMetadata().getSchema().createClass(className);