diff --git a/pom.xml b/pom.xml index 617cc011d3..c876280cb5 100755 --- a/pom.xml +++ b/pom.xml @@ -5,7 +5,7 @@ org.springframework.data spring-data-jpa-parent - 3.4.0-SNAPSHOT + 3.4.x-3427-SNAPSHOT pom Spring Data JPA Parent diff --git a/spring-data-envers/pom.xml b/spring-data-envers/pom.xml index 8b836ae2f3..5896ecf294 100755 --- a/spring-data-envers/pom.xml +++ b/spring-data-envers/pom.xml @@ -5,12 +5,12 @@ org.springframework.data spring-data-envers - 3.4.0-SNAPSHOT + 3.4.x-3427-SNAPSHOT org.springframework.data spring-data-jpa-parent - 3.4.0-SNAPSHOT + 3.4.x-3427-SNAPSHOT ../pom.xml diff --git a/spring-data-jpa-distribution/pom.xml b/spring-data-jpa-distribution/pom.xml index a90c1f7282..3ab9ac6565 100644 --- a/spring-data-jpa-distribution/pom.xml +++ b/spring-data-jpa-distribution/pom.xml @@ -14,7 +14,7 @@ org.springframework.data spring-data-jpa-parent - 3.4.0-SNAPSHOT + 3.4.x-3427-SNAPSHOT ../pom.xml diff --git a/spring-data-jpa/pom.xml b/spring-data-jpa/pom.xml index 3006702796..2065b14663 100644 --- a/spring-data-jpa/pom.xml +++ b/spring-data-jpa/pom.xml @@ -6,7 +6,7 @@ org.springframework.data spring-data-jpa - 3.4.0-SNAPSHOT + 3.4.x-3427-SNAPSHOT Spring Data JPA Spring Data module for JPA repositories. @@ -15,7 +15,7 @@ org.springframework.data spring-data-jpa-parent - 3.4.0-SNAPSHOT + 3.4.x-3427-SNAPSHOT ../pom.xml diff --git a/spring-data-jpa/src/main/java/org/springframework/data/jpa/repository/query/EqlSortedQueryTransformer.java b/spring-data-jpa/src/main/java/org/springframework/data/jpa/repository/query/EqlSortedQueryTransformer.java index 525f48acfb..ed14e9afdf 100644 --- a/spring-data-jpa/src/main/java/org/springframework/data/jpa/repository/query/EqlSortedQueryTransformer.java +++ b/spring-data-jpa/src/main/java/org/springframework/data/jpa/repository/query/EqlSortedQueryTransformer.java @@ -23,6 +23,7 @@ import org.springframework.data.jpa.repository.query.QueryRenderer.QueryRendererBuilder; import org.springframework.lang.Nullable; import org.springframework.util.Assert; +import org.springframework.util.ObjectUtils; /** * An ANTLR {@link org.antlr.v4.runtime.tree.ParseTreeVisitor} that transforms a parsed EQL query by applying @@ -30,6 +31,7 @@ * * @author Greg Turnquist * @author Mark Paluch + * @author Christoph Strobl * @since 3.2 */ @SuppressWarnings("ConstantValue") @@ -67,7 +69,7 @@ public QueryRendererBuilder visitSelect_statement(EqlParser.Select_statementCont builder.appendExpression(visit(ctx.having_clause())); } - doVisitOrderBy(builder, ctx); + doVisitOrderBy(builder, ctx, ObjectUtils.isEmpty(ctx.setOperator()) ? this.sort : Sort.unsorted()); for (int i = 0; i < ctx.setOperator().size(); i++) { @@ -78,7 +80,7 @@ public QueryRendererBuilder visitSelect_statement(EqlParser.Select_statementCont return builder; } - private void doVisitOrderBy(QueryRendererBuilder builder, EqlParser.Select_statementContext ctx) { + private void doVisitOrderBy(QueryRendererBuilder builder, EqlParser.Select_statementContext ctx, Sort sort) { if (ctx.orderby_clause() != null) { QueryTokenStream existingOrder = visit(ctx.orderby_clause()); diff --git a/spring-data-jpa/src/main/java/org/springframework/data/jpa/repository/query/HqlSortedQueryTransformer.java b/spring-data-jpa/src/main/java/org/springframework/data/jpa/repository/query/HqlSortedQueryTransformer.java index fc28604047..f439ad6695 100644 --- a/spring-data-jpa/src/main/java/org/springframework/data/jpa/repository/query/HqlSortedQueryTransformer.java +++ b/spring-data-jpa/src/main/java/org/springframework/data/jpa/repository/query/HqlSortedQueryTransformer.java @@ -23,6 +23,7 @@ import org.springframework.data.jpa.repository.query.QueryRenderer.QueryRendererBuilder; import org.springframework.lang.Nullable; import org.springframework.util.Assert; +import org.springframework.util.ObjectUtils; /** * An ANTLR {@link org.antlr.v4.runtime.tree.ParseTreeVisitor} that transforms a parsed HQL query. @@ -46,8 +47,72 @@ class HqlSortedQueryTransformer extends HqlQueryRenderer { this.primaryFromAlias = primaryFromAlias; } + + public QueryTokenStream visitQueryExpression(HqlParser.QueryExpressionContext ctx) { + + if(ObjectUtils.isEmpty(ctx.setOperator())) { + return super.visitQueryExpression(ctx); + } + + QueryRendererBuilder builder = QueryRenderer.builder(); + if (ctx.withClause() != null) { + builder.appendExpression(visit(ctx.withClause())); + } + + builder.append(visitOrderedQuery(ctx.orderedQuery(0), Sort.unsorted())); + + for (int i = 1; i < ctx.orderedQuery().size(); i++) { + + builder.append(visit(ctx.setOperator(i - 1))); + builder.append(visit(ctx.orderedQuery(i))); + } + + + return builder; + } + @Override public QueryRendererBuilder visitOrderedQuery(HqlParser.OrderedQueryContext ctx) { + return visitOrderedQuery(ctx, this.sort); + } + + @Override + public QueryTokenStream visitJoinPath(HqlParser.JoinPathContext ctx) { + + QueryTokenStream tokens = super.visitJoinPath(ctx); + + if (ctx.variable() != null && !isSubquery(ctx)) { + transformerSupport.registerAlias(tokens.getLast()); + } + + return tokens; + } + + @Override + public QueryTokenStream visitJoinSubquery(HqlParser.JoinSubqueryContext ctx) { + + QueryTokenStream tokens = super.visitJoinSubquery(ctx); + + if (ctx.variable() != null && !tokens.isEmpty() && !isSubquery(ctx)) { + transformerSupport.registerAlias(tokens.getLast()); + } + + return tokens; + } + + @Override + public QueryTokenStream visitVariable(HqlParser.VariableContext ctx) { + + QueryTokenStream tokens = super.visitVariable(ctx); + + if (ctx.identifier() != null && !tokens.isEmpty() && !isSubquery(ctx)) { + transformerSupport.registerAlias(tokens.getLast()); + } + + return tokens; + } + + private QueryRendererBuilder visitOrderedQuery(HqlParser.OrderedQueryContext ctx, Sort sort) { QueryRendererBuilder builder = QueryRenderer.builder(); @@ -95,40 +160,4 @@ public QueryRendererBuilder visitOrderedQuery(HqlParser.OrderedQueryContext ctx) return builder; } - @Override - public QueryTokenStream visitJoinPath(HqlParser.JoinPathContext ctx) { - - QueryTokenStream tokens = super.visitJoinPath(ctx); - - if (ctx.variable() != null) { - transformerSupport.registerAlias(tokens.getLast()); - } - - return tokens; - } - - @Override - public QueryTokenStream visitJoinSubquery(HqlParser.JoinSubqueryContext ctx) { - - QueryTokenStream tokens = super.visitJoinSubquery(ctx); - - if (ctx.variable() != null && !tokens.isEmpty()) { - transformerSupport.registerAlias(tokens.getLast()); - } - - return tokens; - } - - @Override - public QueryTokenStream visitVariable(HqlParser.VariableContext ctx) { - - QueryTokenStream tokens = super.visitVariable(ctx); - - if (ctx.identifier() != null && !tokens.isEmpty()) { - transformerSupport.registerAlias(tokens.getLast()); - } - - return tokens; - } - } diff --git a/spring-data-jpa/src/main/java/org/springframework/data/jpa/repository/query/JpaQueryTransformerSupport.java b/spring-data-jpa/src/main/java/org/springframework/data/jpa/repository/query/JpaQueryTransformerSupport.java index c5e4cacba4..5ec36640e9 100644 --- a/spring-data-jpa/src/main/java/org/springframework/data/jpa/repository/query/JpaQueryTransformerSupport.java +++ b/spring-data-jpa/src/main/java/org/springframework/data/jpa/repository/query/JpaQueryTransformerSupport.java @@ -53,7 +53,7 @@ void registerAlias(QueryToken token) { * @param sort * @return */ - List orderBy(String primaryFromAlias, Sort sort) { + List orderBy(@Nullable String primaryFromAlias, Sort sort) { List tokens = new ArrayList<>(); diff --git a/spring-data-jpa/src/test/java/org/springframework/data/jpa/repository/query/EqlQueryTransformerTests.java b/spring-data-jpa/src/test/java/org/springframework/data/jpa/repository/query/EqlQueryTransformerTests.java index 52bdcd82d1..e0de136bc8 100644 --- a/spring-data-jpa/src/test/java/org/springframework/data/jpa/repository/query/EqlQueryTransformerTests.java +++ b/spring-data-jpa/src/test/java/org/springframework/data/jpa/repository/query/EqlQueryTransformerTests.java @@ -759,6 +759,15 @@ void sortingRecognizesJoinAliases() { """); } + @Test // GH-3427 + void sortShouldBeAppendedToFullSelectOnlyInCaseOfSetOperator() { + + String source = "SELECT tb FROM Test tb WHERE (tb.type='A') UNION SELECT tb FROM Test tb WHERE (tb.type='B')"; + String target = createQueryFor(source, Sort.by("Type").ascending()); + + assertThat(target).isEqualTo("SELECT tb FROM Test tb WHERE (tb.type = 'A') UNION SELECT tb FROM Test tb WHERE (tb.type = 'B') order by tb.Type asc"); + } + static Stream queriesWithReservedWordsAsIdentifiers() { return Stream.of( // diff --git a/spring-data-jpa/src/test/java/org/springframework/data/jpa/repository/query/HqlQueryTransformerTests.java b/spring-data-jpa/src/test/java/org/springframework/data/jpa/repository/query/HqlQueryTransformerTests.java index b4ce21db0a..44350d2f0a 100644 --- a/spring-data-jpa/src/test/java/org/springframework/data/jpa/repository/query/HqlQueryTransformerTests.java +++ b/spring-data-jpa/src/test/java/org/springframework/data/jpa/repository/query/HqlQueryTransformerTests.java @@ -17,6 +17,8 @@ import static org.assertj.core.api.Assertions.*; +import java.util.regex.Matcher; +import java.util.regex.Pattern; import java.util.stream.Stream; import org.assertj.core.api.SoftAssertions; @@ -24,11 +26,13 @@ import org.junit.jupiter.params.ParameterizedTest; import org.junit.jupiter.params.provider.Arguments; import org.junit.jupiter.params.provider.MethodSource; +import org.junit.jupiter.params.provider.ValueSource; import org.springframework.dao.InvalidDataAccessApiUsageException; import org.springframework.data.domain.PageRequest; import org.springframework.data.domain.Sort; import org.springframework.data.jpa.domain.JpaSort; import org.springframework.lang.Nullable; +import org.springframework.util.StringUtils; /** * Verify that HQL queries are properly transformed through the {@link JpaQueryEnhancer} and the @@ -1061,6 +1065,40 @@ void createsCountQueryUsingAliasCorrectly() { "select count(distinct a, count(b)) from Employee AS __ GROUP BY n"); } + @Test // GH-3427 + void sortShouldBeAppendedWithSpacingInCaseOfSetOperator() { + + String source = "SELECT tb FROM Test tb WHERE (tb.type='A') UNION SELECT tb FROM Test tb WHERE (tb.type='B')"; + String target = createQueryFor(source, Sort.by("Type").ascending()); + + assertThat(target).isEqualTo("SELECT tb FROM Test tb WHERE (tb.type = 'A') UNION SELECT tb FROM Test tb WHERE (tb.type = 'B') order by tb.Type asc"); + } + + @ParameterizedTest // GH-3427 + @ValueSource(strings = {"", "res"}) + void sortShouldBeAppendedToSubSelectWithSetOperatorInSubselect(String alias) { + + String prefix = StringUtils.hasText(alias) ? (alias + ".") : ""; + String source = "SELECT %sname FROM (SELECT c.name as name FROM Category c UNION SELECT t.name as name FROM Tag t)".formatted(prefix); + if(StringUtils.hasText(alias)) { + source = source + " %s".formatted(alias); + } + + String target = createQueryFor(source, Sort.by("name").ascending()); + + assertThat(target).contains(" UNION SELECT ").doesNotContainPattern(Pattern.compile(".*\\SUNION")); + assertThat(target).endsWith("order by %sname asc".formatted(prefix)).satisfies(it -> { + Pattern pattern = Pattern.compile("order by %sname".formatted(prefix)); + Matcher matcher = pattern.matcher(target); + int count = 0; + while(matcher.find()) { + count++; + } + assertThat(count).describedAs("Found order by clause more than once in: \n%s", it).isOne(); + }); + + } + private void assertCountQuery(String originalQuery, String countQuery) { assertThat(createCountQueryFor(originalQuery)).isEqualTo(countQuery); }