From eb3920d100ced6a6e1569fad9c72a9a3084ab4c1 Mon Sep 17 00:00:00 2001 From: "Greg L. Turnquist" Date: Mon, 20 Mar 2023 15:00:27 -0500 Subject: [PATCH 1/4] Create new branch. --- pom.xml | 2 +- spring-data-envers/pom.xml | 4 ++-- spring-data-jpa-distribution/pom.xml | 2 +- spring-data-jpa/pom.xml | 4 ++-- 4 files changed, 6 insertions(+), 6 deletions(-) diff --git a/pom.xml b/pom.xml index c77686453f..00dcb23f86 100644 --- a/pom.xml +++ b/pom.xml @@ -5,7 +5,7 @@ org.springframework.data spring-data-jpa-parent - 3.1.0-SNAPSHOT + 3.1.0-gh-2864-SNAPSHOT pom Spring Data JPA Parent diff --git a/spring-data-envers/pom.xml b/spring-data-envers/pom.xml index db915d7c3b..ea3bdb0629 100755 --- a/spring-data-envers/pom.xml +++ b/spring-data-envers/pom.xml @@ -5,12 +5,12 @@ org.springframework.data spring-data-envers - 3.1.0-SNAPSHOT + 3.1.0-gh-2864-SNAPSHOT org.springframework.data spring-data-jpa-parent - 3.1.0-SNAPSHOT + 3.1.0-gh-2864-SNAPSHOT ../pom.xml diff --git a/spring-data-jpa-distribution/pom.xml b/spring-data-jpa-distribution/pom.xml index a5cb2f09b5..46f8ea3d24 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.1.0-SNAPSHOT + 3.1.0-gh-2864-SNAPSHOT ../pom.xml diff --git a/spring-data-jpa/pom.xml b/spring-data-jpa/pom.xml index 27313e9e3c..e5a4e12ddc 100644 --- a/spring-data-jpa/pom.xml +++ b/spring-data-jpa/pom.xml @@ -6,7 +6,7 @@ org.springframework.data spring-data-jpa - 3.1.0-SNAPSHOT + 3.1.0-gh-2864-SNAPSHOT Spring Data JPA Spring Data module for JPA repositories. @@ -15,7 +15,7 @@ org.springframework.data spring-data-jpa-parent - 3.1.0-SNAPSHOT + 3.1.0-gh-2864-SNAPSHOT ../pom.xml From 718cfa853ee4c3a1715b531810a2231ca495fe1c Mon Sep 17 00:00:00 2001 From: "Greg L. Turnquist" Date: Mon, 20 Mar 2023 16:14:56 -0500 Subject: [PATCH 2/4] Recognize RIGHT, LEFT, OUTER, INNER and FULL as HQL reserved words. RIGHT, LEFT, OUTER, INNER, and FULL are HQL tokens. This means they also need to be recognized as potential reserved words and thus possibly identifiers. This can show up if someone, for example, uses "right" as the name of a relationship in a JPA entity. Resolves #2864. --- .../data/jpa/repository/query/Hql.g4 | 10 +- .../query/HqlQueryTransformerTests.java | 115 ++++++++++++++++++ 2 files changed, 120 insertions(+), 5 deletions(-) diff --git a/spring-data-jpa/src/main/antlr4/org/springframework/data/jpa/repository/query/Hql.g4 b/spring-data-jpa/src/main/antlr4/org/springframework/data/jpa/repository/query/Hql.g4 index 253caafc11..b07504c316 100644 --- a/spring-data-jpa/src/main/antlr4/org/springframework/data/jpa/repository/query/Hql.g4 +++ b/spring-data-jpa/src/main/antlr4/org/springframework/data/jpa/repository/query/Hql.g4 @@ -700,7 +700,7 @@ reservedWord | FOR | FORMAT | FROM -// | FULL + | FULL | FUNCTION | GROUP | GROUPS @@ -712,7 +712,7 @@ reservedWord | IN | INDEX | INDICES -// | INNER + | INNER | INSERT | INSTANT | INTERSECT @@ -722,7 +722,7 @@ reservedWord | KEY | LAST | LEADING -// | LEFT + | LEFT | LIKE | LIMIT | LIST @@ -760,7 +760,7 @@ reservedWord | OR | ORDER | OTHERS -// | OUTER + | OUTER | OVER | OVERFLOW | OVERLAY @@ -773,7 +773,7 @@ reservedWord | QUARTER | RANGE | RESPECT -// | RIGHT + | RIGHT | ROLLUP | ROW | ROWS 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 cae0d71d31..009c8e6eac 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 @@ -808,6 +808,121 @@ void countQueryShouldWorkEvenWithoutExplicitAlias() { "select count(b) FROM BookError b WHERE portal = :portal"); } + @Test // GH-2864 + void usingRightAsARelationshipNameShouldWork() { + + HqlQueryParser.parseQuery(""" + select u + from UserAccountEntity u + join fetch u.lossInspectorLimitConfiguration lil + join fetch u.companyTeam ct + where exists ( + select iu + from UserAccountEntity iu + join iu.roles u2r + join u2r.role r + join r.rights r2r + join r2r.right rt + where + rt.code = :rightCode + and iu = u + ) + and ct.id = :teamId + """); + } + + @Test // GH-2864 + void usingLeftAsARelationshipNameShouldWork() { + + HqlQueryParser.parseQuery(""" + select u + from UserAccountEntity u + join fetch u.lossInspectorLimitConfiguration lil + join fetch u.companyTeam ct + where exists ( + select iu + from UserAccountEntity iu + join iu.roles u2r + join u2r.role r + join r.rights r2r + join r2r.left lt + where + lt.code = :rightCode + and iu = u + ) + and ct.id = :teamId + """); + } + + @Test // GH-2864 + void usingOuterAsARelationshipNameShouldWork() { + + HqlQueryParser.parseQuery(""" + select u + from UserAccountEntity u + join fetch u.lossInspectorLimitConfiguration lil + join fetch u.companyTeam ct + where exists ( + select iu + from UserAccountEntity iu + join iu.roles u2r + join u2r.role r + join r.rights r2r + join r2r.outer ou + where + ou.code = :rightCode + and iu = u + ) + and ct.id = :teamId + """); + } + + @Test // GH-2864 + void usingFullAsARelationshipNameShouldWork() { + + HqlQueryParser.parseQuery(""" + select u + from UserAccountEntity u + join fetch u.lossInspectorLimitConfiguration lil + join fetch u.companyTeam ct + where exists ( + select iu + from UserAccountEntity iu + join iu.roles u2r + join u2r.role r + join r.rights r2r + join r2r.full fu + where + fu.code = :rightCode + and iu = u + ) + and ct.id = :teamId + """); + } + + @Test // GH-2864 + void usingInnerAsARelationshipNameShouldWork() { + + HqlQueryParser.parseQuery(""" + select u + from UserAccountEntity u + join fetch u.lossInspectorLimitConfiguration lil + join fetch u.companyTeam ct + where exists ( + select iu + from UserAccountEntity iu + join iu.roles u2r + join u2r.role r + join r.rights r2r + join r2r.inner in + where + in.code = :rightCode + and iu = u + ) + and ct.id = :teamId + """); + } + private void assertCountQuery(String originalQuery, String countQuery) { assertThat(createCountQueryFor(originalQuery)).isEqualTo(countQuery); } From cdfe673eec5a5e57c2aa8cb9d75da428acd771c4 Mon Sep 17 00:00:00 2001 From: "Greg L. Turnquist" Date: Wed, 22 Mar 2023 09:01:42 -0500 Subject: [PATCH 3/4] JPQL parser must support LEFT, INNER, and OUTER as possible identifiers. --- .../data/jpa/repository/query/Jpql.g4 | 9 +- .../query/JpqlQueryTransformerTests.java | 115 ++++++++++++++++++ 2 files changed, 121 insertions(+), 3 deletions(-) diff --git a/spring-data-jpa/src/main/antlr4/org/springframework/data/jpa/repository/query/Jpql.g4 b/spring-data-jpa/src/main/antlr4/org/springframework/data/jpa/repository/query/Jpql.g4 index dc96a6ea46..fb12b38efa 100644 --- a/spring-data-jpa/src/main/antlr4/org/springframework/data/jpa/repository/query/Jpql.g4 +++ b/spring-data-jpa/src/main/antlr4/org/springframework/data/jpa/repository/query/Jpql.g4 @@ -596,9 +596,12 @@ trim_character identification_variable : IDENTIFICATION_VARIABLE - | ORDER // Gap in the spec requires supporting 'Order' as an entity name - | COUNT // Gap in the spec requires supporting 'count' as a possible name - | KEY // Gap in the sepc requires supported 'key' as a possible name + | ORDER + | COUNT + | KEY + | LEFT + | INNER + | OUTER | spel_expression // we use various SpEL expressions in our queries ; 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 d63cbd26d8..aa2d62b420 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 @@ -679,6 +679,121 @@ void queryParserPicksCorrectAliasAmidstMultipleAlises() { assertThat(alias("select u from User as u left join u.roles as r")).isEqualTo("u"); } + @Test // GH-2864 + void usingRightAsARelationshipNameShouldWork() { + + JpqlQueryParser.parseQuery(""" + select u + from UserAccountEntity u + join u.lossInspectorLimitConfiguration lil + join u.companyTeam ct + where exists ( + select iu + from UserAccountEntity iu + join iu.roles u2r + join u2r.role r + join r.rights r2r + join r2r.right rt + where + rt.code = :rightCode + and iu = u + ) + and ct.id = :teamId + """); + } + + @Test // GH-2864 + void usingLeftAsARelationshipNameShouldWork() { + + JpqlQueryParser.parseQuery(""" + select u + from UserAccountEntity u + join u.lossInspectorLimitConfiguration lil + join u.companyTeam ct + where exists ( + select iu + from UserAccountEntity iu + join iu.roles u2r + join u2r.role r + join r.rights r2r + join r2r.left lt + where + lt.code = :rightCode + and iu = u + ) + and ct.id = :teamId + """); + } + + @Test // GH-2864 + void usingOuterAsARelationshipNameShouldWork() { + + JpqlQueryParser.parseQuery(""" + select u + from UserAccountEntity u + join u.lossInspectorLimitConfiguration lil + join u.companyTeam ct + where exists ( + select iu + from UserAccountEntity iu + join iu.roles u2r + join u2r.role r + join r.rights r2r + join r2r.outer ou + where + ou.code = :rightCode + and iu = u + ) + and ct.id = :teamId + """); + } + + @Test // GH-2864 + void usingFullAsARelationshipNameShouldWork() { + + JpqlQueryParser.parseQuery(""" + select u + from UserAccountEntity u + join u.lossInspectorLimitConfiguration lil + join u.companyTeam ct + where exists ( + select iu + from UserAccountEntity iu + join iu.roles u2r + join u2r.role r + join r.rights r2r + join r2r.full fu + where + fu.code = :rightCode + and iu = u + ) + and ct.id = :teamId + """); + } + + @Test // GH-2864 + void usingInnerAsARelationshipNameShouldWork() { + + JpqlQueryParser.parseQuery(""" + select u + from UserAccountEntity u + join u.lossInspectorLimitConfiguration lil + join u.companyTeam ct + where exists ( + select iu + from UserAccountEntity iu + join iu.roles u2r + join u2r.role r + join r.rights r2r + join r2r.inner inr + where + inr.code = :rightCode + and iu = u + ) + and ct.id = :teamId + """); + } + private void assertCountQuery(String originalQuery, String countQuery) { assertThat(createCountQueryFor(originalQuery)).isEqualTo(countQuery); } From b299a732f8ea6bb54964232faa1ae053a8c1f144 Mon Sep 17 00:00:00 2001 From: "Greg L. Turnquist" Date: Wed, 22 Mar 2023 12:50:04 -0500 Subject: [PATCH 4/4] Migrate these test cases to @ParameterizedTest --- .../query/HqlQueryTransformerTests.java | 113 +++--------------- .../query/JpqlQueryTransformerTests.java | 113 +++--------------- 2 files changed, 36 insertions(+), 190 deletions(-) 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 009c8e6eac..defb5aafd2 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 @@ -18,9 +18,13 @@ import static org.assertj.core.api.Assertions.*; import java.util.regex.Pattern; +import java.util.stream.Stream; import org.assertj.core.api.SoftAssertions; import org.junit.jupiter.api.Test; +import org.junit.jupiter.params.ParameterizedTest; +import org.junit.jupiter.params.provider.Arguments; +import org.junit.jupiter.params.provider.MethodSource; import org.springframework.dao.InvalidDataAccessApiUsageException; import org.springframework.data.domain.Sort; import org.springframework.data.jpa.domain.JpaSort; @@ -808,10 +812,11 @@ void countQueryShouldWorkEvenWithoutExplicitAlias() { "select count(b) FROM BookError b WHERE portal = :portal"); } - @Test // GH-2864 - void usingRightAsARelationshipNameShouldWork() { + @ParameterizedTest + @MethodSource("queriesWithReservedWordsAsIdentifiers") // GH-2864 + void usingReservedWordAsRelationshipNameShouldWork(String relationshipName, String joinAlias) { - HqlQueryParser.parseQuery(""" + HqlQueryParser.parseQuery(String.format(""" select u from UserAccountEntity u join fetch u.lossInspectorLimitConfiguration lil @@ -822,105 +827,23 @@ where exists ( join iu.roles u2r join u2r.role r join r.rights r2r - join r2r.right rt + join r2r.%s %s where - rt.code = :rightCode + %s.code = :rightCode and iu = u ) and ct.id = :teamId - """); + """, relationshipName, joinAlias, joinAlias)); } - @Test // GH-2864 - void usingLeftAsARelationshipNameShouldWork() { + static Stream queriesWithReservedWordsAsIdentifiers() { - HqlQueryParser.parseQuery(""" - select u - from UserAccountEntity u - join fetch u.lossInspectorLimitConfiguration lil - join fetch u.companyTeam ct - where exists ( - select iu - from UserAccountEntity iu - join iu.roles u2r - join u2r.role r - join r.rights r2r - join r2r.left lt - where - lt.code = :rightCode - and iu = u - ) - and ct.id = :teamId - """); - } - - @Test // GH-2864 - void usingOuterAsARelationshipNameShouldWork() { - - HqlQueryParser.parseQuery(""" - select u - from UserAccountEntity u - join fetch u.lossInspectorLimitConfiguration lil - join fetch u.companyTeam ct - where exists ( - select iu - from UserAccountEntity iu - join iu.roles u2r - join u2r.role r - join r.rights r2r - join r2r.outer ou - where - ou.code = :rightCode - and iu = u - ) - and ct.id = :teamId - """); - } - - @Test // GH-2864 - void usingFullAsARelationshipNameShouldWork() { - - HqlQueryParser.parseQuery(""" - select u - from UserAccountEntity u - join fetch u.lossInspectorLimitConfiguration lil - join fetch u.companyTeam ct - where exists ( - select iu - from UserAccountEntity iu - join iu.roles u2r - join u2r.role r - join r.rights r2r - join r2r.full fu - where - fu.code = :rightCode - and iu = u - ) - and ct.id = :teamId - """); - } - - @Test // GH-2864 - void usingInnerAsARelationshipNameShouldWork() { - - HqlQueryParser.parseQuery(""" - select u - from UserAccountEntity u - join fetch u.lossInspectorLimitConfiguration lil - join fetch u.companyTeam ct - where exists ( - select iu - from UserAccountEntity iu - join iu.roles u2r - join u2r.role r - join r.rights r2r - join r2r.inner in - where - in.code = :rightCode - and iu = u - ) - and ct.id = :teamId - """); + return Stream.of( // + Arguments.of("right", "rt"), // + Arguments.of("left", "lt"), // + Arguments.of("outer", "ou"), // + Arguments.of("full", "full"), // + Arguments.of("inner", "inr")); } private void assertCountQuery(String originalQuery, String 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 aa2d62b420..8ebcfa83e0 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 @@ -18,9 +18,13 @@ import static org.assertj.core.api.Assertions.*; import java.util.regex.Pattern; +import java.util.stream.Stream; import org.assertj.core.api.SoftAssertions; import org.junit.jupiter.api.Test; +import org.junit.jupiter.params.ParameterizedTest; +import org.junit.jupiter.params.provider.Arguments; +import org.junit.jupiter.params.provider.MethodSource; import org.springframework.dao.InvalidDataAccessApiUsageException; import org.springframework.data.domain.Sort; import org.springframework.data.jpa.domain.JpaSort; @@ -679,10 +683,11 @@ void queryParserPicksCorrectAliasAmidstMultipleAlises() { assertThat(alias("select u from User as u left join u.roles as r")).isEqualTo("u"); } - @Test // GH-2864 - void usingRightAsARelationshipNameShouldWork() { + @ParameterizedTest + @MethodSource("queriesWithReservedWordsAsIdentifiers") // GH-2864 + void usingReservedWordAsRelationshipNameShouldWork(String relationshipName, String joinAlias) { - JpqlQueryParser.parseQuery(""" + JpqlQueryParser.parseQuery(String.format(""" select u from UserAccountEntity u join u.lossInspectorLimitConfiguration lil @@ -693,105 +698,23 @@ where exists ( join iu.roles u2r join u2r.role r join r.rights r2r - join r2r.right rt - where - rt.code = :rightCode - and iu = u - ) - and ct.id = :teamId - """); - } - - @Test // GH-2864 - void usingLeftAsARelationshipNameShouldWork() { - - JpqlQueryParser.parseQuery(""" - select u - from UserAccountEntity u - join u.lossInspectorLimitConfiguration lil - join u.companyTeam ct - where exists ( - select iu - from UserAccountEntity iu - join iu.roles u2r - join u2r.role r - join r.rights r2r - join r2r.left lt - where - lt.code = :rightCode - and iu = u - ) - and ct.id = :teamId - """); - } - - @Test // GH-2864 - void usingOuterAsARelationshipNameShouldWork() { - - JpqlQueryParser.parseQuery(""" - select u - from UserAccountEntity u - join u.lossInspectorLimitConfiguration lil - join u.companyTeam ct - where exists ( - select iu - from UserAccountEntity iu - join iu.roles u2r - join u2r.role r - join r.rights r2r - join r2r.outer ou - where - ou.code = :rightCode - and iu = u - ) - and ct.id = :teamId - """); - } - - @Test // GH-2864 - void usingFullAsARelationshipNameShouldWork() { - - JpqlQueryParser.parseQuery(""" - select u - from UserAccountEntity u - join u.lossInspectorLimitConfiguration lil - join u.companyTeam ct - where exists ( - select iu - from UserAccountEntity iu - join iu.roles u2r - join u2r.role r - join r.rights r2r - join r2r.full fu + join r2r.inner inr where - fu.code = :rightCode + inr.code = :rightCode and iu = u ) and ct.id = :teamId - """); + """, relationshipName, joinAlias, joinAlias)); } - @Test // GH-2864 - void usingInnerAsARelationshipNameShouldWork() { + static Stream queriesWithReservedWordsAsIdentifiers() { - JpqlQueryParser.parseQuery(""" - select u - from UserAccountEntity u - join u.lossInspectorLimitConfiguration lil - join u.companyTeam ct - where exists ( - select iu - from UserAccountEntity iu - join iu.roles u2r - join u2r.role r - join r.rights r2r - join r2r.inner inr - where - inr.code = :rightCode - and iu = u - ) - and ct.id = :teamId - """); + return Stream.of( // + Arguments.of("right", "rt"), // + Arguments.of("left", "lt"), // + Arguments.of("outer", "ou"), // + Arguments.of("full", "full"), // + Arguments.of("inner", "inr")); } private void assertCountQuery(String originalQuery, String countQuery) {