From a6e33ad1b9bc0c718cd1fd0beca5e39fc12b2e00 Mon Sep 17 00:00:00 2001 From: "Greg L. Turnquist" Date: Mon, 22 May 2023 16:34:21 -0500 Subject: [PATCH] Properly handle Sort's that start with a join alias. JOIN clauses can have aliases as well, despite not using an AS reserved word. The HQL query parser needs to handle this. See #2960, #1066, #664 Original Pull Request: 2967 --- .../repository/query/HqlQueryTransformer.java | 24 +++++++++++++++++++ .../query/JpaQueryTransformerSupport.java | 7 +++++- .../query/JpqlQueryTransformer.java | 10 ++++++++ .../query/HqlQueryTransformerTests.java | 24 +++++++++++++++++-- .../query/JpqlQueryTransformerTests.java | 24 +++++++++++++++++-- 5 files changed, 84 insertions(+), 5 deletions(-) diff --git a/spring-data-jpa/src/main/java/org/springframework/data/jpa/repository/query/HqlQueryTransformer.java b/spring-data-jpa/src/main/java/org/springframework/data/jpa/repository/query/HqlQueryTransformer.java index 6afba0f1f..7d6be749d 100644 --- a/spring-data-jpa/src/main/java/org/springframework/data/jpa/repository/query/HqlQueryTransformer.java +++ b/spring-data-jpa/src/main/java/org/springframework/data/jpa/repository/query/HqlQueryTransformer.java @@ -282,6 +282,30 @@ class HqlQueryTransformer extends HqlQueryRenderer { return tokens; } + @Override + public List visitJoinPath(HqlParser.JoinPathContext ctx) { + + List tokens = super.visitJoinPath(ctx); + + if (ctx.variable() != null) { + transformerSupport.registerAlias(tokens.get(tokens.size() - 1).getToken()); + } + + return tokens; + } + + @Override + public List visitJoinSubquery(HqlParser.JoinSubqueryContext ctx) { + + List tokens = super.visitJoinSubquery(ctx); + + if (ctx.variable() != null) { + transformerSupport.registerAlias(tokens.get(tokens.size() - 1).getToken()); + } + + return tokens; + } + @Override public List visitAlias(HqlParser.AliasContext ctx) { 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 20b6fe920..bce349e87 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 @@ -131,11 +131,16 @@ class JpaQueryTransformerSupport { return false; } - // If the Sort references an alias + // If the Sort references an alias directly if (projectionAliases.contains(order.getProperty())) { return false; } + // If the Sort property starts with an alias + if (projectionAliases.stream().anyMatch(alias -> order.getProperty().startsWith(alias))) { + return false; + } + return true; } } diff --git a/spring-data-jpa/src/main/java/org/springframework/data/jpa/repository/query/JpqlQueryTransformer.java b/spring-data-jpa/src/main/java/org/springframework/data/jpa/repository/query/JpqlQueryTransformer.java index a89abcb68..a9aeec36a 100644 --- a/spring-data-jpa/src/main/java/org/springframework/data/jpa/repository/query/JpqlQueryTransformer.java +++ b/spring-data-jpa/src/main/java/org/springframework/data/jpa/repository/query/JpqlQueryTransformer.java @@ -219,6 +219,16 @@ class JpqlQueryTransformer extends JpqlQueryRenderer { return tokens; } + @Override + public List visitJoin(JpqlParser.JoinContext ctx) { + + List tokens = super.visitJoin(ctx); + + transformerSupport.registerAlias(tokens.get(tokens.size() - 1).getToken()); + + return tokens; + } + @Override public List visitConstructor_expression(JpqlParser.Constructor_expressionContext ctx) { 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 e11dec1eb..460d8f456 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 @@ -245,12 +245,12 @@ class HqlQueryTransformerTests { """)).isEqualTo("o"); } - @Test // DATAJPA-252 + @Test // DATAJPA-252, GH-664, GH-1066, GH-2960 void doesNotPrefixOrderReferenceIfOuterJoinAliasDetected() { String query = "select p from Person p left join p.address address"; Sort sort = Sort.by("address.city"); - assertThat(createQueryFor(query, sort)).endsWith("order by p.address.city asc"); + assertThat(createQueryFor(query, sort)).endsWith("order by address.city asc"); } @Test // DATAJPA-252 @@ -986,6 +986,26 @@ class HqlQueryTransformerTests { assertThat(createQueryFor(query, Sort.unsorted())).isEqualToIgnoringWhitespace(query); } + @Test // GH-664, GH-1066, GH-2960 + void sortingRecognizesJoinAliases() { + + String query = "select p from Customer c join c.productOrder p where p.delayed = true"; + + assertThat(createQueryFor(query, Sort.by(Sort.Order.desc("lastName")))).isEqualToIgnoringWhitespace(""" + select p from Customer c + join c.productOrder p + where p.delayed = true + order by c.lastName desc + """); + + assertThat(createQueryFor(query, Sort.by(Sort.Order.desc("p.lineItems")))).isEqualToIgnoringWhitespace(""" + select p from Customer c + join c.productOrder p + where p.delayed = true + order by p.lineItems desc + """); + } + private void assertCountQuery(String originalQuery, String countQuery) { assertThat(createCountQueryFor(originalQuery)).isEqualTo(countQuery); } 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 bd2b6a469..2559c37ed 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 @@ -235,12 +235,12 @@ class JpqlQueryTransformerTests { """)).isEqualTo("o"); } - @Test // DATAJPA-252 + @Test // DATAJPA-252, GH-664, GH-1066, GH-2960 void doesNotPrefixOrderReferenceIfOuterJoinAliasDetected() { String query = "select p from Person p left join p.address address"; Sort sort = Sort.by("address.city"); - assertThat(createQueryFor(query, sort)).endsWith("order by p.address.city asc"); + assertThat(createQueryFor(query, sort)).endsWith("order by address.city asc"); } @Test // DATAJPA-252 @@ -742,6 +742,26 @@ class JpqlQueryTransformerTests { """, relationshipName, joinAlias, joinAlias)); } + @Test // GH-664, GH-1066, GH-2960 + void sortingRecognizesJoinAliases() { + + String query = "select p from Customer c join c.productOrder p where p.delayed = true"; + + assertThat(createQueryFor(query, Sort.by(Sort.Order.desc("lastName")))).isEqualToIgnoringWhitespace(""" + select p from Customer c + join c.productOrder p + where p.delayed = true + order by c.lastName desc + """); + + assertThat(createQueryFor(query, Sort.by(Sort.Order.desc("p.lineItems")))).isEqualToIgnoringWhitespace(""" + select p from Customer c + join c.productOrder p + where p.delayed = true + order by p.lineItems desc + """); + } + static Stream queriesWithReservedWordsAsIdentifiers() { return Stream.of( //