Repository: marmotta Updated Branches: refs/heads/develop 8610935c9 -> f7b335028
Solved MARMOTTA-657 - SPARQL query GROUP BY with ORDER BY clause fails. When building a SQL query, GROUP BY statements allow aggregate functions. This was solved comparing if an statement from ORDER BY is not an aggregate funtion, then add if not continue. Signed-off-by: Gustavo Mora <[email protected]> Project: http://git-wip-us.apache.org/repos/asf/marmotta/repo Commit: http://git-wip-us.apache.org/repos/asf/marmotta/commit/9abcdd92 Tree: http://git-wip-us.apache.org/repos/asf/marmotta/tree/9abcdd92 Diff: http://git-wip-us.apache.org/repos/asf/marmotta/diff/9abcdd92 Branch: refs/heads/develop Commit: 9abcdd92f3e1615bf7c98540ecc2d9d776313f73 Parents: 8610935 Author: gmora1223 <[email protected]> Authored: Sun Apr 9 17:44:17 2017 -0500 Committer: gmora1223 <[email protected]> Committed: Sun Apr 9 17:44:17 2017 -0500 ---------------------------------------------------------------------- .../kiwi/sparql/builder/SQLBuilder.java | 68 +++++++++++++++++--- .../kiwi/sparql/test/KiWiSparqlTest.java | 37 ++++++++--- .../kiwi/sparql/test/MARMOTTA-657.sparql | 25 +++++++ 3 files changed, 114 insertions(+), 16 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/marmotta/blob/9abcdd92/libraries/kiwi/kiwi-sparql/src/main/java/org/apache/marmotta/kiwi/sparql/builder/SQLBuilder.java ---------------------------------------------------------------------- diff --git a/libraries/kiwi/kiwi-sparql/src/main/java/org/apache/marmotta/kiwi/sparql/builder/SQLBuilder.java b/libraries/kiwi/kiwi-sparql/src/main/java/org/apache/marmotta/kiwi/sparql/builder/SQLBuilder.java index c842aca..ca561e8 100644 --- a/libraries/kiwi/kiwi-sparql/src/main/java/org/apache/marmotta/kiwi/sparql/builder/SQLBuilder.java +++ b/libraries/kiwi/kiwi-sparql/src/main/java/org/apache/marmotta/kiwi/sparql/builder/SQLBuilder.java @@ -19,10 +19,31 @@ package org.apache.marmotta.kiwi.sparql.builder; import com.google.common.base.Joiner; import com.google.common.base.Preconditions; +import java.util.ArrayList; +import java.util.Arrays; +import java.util.Collections; +import java.util.HashMap; +import java.util.HashSet; +import java.util.Iterator; +import java.util.LinkedList; +import java.util.List; +import java.util.Map; +import java.util.Set; +import org.apache.commons.lang3.StringUtils; import org.apache.marmotta.kiwi.model.rdf.KiWiNode; import org.apache.marmotta.kiwi.persistence.KiWiDialect; import org.apache.marmotta.kiwi.sail.KiWiValueFactory; -import org.apache.marmotta.kiwi.sparql.builder.collect.*; +import org.apache.marmotta.kiwi.sparql.builder.collect.ConditionFinder; +import org.apache.marmotta.kiwi.sparql.builder.collect.DistinctFinder; +import org.apache.marmotta.kiwi.sparql.builder.collect.ExtensionFinder; +import org.apache.marmotta.kiwi.sparql.builder.collect.GroupFinder; +import org.apache.marmotta.kiwi.sparql.builder.collect.LimitFinder; +import org.apache.marmotta.kiwi.sparql.builder.collect.LiteralTypeExpressionFinder; +import org.apache.marmotta.kiwi.sparql.builder.collect.OPTypeFinder; +import org.apache.marmotta.kiwi.sparql.builder.collect.OrderFinder; +import org.apache.marmotta.kiwi.sparql.builder.collect.PatternCollector; +import org.apache.marmotta.kiwi.sparql.builder.collect.SQLProjectionFinder; +import org.apache.marmotta.kiwi.sparql.builder.collect.VariableFinder; import org.apache.marmotta.kiwi.sparql.builder.eval.ValueExpressionEvaluator; import org.apache.marmotta.kiwi.sparql.builder.model.SQLAbstractSubquery; import org.apache.marmotta.kiwi.sparql.builder.model.SQLFragment; @@ -36,12 +57,38 @@ import org.openrdf.model.Value; import org.openrdf.model.vocabulary.SESAME; import org.openrdf.query.BindingSet; import org.openrdf.query.Dataset; -import org.openrdf.query.algebra.*; +import org.openrdf.query.algebra.Avg; +import org.openrdf.query.algebra.BNodeGenerator; +import org.openrdf.query.algebra.Compare; +import org.openrdf.query.algebra.Count; +import org.openrdf.query.algebra.Distinct; +import org.openrdf.query.algebra.Exists; +import org.openrdf.query.algebra.Extension; +import org.openrdf.query.algebra.ExtensionElem; +import org.openrdf.query.algebra.Filter; +import org.openrdf.query.algebra.FunctionCall; +import org.openrdf.query.algebra.Group; +import org.openrdf.query.algebra.IRIFunction; +import org.openrdf.query.algebra.If; +import org.openrdf.query.algebra.Join; +import org.openrdf.query.algebra.LeftJoin; +import org.openrdf.query.algebra.MathExpr; +import org.openrdf.query.algebra.NAryValueOperator; +import org.openrdf.query.algebra.Order; +import org.openrdf.query.algebra.OrderElem; +import org.openrdf.query.algebra.Projection; +import org.openrdf.query.algebra.Reduced; +import org.openrdf.query.algebra.Slice; +import org.openrdf.query.algebra.StatementPattern; +import org.openrdf.query.algebra.Sum; +import org.openrdf.query.algebra.TupleExpr; +import org.openrdf.query.algebra.Union; +import org.openrdf.query.algebra.ValueConstant; +import org.openrdf.query.algebra.ValueExpr; +import org.openrdf.query.algebra.Var; import org.slf4j.Logger; import org.slf4j.LoggerFactory; -import java.util.*; - /** * A builder for translating SPARQL queries into SQL. * @@ -53,6 +100,11 @@ public class SQLBuilder { private static Logger log = LoggerFactory.getLogger(SQLBuilder.class); /** + * Aggregates defined in version 1.1 of SPARQL. List used to avoid MARMOTTA-657 because GROUP BY don't allow aggregate functions. + */ + private static final String[] aggregateFuncs = {"COUNT", "SUM", "MIN", "MAX", "AVG", "GROUP_CONCAT","SAMPLE"}; + + /** * Simplify access to different node ids */ private static final String[] positions = new String[] {"subject","predicate","object","context"}; @@ -796,13 +848,13 @@ public class SQLBuilder { } if (orderby.size() > 0) { - groupClause.append(", "); for(Iterator<OrderElem> it = orderby.iterator(); it.hasNext(); ) { OrderElem elem = it.next(); - groupClause.append(evaluateExpression(elem.getExpr(), ValueType.STRING)); - if (it.hasNext()) { - groupClause.append(", "); + String expr = evaluateExpression(elem.getExpr(), ValueType.STRING); + if (StringUtils.indexOfAny(expr, aggregateFuncs) != -1) { + continue; } + groupClause.append(", ").append(expr); } } http://git-wip-us.apache.org/repos/asf/marmotta/blob/9abcdd92/libraries/kiwi/kiwi-sparql/src/test/java/org/apache/marmotta/kiwi/sparql/test/KiWiSparqlTest.java ---------------------------------------------------------------------- diff --git a/libraries/kiwi/kiwi-sparql/src/test/java/org/apache/marmotta/kiwi/sparql/test/KiWiSparqlTest.java b/libraries/kiwi/kiwi-sparql/src/test/java/org/apache/marmotta/kiwi/sparql/test/KiWiSparqlTest.java index fa0ae12..4451399 100644 --- a/libraries/kiwi/kiwi-sparql/src/test/java/org/apache/marmotta/kiwi/sparql/test/KiWiSparqlTest.java +++ b/libraries/kiwi/kiwi-sparql/src/test/java/org/apache/marmotta/kiwi/sparql/test/KiWiSparqlTest.java @@ -20,19 +20,37 @@ package org.apache.marmotta.kiwi.sparql.test; import com.google.common.base.Function; import com.google.common.collect.Lists; import info.aduna.iteration.Iterations; +import java.io.IOException; +import java.sql.SQLException; +import java.util.HashSet; +import java.util.List; +import java.util.Set; import org.apache.commons.collections.CollectionUtils; import org.apache.commons.io.IOUtils; import org.apache.marmotta.kiwi.config.KiWiConfiguration; import org.apache.marmotta.kiwi.sail.KiWiStore; import org.apache.marmotta.kiwi.sparql.sail.KiWiSparqlSail; import org.apache.marmotta.kiwi.test.junit.KiWiDatabaseRunner; -import org.junit.*; +import org.junit.After; +import org.junit.Assert; +import org.junit.Assume; +import org.junit.Before; +import org.junit.Rule; +import org.junit.Test; import org.junit.rules.TestWatcher; import org.junit.runner.Description; import org.junit.runner.RunWith; import org.openrdf.model.BNode; import org.openrdf.model.Literal; -import org.openrdf.query.*; +import org.openrdf.query.Binding; +import org.openrdf.query.BindingSet; +import org.openrdf.query.GraphQuery; +import org.openrdf.query.GraphQueryResult; +import org.openrdf.query.MalformedQueryException; +import org.openrdf.query.QueryEvaluationException; +import org.openrdf.query.QueryLanguage; +import org.openrdf.query.TupleQuery; +import org.openrdf.query.TupleQueryResult; import org.openrdf.repository.Repository; import org.openrdf.repository.RepositoryConnection; import org.openrdf.repository.RepositoryException; @@ -43,12 +61,6 @@ import org.openrdf.sail.memory.MemoryStore; import org.slf4j.Logger; import org.slf4j.LoggerFactory; -import java.io.IOException; -import java.sql.SQLException; -import java.util.HashSet; -import java.util.List; -import java.util.Set; - /** * Test the KiWi SPARQL * @@ -132,6 +144,15 @@ public class KiWiSparqlTest { testQueryCompareResults("MARMOTTA-578.sparql"); } + /** + * Tests if evaluation of query executes. Solves MARMOTTA-657. + * @throws Exception + */ + @Test + public void testMarmotta657() throws Exception{ + final String queryString = IOUtils.toString(this.getClass().getResourceAsStream("MARMOTTA-657.sparql"), "UTF-8"); + testQueryEvaluation(queryString); + } //TODO: generalize this infrastructure code also used by KiWiSparqlJoinTest private void testQueryCompareResults(String filename) throws Exception { http://git-wip-us.apache.org/repos/asf/marmotta/blob/9abcdd92/libraries/kiwi/kiwi-sparql/src/test/resources/org/apache/marmotta/kiwi/sparql/test/MARMOTTA-657.sparql ---------------------------------------------------------------------- diff --git a/libraries/kiwi/kiwi-sparql/src/test/resources/org/apache/marmotta/kiwi/sparql/test/MARMOTTA-657.sparql b/libraries/kiwi/kiwi-sparql/src/test/resources/org/apache/marmotta/kiwi/sparql/test/MARMOTTA-657.sparql new file mode 100644 index 0000000..114a8db --- /dev/null +++ b/libraries/kiwi/kiwi-sparql/src/test/resources/org/apache/marmotta/kiwi/sparql/test/MARMOTTA-657.sparql @@ -0,0 +1,25 @@ +# +# Licensed to the Apache Software Foundation (ASF) under one +# or more contributor license agreements. See the NOTICE file +# distributed with this work for additional information +# regarding copyright ownership. The ASF licenses this file +# to you under the Apache License, Version 2.0 (the +# "License"); you may not use this file except in compliance +# with the License. You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +# See the License for the specific language governing permissions and +# limitations under the License. +# + +SELECT ?g (COUNT(*) AS ?c) { + GRAPH ?g { + ?s ?p ?o + } +} +GROUP BY ?g +ORDER BY DESC(?c)
