Merge branch 'master' of github.com:eclipse/rdf4j into develop

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/repository/sparql/src/main/java/org/eclipse/rdf4j/repository/sparql/query/QueryStringUtil.java

@@ -35,8 +35,22 @@ public class QueryStringUtil {
 	 * @param queryString
 	 * @param bindings
 	 * @return the modified queryString
+	 * @deprecated since 2.3 use {@link #getTupleQueryString(String, BindingSet)}
 	 */
+	@Deprecated
 	public static String getQueryString(String queryString, BindingSet bindings) {
+		return getTupleQueryString(queryString, bindings);
+	}
+
+	/**
+	 * Retrieve a modified queryString into which all bindings of the given argument are replaced, with the
+	 * binding names included in the SELECT clause.
+	 *
+	 * @param queryString
+	 * @param bindings
+	 * @return the modified queryString
+	 */
+	public static String getTupleQueryString(String queryString, BindingSet bindings) {
 		if (bindings.size() == 0) {
 			return queryString;
 		}
@@ -61,6 +75,56 @@ public static String getQueryString(String queryString, BindingSet bindings) {
 		return select + where;
 	}
 
+	/**
+	 * Retrieve a modified queryString into which all bindings of the given argument are replaced with their
+	 * value.
+	 *
+	 * @param queryString
+	 * @param bindings
+	 * @return the modified queryString
+	 */
+	public static String getUpdateString(String queryString, BindingSet bindings) {
+		return getGraphQueryString(queryString, bindings);
+	}
+
+	/**
+	 * Retrieve a modified queryString into which all bindings of the given argument are replaced with their
+	 * value.
+	 *
+	 * @param queryString
+	 * @param bindings
+	 * @return the modified queryString
+	 */
+	public static String getBooleanQueryString(String queryString, BindingSet bindings) {
+		return getGraphQueryString(queryString, bindings);
+	}
+
+	/**
+	 * Retrieve a modified queryString into which all bindings of the given argument are replaced with their
+	 * value.
+	 *
+	 * @param queryString
+	 * @param bindings
+	 * @return the modified queryString
+	 */
+	public static String getGraphQueryString(String queryString, BindingSet bindings) {
+		if (bindings.size() == 0) {
+			return queryString;
+		}
+
+		String qry = queryString;
+		for (String name : bindings.getBindingNames()) {
+			String replacement = getReplacement(bindings.getValue(name));
+			if (replacement != null) {
+				String pattern = "[\\?\\$]" + name + "(?=\\W)";
+				// we use Matcher.quoteReplacement to make sure things like newlines
+				// in literal values  are preserved
+				qry = qry.replaceAll(pattern, Matcher.quoteReplacement(replacement));
+			}
+		}
+		return qry;
+	}
+
 	private static String getReplacement(Value value) {
 		StringBuilder sb = new StringBuilder();
 		if (value instanceof IRI) {

core/repository/sparql/src/main/java/org/eclipse/rdf4j/repository/sparql/query/SPARQLBooleanQuery.java

@@ -50,6 +50,6 @@ public boolean evaluate()
 	}
 
 	private String getQueryString() {
-		return QueryStringUtil.getQueryString(queryString, getBindings());
+		return QueryStringUtil.getBooleanQueryString(queryString, getBindings());
 	}
 }

core/repository/sparql/src/main/java/org/eclipse/rdf4j/repository/sparql/query/SPARQLGraphQuery.java

@@ -74,6 +74,6 @@ public void evaluate(RDFHandler handler)
 	}
 
 	private String getQueryString() {
-		return QueryStringUtil.getQueryString(queryString, getBindings());
+		return QueryStringUtil.getGraphQueryString(queryString, getBindings());
 	}
 }

core/repository/sparql/src/main/java/org/eclipse/rdf4j/repository/sparql/query/SPARQLTupleQuery.java

@@ -75,6 +75,6 @@ public void evaluate(TupleQueryResultHandler handler)
 	}
 
 	private String getQueryString() {
-		return QueryStringUtil.getQueryString(queryString, getBindings());
+		return QueryStringUtil.getTupleQueryString(queryString, getBindings());
 	}
 }

core/repository/sparql/src/main/java/org/eclipse/rdf4j/repository/sparql/query/SPARQLUpdate.java

@@ -64,6 +64,6 @@ public void execute()
 
 	@Override
 	public String getQueryString() {
-		return QueryStringUtil.getQueryString(queryString, getBindings());
+		return QueryStringUtil.getUpdateString(queryString, getBindings());
 	}
 }

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);