Skip to content

Commit

Permalink
Polishing.
Browse files Browse the repository at this point in the history 
Ensure ordering of the last query only, applying ordering after the set operation.

Original pull request: #3429
See #3427
  • Loading branch information
@mp911de
mp911de committed Aug 2, 2024
1 parent b02081b commit 3191b88
Show file tree
Hide file tree
Showing 4 changed files with 35 additions and 36 deletions.
25 changes: 15 additions & 10 deletions ...rc/main/java/org/springframework/data/jpa/repository/query/HqlSortedQueryTransformer.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}

Expand All @@ -59,15 +59,20 @@ public QueryTokenStream visitQueryExpression(HqlParser.QueryExpressionContext ct
builder.appendExpression(visit(ctx.withClause()));
}

builder.append(visitOrderedQuery(ctx.orderedQuery(0), Sort.unsorted()));
List<HqlParser.OrderedQueryContext> 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;
}

Expand All @@ -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());
}

Expand All @@ -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());
}

Expand All @@ -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());
}

Expand Down
8 changes: 5 additions & 3 deletions ...src/test/java/org/springframework/data/jpa/repository/query/EqlQueryTransformerTests.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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";
Expand Down Expand Up @@ -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<Arguments> queriesWithReservedWordsAsIdentifiers() {
Expand Down
28 changes: 15 additions & 13 deletions ...src/test/java/org/springframework/data/jpa/repository/query/HqlQueryTransformerTests.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -1068,30 +1066,34 @@ 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);
}

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));
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();
Expand Down
10 changes: 0 additions & 10 deletions ...rc/test/java/org/springframework/data/jpa/repository/query/JpqlQueryTransformerTests.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -198,16 +198,6 @@ void applySortingAccountsForNewlinesInSubselect() {

Sort sort = Sort.by(Sort.Order.desc("age"));

//
//
//
//
//
//
//
//
//
//
assertThat(newParser("""
select u
from user u
Expand Down

0 comments on commit 3191b88

Please sign in to comment.