diff --git a/jopa-impl/src/main/java/cz/cvut/kbss/jopa/query/sparql/loader/AttributeEnumeratingSparqlAssemblyModifier.java b/jopa-impl/src/main/java/cz/cvut/kbss/jopa/query/sparql/loader/AttributeEnumeratingSparqlAssemblyModifier.java index ec5b8b247..562aca66a 100644 --- a/jopa-impl/src/main/java/cz/cvut/kbss/jopa/query/sparql/loader/AttributeEnumeratingSparqlAssemblyModifier.java +++ b/jopa-impl/src/main/java/cz/cvut/kbss/jopa/query/sparql/loader/AttributeEnumeratingSparqlAssemblyModifier.java @@ -114,7 +114,7 @@ private void modifyProjection(TokenQueryParameter firstProjected, TokenStream boolean hasGroupConcat = false; for (QueryVariableMapping varMapping : queryMod.variables()) { // PERF: GROUP_CONCAT may prevent combinatorial blowup of the number of rows - if (varMapping.canGroupConcat()) { + if (!queryAttributes.hasOrderBy() && varMapping.canGroupConcat()) { projection.append(' ').append(varMapping.generateGroupConcat()); hasGroupConcat = true; } else { @@ -126,10 +126,8 @@ private void modifyProjection(TokenQueryParameter firstProjected, TokenStream tokenRewriter.insertAfter(firstProjected.getSingleToken(), projection.toString()); if (hasGroupConcat) { tokenRewriter.insertAfter(queryAttributes.lastClosingCurlyBraceToken(), groupBy.toString()); - if (!queryAttributes.hasOrderBy()) { - // Add ordering to ensure stable results with rows with the same subject in sequence - tokenRewriter.insertAfter(queryAttributes.lastClosingCurlyBraceToken(), " ORDER BY " + firstProjected.getIdentifierAsQueryString()); - } + // Add ordering to ensure stable results with rows with the same subject in sequence + tokenRewriter.insertAfter(queryAttributes.lastClosingCurlyBraceToken(), " ORDER BY " + firstProjected.getIdentifierAsQueryString()); } } diff --git a/jopa-impl/src/test/java/cz/cvut/kbss/jopa/query/sparql/loader/AttributeEnumeratingSparqlAssemblyModifierTest.java b/jopa-impl/src/test/java/cz/cvut/kbss/jopa/query/sparql/loader/AttributeEnumeratingSparqlAssemblyModifierTest.java index fc685617d..16bc5a6d3 100644 --- a/jopa-impl/src/test/java/cz/cvut/kbss/jopa/query/sparql/loader/AttributeEnumeratingSparqlAssemblyModifierTest.java +++ b/jopa-impl/src/test/java/cz/cvut/kbss/jopa/query/sparql/loader/AttributeEnumeratingSparqlAssemblyModifierTest.java @@ -298,6 +298,20 @@ void modifyGroupConcatenatesPluralStringAttributeValues() { assertThat(result, containsString("GROUP BY ?x")); } + @Test + void modifyDoesNotAddGroupConcatWhenQueryHasOrderBy() { + final TokenStreamSparqlQueryHolder holder = parser.parseQuery("SELECT ?x WHERE { ?x a ?type ; ?hasString ?stringAttribute } ORDER BY ?stringAttribute"); + final EntityGraph fetchGraph = new EntityGraphImpl<>(metamodelMocks.forOwlClassN() + .entityType(), metamodel); + fetchGraph.addAttributeNodes("pluralAnnotation"); + final AttributeEnumeratingSparqlAssemblyModifier sut = createSut(metamodelMocks.forOwlClassN() + .entityType(), new EntityDescriptor(), fetchGraph); + holder.setAssemblyModifier(sut); + + final String result = holder.assembleQuery(); + assertThat(result, not(containsString("GROUP_CONCAT"))); + } + /** * Simplified Hamcrest matcher that checks if a SPARQL query projects a specific variable. The matcher extracts the * projection part of the query (everything before the WHERE clause) and checks if it contains the specified