Skip to content

Commit 7261bd5

Browse files
author
James Leigh
committed
Fix #87: Handle multiple aggregates in ORDER BY clause
Signed-off-by: James Leigh <james.leigh@ontotext.com>
1 parent 9a2471f commit 7261bd5

5 files changed

Lines changed: 30 additions & 0 deletions

File tree

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

Lines changed: 24 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@
88
package org.eclipse.rdf4j.query.algebra.evaluation.impl;
99

1010
import static org.junit.Assert.assertEquals;
11+
import static org.junit.Assert.assertNotEquals;
1112

1213
import org.eclipse.rdf4j.RDF4JException;
1314
import org.eclipse.rdf4j.query.MalformedQueryException;
@@ -19,6 +20,7 @@
1920
import org.eclipse.rdf4j.query.algebra.QueryRoot;
2021
import org.eclipse.rdf4j.query.algebra.TupleExpr;
2122
import org.eclipse.rdf4j.query.algebra.UnaryTupleOperator;
23+
import org.eclipse.rdf4j.query.algebra.helpers.AbstractQueryModelVisitor;
2224
import org.eclipse.rdf4j.query.parser.ParsedQuery;
2325
import org.eclipse.rdf4j.query.parser.QueryParserUtil;
2426
import org.eclipse.rdf4j.query.parser.sparql.SPARQLParser;
@@ -65,6 +67,28 @@ public void testContextOptimization()
6567
testOptimizer(expectedQuery, query);
6668
}
6769

70+
@Test
71+
public void testSES2306AggregateOrderBy()
72+
throws Exception
73+
{
74+
String select = "PREFIX ex: <ex:>\n" + "SELECT ((MIN(?x+1) + MAX(?y-1))/2 AS ?r) {\n"
75+
+ " ?this ex:name ?n . ?this ex:id ?id . ?this ex:prop1 ?x . ?this ex:prop2 ?y .\n"
76+
+ "} GROUP BY concat(?n, ?id) HAVING (SUM(?x) + SUM(?y) < 5) ORDER BY (COUNT(?x) + COUNT(?y))";
77+
78+
SPARQLParser parser = new SPARQLParser();
79+
ParsedQuery q = parser.parseQuery(select, null);
80+
q.getTupleExpr().visit(new AbstractQueryModelVisitor<Exception>() {
81+
82+
@Override
83+
protected void meetUnaryTupleOperator(UnaryTupleOperator node)
84+
throws Exception
85+
{
86+
assertNotEquals(node, node.getArg());
87+
super.meetUnaryTupleOperator(node);
88+
}
89+
});
90+
}
91+
6892
@Test
6993
public void testSES2116JoinBind()
7094
throws Exception

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

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -67,6 +67,7 @@ public TupleExpr getLeftArg() {
6767
*/
6868
public void setLeftArg(TupleExpr leftArg) {
6969
assert leftArg != null : "leftArg must not be null";
70+
assert leftArg != this : "leftArg must not be itself";
7071
leftArg.setParentNode(this);
7172
this.leftArg = leftArg;
7273
}
@@ -88,6 +89,7 @@ public TupleExpr getRightArg() {
8889
*/
8990
public void setRightArg(TupleExpr rightArg) {
9091
assert rightArg != null : "rightArg must not be null";
92+
assert rightArg != this : "rightArg must not be itself";
9193
rightArg.setParentNode(this);
9294
this.rightArg = rightArg;
9395
}

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

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -61,6 +61,7 @@ public TupleExpr getArg() {
6161
*/
6262
public void setArg(TupleExpr arg) {
6363
assert arg != null : "arg must not be null";
64+
assert arg != this : "arg must not be itself";
6465
arg.setParentNode(this);
6566
this.arg = arg;
6667
}

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

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -375,7 +375,9 @@ private TupleExpr processOrderClause(ASTOrderClause orderNode, TupleExpr tupleEx
375375
// add the aggregate operator to the group.
376376
GroupElem ge = new GroupElem(alias, operator);
377377
group.addGroupElement(ge);
378+
}
378379

380+
if (!extension.getElements().isEmpty()) {
379381
extension.setArg(tupleExpr);
380382
tupleExpr = extension;
381383
}

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

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -115,6 +115,7 @@ private final void setArgs(final List<? extends Expr> args) {
115115
* Sets the <tt>idx</tt>-th argument of this n-ary tuple operator.
116116
*/
117117
protected final void setArg(final int idx, final Expr arg) {
118+
assert arg != this : "arg must not be itself";
118119
if (arg != null) {
119120
// arg can be null (i.e. Regex)
120121
arg.setParentNode(this);

0 commit comments

Comments
 (0)