From 3191b88e82867c6ee597a6445338302a80d01a5e Mon Sep 17 00:00:00 2001 From: Mark Paluch Date: Fri, 2 Aug 2024 10:20:33 +0200 Subject: [PATCH] Polishing. Ensure ordering of the last query only, applying ordering after the set operation. Original pull request: #3429 See #3427 --- .../query/HqlSortedQueryTransformer.java | 25 ++++++++++------- .../query/EqlQueryTransformerTests.java | 8 ++++-- .../query/HqlQueryTransformerTests.java | 28 ++++++++++--------- .../query/JpqlQueryTransformerTests.java | 10 ------- 4 files changed, 35 insertions(+), 36 deletions(-) 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 f439ad6695..b6b8853f93 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 @@ -47,10 +47,10 @@ class HqlSortedQueryTransformer extends HqlQueryRenderer { this.primaryFromAlias = primaryFromAlias; } - + @Override public QueryTokenStream visitQueryExpression(HqlParser.QueryExpressionContext ctx) { - if(ObjectUtils.isEmpty(ctx.setOperator())) { + if (ObjectUtils.isEmpty(ctx.setOperator())) { return super.visitQueryExpression(ctx); } @@ -59,15 +59,20 @@ public QueryTokenStream visitQueryExpression(HqlParser.QueryExpressionContext ct builder.appendExpression(visit(ctx.withClause())); } - builder.append(visitOrderedQuery(ctx.orderedQuery(0), Sort.unsorted())); + List orderedQueries = ctx.orderedQuery(); + for (int i = 0; i < orderedQueries.size(); i++) { - for (int i = 1; i < ctx.orderedQuery().size(); i++) { + if (i != 0) { + builder.append(visit(ctx.setOperator(i - 1))); + } - builder.append(visit(ctx.setOperator(i - 1))); - builder.append(visit(ctx.orderedQuery(i))); + if (i == orderedQueries.size() - 1) { + builder.append(visitOrderedQuery(ctx.orderedQuery(i), this.sort)); + } else { + builder.append(visitOrderedQuery(ctx.orderedQuery(i), Sort.unsorted())); + } } - return builder; } @@ -81,7 +86,7 @@ public QueryTokenStream visitJoinPath(HqlParser.JoinPathContext ctx) { QueryTokenStream tokens = super.visitJoinPath(ctx); - if (ctx.variable() != null && !isSubquery(ctx)) { + if (ctx.variable() != null && !isSubquery(ctx)) { transformerSupport.registerAlias(tokens.getLast()); } @@ -93,7 +98,7 @@ public QueryTokenStream visitJoinSubquery(HqlParser.JoinSubqueryContext ctx) { QueryTokenStream tokens = super.visitJoinSubquery(ctx); - if (ctx.variable() != null && !tokens.isEmpty() && !isSubquery(ctx)) { + if (ctx.variable() != null && !tokens.isEmpty() && !isSubquery(ctx)) { transformerSupport.registerAlias(tokens.getLast()); } @@ -105,7 +110,7 @@ public QueryTokenStream visitVariable(HqlParser.VariableContext ctx) { QueryTokenStream tokens = super.visitVariable(ctx); - if (ctx.identifier() != null && !tokens.isEmpty() && !isSubquery(ctx)) { + if (ctx.identifier() != null && !tokens.isEmpty() && !isSubquery(ctx)) { transformerSupport.registerAlias(tokens.getLast()); } 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 e0de136bc8..52d85ddd81 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 @@ -98,7 +98,7 @@ void applyCountToMoreComplexQuery() { } @Test - void applyCountToAlreadySorteQuery() { + void applyCountToAlreadySortedQuery() { // given var original = "SELECT e FROM Employee e where e.name = :name ORDER BY e.modified_date"; @@ -762,10 +762,12 @@ 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 source = "SELECT tb FROM Test tb WHERE (tb.type='A') UNION SELECT tb FROM Test tb WHERE (tb.type='B') UNION SELECT tb FROM Test tb WHERE (tb.type='C')"; 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"); + assertThat(target).isEqualTo("SELECT tb FROM Test tb WHERE (tb.type = 'A') " // + + "UNION SELECT tb FROM Test tb WHERE (tb.type = 'B') " // + + "UNION SELECT tb FROM Test tb WHERE (tb.type = 'C') order by tb.Type asc"); } static Stream queriesWithReservedWordsAsIdentifiers() { 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 44350d2f0a..e19036dace 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 @@ -705,11 +705,9 @@ void applySortingAccountsForNativeWindowFunction() { .isEqualTo("select dense_rank() over (order by lastname) from user u order by u.lastname, u.age desc"); // partition by + order by in over clause - assertThat( - createQueryFor( - "select dense_rank() over (partition by active, age order by lastname range between 1.0 preceding and 1.0 following) from user u", - sort)) - .isEqualTo( + assertThat(createQueryFor( + "select dense_rank() over (partition by active, age order by lastname range between 1.0 preceding and 1.0 following) from user u", + sort)).isEqualTo( "select dense_rank() over (partition by active, age order by lastname range between 1.0 preceding and 1.0 following) from user u order by u.age desc"); // partition by + order by in over clause + order by at the end @@ -1068,19 +1066,23 @@ void createsCountQueryUsingAliasCorrectly() { @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 source = "SELECT tb FROM Test tb WHERE (tb.type='A') UNION SELECT tb FROM Test tb WHERE (tb.type='B') UNION SELECT tb FROM Test tb WHERE (tb.type='C')"; 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"); + + assertThat(target).isEqualTo("SELECT tb FROM Test tb WHERE (tb.type = 'A') " // + + "UNION SELECT tb FROM Test tb WHERE (tb.type = 'B') " // + + "UNION SELECT tb FROM Test tb WHERE (tb.type = 'C') order by tb.Type asc"); } @ParameterizedTest // GH-3427 - @ValueSource(strings = {"", "res"}) + @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)) { + 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); } @@ -1088,10 +1090,10 @@ void sortShouldBeAppendedToSubSelectWithSetOperatorInSubselect(String alias) { 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)); + Pattern pattern = Pattern.compile("order by"); Matcher matcher = pattern.matcher(target); int count = 0; - while(matcher.find()) { + while (matcher.find()) { count++; } assertThat(count).describedAs("Found order by clause more than once in: \n%s", it).isOne(); diff --git a/spring-data-jpa/src/test/java/org/springframework/data/jpa/repository/query/JpqlQueryTransformerTests.java b/spring-data-jpa/src/test/java/org/springframework/data/jpa/repository/query/JpqlQueryTransformerTests.java index eb04b8de07..c71079a807 100644 --- a/spring-data-jpa/src/test/java/org/springframework/data/jpa/repository/query/JpqlQueryTransformerTests.java +++ b/spring-data-jpa/src/test/java/org/springframework/data/jpa/repository/query/JpqlQueryTransformerTests.java @@ -198,16 +198,6 @@ void applySortingAccountsForNewlinesInSubselect() { Sort sort = Sort.by(Sort.Order.desc("age")); - // - // - // - // - // - // - // - // - // - // assertThat(newParser(""" select u from user u