Merge pull request #897 from jamesrdf/issues/#87-aggregate-order-by

Fix #87: Handle multiple aggregates in ORDER BY clause

Author: James Leigh

core/queryalgebra/evaluation/src/test/java/org/eclipse/rdf4j/query/algebra/evaluation/impl/QueryJoinOptimizerTest.java

@@ -8,6 +8,7 @@
 package org.eclipse.rdf4j.query.algebra.evaluation.impl;
 
 import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertNotEquals;
 
 import org.eclipse.rdf4j.RDF4JException;
 import org.eclipse.rdf4j.query.MalformedQueryException;
@@ -19,6 +20,7 @@
 import org.eclipse.rdf4j.query.algebra.QueryRoot;
 import org.eclipse.rdf4j.query.algebra.TupleExpr;
 import org.eclipse.rdf4j.query.algebra.UnaryTupleOperator;
+import org.eclipse.rdf4j.query.algebra.helpers.AbstractQueryModelVisitor;
 import org.eclipse.rdf4j.query.parser.ParsedQuery;
 import org.eclipse.rdf4j.query.parser.QueryParserUtil;
 import org.eclipse.rdf4j.query.parser.sparql.SPARQLParser;
@@ -65,6 +67,28 @@ public void testContextOptimization()
 		testOptimizer(expectedQuery, query);
 	}
 
+	@Test
+	public void testSES2306AggregateOrderBy()
+		throws Exception
+	{
+		String select = "PREFIX ex: <ex:>\n" + "SELECT ((MIN(?x+1) + MAX(?y-1))/2 AS ?r) {\n"
+				+ "	?this ex:name ?n . ?this ex:id ?id . ?this ex:prop1 ?x . ?this ex:prop2 ?y .\n"
+				+ "} GROUP BY concat(?n, ?id) HAVING (SUM(?x) + SUM(?y) < 5) ORDER BY (COUNT(?x) + COUNT(?y))";
+
+		SPARQLParser parser = new SPARQLParser();
+		ParsedQuery q = parser.parseQuery(select, null);
+		q.getTupleExpr().visit(new AbstractQueryModelVisitor<Exception>() {
+
+			@Override
+			protected void meetUnaryTupleOperator(UnaryTupleOperator node)
+				throws Exception
+			{
+				assertNotEquals(node, node.getArg());
+				super.meetUnaryTupleOperator(node);
+			}
+		});
+	}
+
 	@Test
 	public void testSES2116JoinBind()
 		throws Exception

core/queryalgebra/model/src/main/java/org/eclipse/rdf4j/query/algebra/BinaryTupleOperator.java

@@ -67,6 +67,7 @@ public TupleExpr getLeftArg() {
 	 */
 	public void setLeftArg(TupleExpr leftArg) {
 		assert leftArg != null : "leftArg must not be null";
+		assert leftArg != this : "leftArg must not be itself";
 		leftArg.setParentNode(this);
 		this.leftArg = leftArg;
 	}
@@ -88,6 +89,7 @@ public TupleExpr getRightArg() {
 	 */
 	public void setRightArg(TupleExpr rightArg) {
 		assert rightArg != null : "rightArg must not be null";
+		assert rightArg != this : "rightArg must not be itself";
 		rightArg.setParentNode(this);
 		this.rightArg = rightArg;
 	}

core/queryalgebra/model/src/main/java/org/eclipse/rdf4j/query/algebra/UnaryTupleOperator.java

@@ -61,6 +61,7 @@ public TupleExpr getArg() {
 	 */
 	public void setArg(TupleExpr arg) {
 		assert arg != null : "arg must not be null";
+		assert arg != this : "arg must not be itself";
 		arg.setParentNode(this);
 		this.arg = arg;
 	}

core/queryparser/sparql/src/main/java/org/eclipse/rdf4j/query/parser/sparql/TupleExprBuilder.java

@@ -375,7 +375,9 @@ private TupleExpr processOrderClause(ASTOrderClause orderNode, TupleExpr tupleEx
 					// add the aggregate operator to the group.
 					GroupElem ge = new GroupElem(alias, operator);
 					group.addGroupElement(ge);
+				}
 
+				if (!extension.getElements().isEmpty()) {
 					extension.setArg(tupleExpr);
 					tupleExpr = extension;
 				}

core/sail/federation/src/main/java/org/eclipse/rdf4j/sail/federation/algebra/AbstractNaryOperator.java

@@ -115,6 +115,7 @@ private final void setArgs(final List<? extends Expr> args) {
 	 * Sets the <tt>idx</tt>-th argument of this n-ary tuple operator.
 	 */
 	protected final void setArg(final int idx, final Expr arg) {
+		assert arg != this : "arg must not be itself";
 		if (arg != null) {
 			// arg can be null (i.e. Regex)
 			arg.setParentNode(this);