From e1be294004d9ca8350d32c0600563b1e8e195efb Mon Sep 17 00:00:00 2001 From: kssumin Date: Sat, 18 Oct 2025 12:25:41 +0900 Subject: [PATCH 1/3] Prevent sorting and count queries for non-SELECT statements. Add statement type detection to QueryEnhancer implementations to validate operations. QueryEnhancer now throws IllegalStateException when attempting to create count queries or apply sorting to INSERT, UPDATE, DELETE, or MERGE statements, as these operations are only valid for SELECT queries. Signed-off-by: kssumin Original pull request: #4052 Closes #2856 --- .../query/DefaultQueryEnhancer.java | 2 +- .../query/EqlQueryIntrospector.java | 28 +++++- .../query/HibernateQueryInformation.java | 17 +++- .../query/HqlQueryIntrospector.java | 28 +++++- .../query/JSqlParserQueryEnhancer.java | 16 +++- .../repository/query/JpaQueryEnhancer.java | 15 +++ .../query/JpqlQueryIntrospector.java | 28 +++++- .../repository/query/QueryInformation.java | 64 +++++++++++++ .../JSqlParserQueryEnhancerUnitTests.java | 54 ++++++++++- .../query/QueryEnhancerUnitTests.java | 92 +++++++++++++++++-- 10 files changed, 321 insertions(+), 23 deletions(-) diff --git a/spring-data-jpa/src/main/java/org/springframework/data/jpa/repository/query/DefaultQueryEnhancer.java b/spring-data-jpa/src/main/java/org/springframework/data/jpa/repository/query/DefaultQueryEnhancer.java index 456c3139b3..0092dd5e72 100644 --- a/spring-data-jpa/src/main/java/org/springframework/data/jpa/repository/query/DefaultQueryEnhancer.java +++ b/spring-data-jpa/src/main/java/org/springframework/data/jpa/repository/query/DefaultQueryEnhancer.java @@ -27,7 +27,7 @@ class DefaultQueryEnhancer implements QueryEnhancer { private final QueryProvider query; private final boolean hasConstructorExpression; - private final @Nullable String alias; + private final @Nullable String alias; private final String projection; public DefaultQueryEnhancer(QueryProvider query) { diff --git a/spring-data-jpa/src/main/java/org/springframework/data/jpa/repository/query/EqlQueryIntrospector.java b/spring-data-jpa/src/main/java/org/springframework/data/jpa/repository/query/EqlQueryIntrospector.java index 71f65523b8..5e78aebb8c 100644 --- a/spring-data-jpa/src/main/java/org/springframework/data/jpa/repository/query/EqlQueryIntrospector.java +++ b/spring-data-jpa/src/main/java/org/springframework/data/jpa/repository/query/EqlQueryIntrospector.java @@ -28,6 +28,7 @@ * * @author Mark Paluch * @author Christoph Strobl + * @author kssumin */ @SuppressWarnings("UnreachableCode") class EqlQueryIntrospector extends EqlBaseVisitor implements ParsedQueryIntrospector { @@ -38,11 +39,36 @@ class EqlQueryIntrospector extends EqlBaseVisitor implements ParsedQueryIn private @Nullable List projection; private boolean projectionProcessed; private boolean hasConstructorExpression = false; + private QueryInformation.StatementType statementType = QueryInformation.StatementType.SELECT; @Override public QueryInformation getParsedQueryInformation() { return new QueryInformation(primaryFromAlias, projection == null ? Collections.emptyList() : projection, - hasConstructorExpression); + hasConstructorExpression, statementType); + } + + @Override + public Void visitSelectQuery(EqlParser.SelectQueryContext ctx) { + statementType = QueryInformation.StatementType.SELECT; + return super.visitSelectQuery(ctx); + } + + @Override + public Void visitFromQuery(EqlParser.FromQueryContext ctx) { + statementType = QueryInformation.StatementType.SELECT; + return super.visitFromQuery(ctx); + } + + @Override + public Void visitUpdate_statement(EqlParser.Update_statementContext ctx) { + statementType = QueryInformation.StatementType.UPDATE; + return super.visitUpdate_statement(ctx); + } + + @Override + public Void visitDelete_statement(EqlParser.Delete_statementContext ctx) { + statementType = QueryInformation.StatementType.DELETE; + return super.visitDelete_statement(ctx); } @Override diff --git a/spring-data-jpa/src/main/java/org/springframework/data/jpa/repository/query/HibernateQueryInformation.java b/spring-data-jpa/src/main/java/org/springframework/data/jpa/repository/query/HibernateQueryInformation.java index e1ea9173d1..1ea4e85ca8 100644 --- a/spring-data-jpa/src/main/java/org/springframework/data/jpa/repository/query/HibernateQueryInformation.java +++ b/spring-data-jpa/src/main/java/org/springframework/data/jpa/repository/query/HibernateQueryInformation.java @@ -24,28 +24,35 @@ * * @author Mark Paluch * @author Oscar Fanchin + * @author kssumin * @since 3.5 */ class HibernateQueryInformation extends QueryInformation { private final boolean hasCte; - + private final boolean hasFromFunction; - public HibernateQueryInformation(@Nullable String alias, List projection, - boolean hasConstructorExpression, boolean hasCte,boolean hasFromFunction) { + boolean hasConstructorExpression, boolean hasCte, boolean hasFromFunction) { super(alias, projection, hasConstructorExpression); this.hasCte = hasCte; this.hasFromFunction = hasFromFunction; } + public HibernateQueryInformation(@Nullable String alias, List projection, + boolean hasConstructorExpression, StatementType statementType, boolean hasCte, boolean hasFromFunction) { + super(alias, projection, hasConstructorExpression, statementType); + this.hasCte = hasCte; + this.hasFromFunction = hasFromFunction; + } + public boolean hasCte() { return hasCte; } - + public boolean hasFromFunction() { return hasFromFunction; } - + } diff --git a/spring-data-jpa/src/main/java/org/springframework/data/jpa/repository/query/HqlQueryIntrospector.java b/spring-data-jpa/src/main/java/org/springframework/data/jpa/repository/query/HqlQueryIntrospector.java index c32bf27d3f..aa8065e06c 100644 --- a/spring-data-jpa/src/main/java/org/springframework/data/jpa/repository/query/HqlQueryIntrospector.java +++ b/spring-data-jpa/src/main/java/org/springframework/data/jpa/repository/query/HqlQueryIntrospector.java @@ -30,6 +30,7 @@ * * @author Mark Paluch * @author Oscar Fanchin + * @author kssumin */ @SuppressWarnings({ "UnreachableCode", "ConstantValue" }) class HqlQueryIntrospector extends HqlBaseVisitor implements ParsedQueryIntrospector { @@ -42,11 +43,36 @@ class HqlQueryIntrospector extends HqlBaseVisitor implements ParsedQueryIn private boolean hasConstructorExpression = false; private boolean hasCte = false; private boolean hasFromFunction = false; + private QueryInformation.StatementType statementType = QueryInformation.StatementType.SELECT; @Override public HibernateQueryInformation getParsedQueryInformation() { return new HibernateQueryInformation(primaryFromAlias, projection == null ? Collections.emptyList() : projection, - hasConstructorExpression, hasCte, hasFromFunction); + hasConstructorExpression, statementType, hasCte, hasFromFunction); + } + + @Override + public Void visitSelectStatement(HqlParser.SelectStatementContext ctx) { + statementType = QueryInformation.StatementType.SELECT; + return super.visitSelectStatement(ctx); + } + + @Override + public Void visitInsertStatement(HqlParser.InsertStatementContext ctx) { + statementType = QueryInformation.StatementType.INSERT; + return super.visitInsertStatement(ctx); + } + + @Override + public Void visitUpdateStatement(HqlParser.UpdateStatementContext ctx) { + statementType = QueryInformation.StatementType.UPDATE; + return super.visitUpdateStatement(ctx); + } + + @Override + public Void visitDeleteStatement(HqlParser.DeleteStatementContext ctx) { + statementType = QueryInformation.StatementType.DELETE; + return super.visitDeleteStatement(ctx); } @Override diff --git a/spring-data-jpa/src/main/java/org/springframework/data/jpa/repository/query/JSqlParserQueryEnhancer.java b/spring-data-jpa/src/main/java/org/springframework/data/jpa/repository/query/JSqlParserQueryEnhancer.java index bf3f70df59..57294788ec 100644 --- a/spring-data-jpa/src/main/java/org/springframework/data/jpa/repository/query/JSqlParserQueryEnhancer.java +++ b/spring-data-jpa/src/main/java/org/springframework/data/jpa/repository/query/JSqlParserQueryEnhancer.java @@ -69,6 +69,7 @@ * @author Yanming Zhou * @author Christoph Strobl * @author Diego Pedregal + * @author kssumin * @since 2.7.0 */ public class JSqlParserQueryEnhancer implements QueryEnhancer { @@ -343,7 +344,16 @@ private String doApplySorting(Sort sort, @Nullable String alias) { String queryString = query.getQueryString(); Assert.hasText(queryString, "Query must not be null or empty"); - if (this.parsedType != ParsedType.SELECT || sort.isUnsorted()) { + if (this.parsedType != ParsedType.SELECT) { + if (!sort.isUnsorted()) { + throw new IllegalStateException(String.format( + "Cannot apply sorting to %s statement. Sorting is only supported for SELECT statements.", + this.parsedType)); + } + return queryString; + } + + if (sort.isUnsorted()) { return queryString; } @@ -383,7 +393,9 @@ private String applySorting(@Nullable Select selectStatement, Sort sort, @Nullab public String createCountQueryFor(@Nullable String countProjection) { if (this.parsedType != ParsedType.SELECT) { - return this.query.getQueryString(); + throw new IllegalStateException(String.format( + "Cannot derive count query for %s statement. Count queries are only supported for SELECT statements.", + this.parsedType)); } Assert.hasText(this.query.getQueryString(), "OriginalQuery must not be null or empty"); diff --git a/spring-data-jpa/src/main/java/org/springframework/data/jpa/repository/query/JpaQueryEnhancer.java b/spring-data-jpa/src/main/java/org/springframework/data/jpa/repository/query/JpaQueryEnhancer.java index 9c6e3e30bb..5bb082da78 100644 --- a/spring-data-jpa/src/main/java/org/springframework/data/jpa/repository/query/JpaQueryEnhancer.java +++ b/spring-data-jpa/src/main/java/org/springframework/data/jpa/repository/query/JpaQueryEnhancer.java @@ -41,6 +41,7 @@ * * @author Greg Turnquist * @author Mark Paluch + * @author kssumin * @since 3.1 * @see JpqlQueryParser * @see HqlQueryParser @@ -220,6 +221,13 @@ public DeclaredQuery getQuery() { @Override public String rewrite(QueryRewriteInformation rewriteInformation) { + + if (!queryInformation.isSelectStatement() && !rewriteInformation.getSort().isUnsorted()) { + throw new IllegalStateException(String.format( + "Cannot apply sorting to %s statement. Sorting is only supported for SELECT statements.", + queryInformation.getStatementType())); + } + return QueryRenderer.TokenRenderer.render( sortFunction.apply(rewriteInformation.getSort(), this.queryInformation, rewriteInformation.getReturnedType()) .visit(context)); @@ -232,6 +240,13 @@ public String rewrite(QueryRewriteInformation rewriteInformation) { */ @Override public String createCountQueryFor(@Nullable String countProjection) { + + if (!queryInformation.isSelectStatement()) { + throw new IllegalStateException(String.format( + "Cannot derive count query for %s statement. Count queries are only supported for SELECT statements.", + queryInformation.getStatementType())); + } + return QueryRenderer.TokenRenderer .render(countQueryFunction.apply(countProjection, this.queryInformation).visit(context)); } diff --git a/spring-data-jpa/src/main/java/org/springframework/data/jpa/repository/query/JpqlQueryIntrospector.java b/spring-data-jpa/src/main/java/org/springframework/data/jpa/repository/query/JpqlQueryIntrospector.java index f819778ce6..21330fb6ec 100644 --- a/spring-data-jpa/src/main/java/org/springframework/data/jpa/repository/query/JpqlQueryIntrospector.java +++ b/spring-data-jpa/src/main/java/org/springframework/data/jpa/repository/query/JpqlQueryIntrospector.java @@ -28,6 +28,7 @@ * * @author Mark Paluch * @author Christoph Strobl + * @author kssumin */ @SuppressWarnings({ "UnreachableCode", "ConstantValue" }) class JpqlQueryIntrospector extends JpqlBaseVisitor implements ParsedQueryIntrospector { @@ -38,11 +39,36 @@ class JpqlQueryIntrospector extends JpqlBaseVisitor implements ParsedQuery private @Nullable List projection; private boolean projectionProcessed; private boolean hasConstructorExpression = false; + private QueryInformation.StatementType statementType = QueryInformation.StatementType.SELECT; @Override public QueryInformation getParsedQueryInformation() { return new QueryInformation(primaryFromAlias, projection == null ? Collections.emptyList() : projection, - hasConstructorExpression); + hasConstructorExpression, statementType); + } + + @Override + public Void visitSelectQuery(JpqlParser.SelectQueryContext ctx) { + statementType = QueryInformation.StatementType.SELECT; + return super.visitSelectQuery(ctx); + } + + @Override + public Void visitFromQuery(JpqlParser.FromQueryContext ctx) { + statementType = QueryInformation.StatementType.SELECT; + return super.visitFromQuery(ctx); + } + + @Override + public Void visitUpdate_statement(JpqlParser.Update_statementContext ctx) { + statementType = QueryInformation.StatementType.UPDATE; + return super.visitUpdate_statement(ctx); + } + + @Override + public Void visitDelete_statement(JpqlParser.Delete_statementContext ctx) { + statementType = QueryInformation.StatementType.DELETE; + return super.visitDelete_statement(ctx); } @Override diff --git a/spring-data-jpa/src/main/java/org/springframework/data/jpa/repository/query/QueryInformation.java b/spring-data-jpa/src/main/java/org/springframework/data/jpa/repository/query/QueryInformation.java index b681037cbf..0832f6166b 100644 --- a/spring-data-jpa/src/main/java/org/springframework/data/jpa/repository/query/QueryInformation.java +++ b/spring-data-jpa/src/main/java/org/springframework/data/jpa/repository/query/QueryInformation.java @@ -23,6 +23,7 @@ * Value object capturing introspection details of a parsed query. * * @author Mark Paluch + * @author kssumin * @since 3.5 */ class QueryInformation { @@ -33,10 +34,18 @@ class QueryInformation { private final boolean hasConstructorExpression; + private final StatementType statementType; + QueryInformation(@Nullable String alias, List projection, boolean hasConstructorExpression) { + this(alias, projection, hasConstructorExpression, StatementType.SELECT); + } + + QueryInformation(@Nullable String alias, List projection, boolean hasConstructorExpression, + StatementType statementType) { this.alias = alias; this.projection = projection; this.hasConstructorExpression = hasConstructorExpression; + this.statementType = statementType; } /** @@ -61,4 +70,59 @@ public List getProjection() { public boolean hasConstructorExpression() { return hasConstructorExpression; } + + /** + * @return the statement type of the query. + * @since 4.0 + */ + public StatementType getStatementType() { + return statementType; + } + + /** + * @return {@code true} if the query is a SELECT statement. + * @since 4.0 + */ + public boolean isSelectStatement() { + return statementType == StatementType.SELECT; + } + + /** + * Enum representing the type of SQL/JPQL statement. + * + * @since 4.0 + */ + enum StatementType { + + /** + * SELECT statement. + */ + SELECT, + + /** + * INSERT statement. + */ + INSERT, + + /** + * UPDATE statement. + */ + UPDATE, + + /** + * DELETE statement. + */ + DELETE, + + /** + * MERGE statement. + */ + MERGE, + + /** + * Other statement types. + */ + OTHER + } + } diff --git a/spring-data-jpa/src/test/java/org/springframework/data/jpa/repository/query/JSqlParserQueryEnhancerUnitTests.java b/spring-data-jpa/src/test/java/org/springframework/data/jpa/repository/query/JSqlParserQueryEnhancerUnitTests.java index e756ea0273..80fe89366a 100644 --- a/spring-data-jpa/src/test/java/org/springframework/data/jpa/repository/query/JSqlParserQueryEnhancerUnitTests.java +++ b/spring-data-jpa/src/test/java/org/springframework/data/jpa/repository/query/JSqlParserQueryEnhancerUnitTests.java @@ -238,8 +238,10 @@ void truncateStatementShouldWork() { assertThat(query.getProjection()).isEmpty(); assertThat(query.hasConstructorExpression()).isFalse(); - assertThat(queryEnhancer.rewrite(getRewriteInformation(Sort.by("day").descending()))) - .isEqualTo("TRUNCATE TABLE foo"); + // GH-2856: QueryEnhancer should throw exceptions for non-SELECT statements with sorting + assertThatThrownBy(() -> queryEnhancer.rewrite(getRewriteInformation(Sort.by("day").descending()))) + .isInstanceOf(IllegalStateException.class) + .hasMessageContaining("Cannot apply sorting to OTHER statement"); assertThat(queryEnhancer.detectAlias()).isNull(); assertThat(queryEnhancer.getProjection()).isEmpty(); assertThat(queryEnhancer.hasConstructorExpression()).isFalse(); @@ -282,6 +284,54 @@ static Stream mergeStatementWorksSource() { null)); } + @Test // GH-2856 + void nativeInsertQueryThrowsExceptionForCountQuery() { + + // Given: Native INSERT query + DeclaredQuery query = DeclaredQuery.nativeQuery("INSERT INTO users(name) VALUES('John')"); + QueryEnhancer enhancer = new JSqlParserQueryEnhancer(query); + + // When/Then: Should throw IllegalStateException for count query + assertThatThrownBy(() -> enhancer.createCountQueryFor(null)) + .isInstanceOf(IllegalStateException.class) + .hasMessageContaining("Cannot derive count query for INSERT statement") + .hasMessageContaining("SELECT"); + } + + @Test // GH-2856 + void nativeUpdateQueryThrowsExceptionForSorting() { + + // Given: UPDATE query + DeclaredQuery query = DeclaredQuery.nativeQuery("UPDATE users SET name = 'test'"); + QueryEnhancer enhancer = new JSqlParserQueryEnhancer(query); + + // When/Then: Should throw IllegalStateException for sorting + Sort sort = Sort.by("id"); + QueryEnhancer.QueryRewriteInformation rewriteInfo = new DefaultQueryRewriteInformation( + sort, ReturnedType.of(Object.class, Object.class, new SpelAwareProxyProjectionFactory())); + + assertThatThrownBy(() -> enhancer.rewrite(rewriteInfo)) + .isInstanceOf(IllegalStateException.class) + .hasMessageContaining("Cannot apply sorting to UPDATE statement") + .hasMessageContaining("SELECT"); + } + + @Test // GH-2856 + void nativeAllowsUnsortedForNonSelectQueries() { + + // Given: UPDATE query + DeclaredQuery query = DeclaredQuery.nativeQuery("UPDATE users SET name = 'test'"); + QueryEnhancer enhancer = new JSqlParserQueryEnhancer(query); + + // When: Unsorted (no sorting) + QueryEnhancer.QueryRewriteInformation rewriteInfo = new DefaultQueryRewriteInformation( + Sort.unsorted(), ReturnedType.of(Object.class, Object.class, new SpelAwareProxyProjectionFactory())); + + // Then: Should not throw exception + String result = enhancer.rewrite(rewriteInfo); + assertThat(result).containsIgnoringCase("UPDATE users"); + } + private static DefaultQueryRewriteInformation getRewriteInformation(Sort sort) { return new DefaultQueryRewriteInformation(sort, ReturnedType.of(Object.class, Object.class, new SpelAwareProxyProjectionFactory())); diff --git a/spring-data-jpa/src/test/java/org/springframework/data/jpa/repository/query/QueryEnhancerUnitTests.java b/spring-data-jpa/src/test/java/org/springframework/data/jpa/repository/query/QueryEnhancerUnitTests.java index da113f567b..cf691219a7 100644 --- a/spring-data-jpa/src/test/java/org/springframework/data/jpa/repository/query/QueryEnhancerUnitTests.java +++ b/spring-data-jpa/src/test/java/org/springframework/data/jpa/repository/query/QueryEnhancerUnitTests.java @@ -191,13 +191,15 @@ void preserveSourceQueryWhenAddingSort() { .startsWithIgnoringCase(query.getQueryString()).endsWithIgnoringCase("ORDER BY p.name ASC"); } - @Test // GH-2812 + @Test // GH-2812, GH-2856 void createCountQueryFromDeleteQuery() { DefaultEntityQuery query = new TestEntityQuery("delete from some_table where id in :ids", true); - assertThat(getEnhancer(query).createCountQueryFor("p.lastname")) - .isEqualToIgnoringCase("delete from some_table where id in :ids"); + // GH-2856: QueryEnhancer should throw exceptions for non-SELECT queries + assertThatThrownBy(() -> getEnhancer(query).createCountQueryFor("p.lastname")) + .isInstanceOf(IllegalStateException.class) + .hasMessageContaining("Cannot derive count query for DELETE statement"); } @Test // DATAJPA-456 @@ -608,7 +610,7 @@ void correctApplySortOnComplexNestedFunctionQuery() { assertThat(result).containsIgnoringCase("order by dd.institutesIds"); } - @Test // GH-2555 + @Test // GH-2555, GH-2856 void modifyingQueriesAreDetectedCorrectly() { String modifyingQuery = "update userinfo user set user.is_in_treatment = false where user.id = :userId"; @@ -624,8 +626,13 @@ void modifyingQueriesAreDetectedCorrectly() { assertThat(modiQuery.getProjection()).isEqualToIgnoringCase(projectionNotConsideringQueryType); assertThat(modiQuery.hasConstructorExpression()).isEqualTo(constructorExpressionNotConsideringQueryType); + // QueryUtils returns original query (not aware of statement type) assertThat(countQueryForNotConsiderQueryType).isEqualToIgnoringCase(modifyingQuery); - assertThat(QueryEnhancer.create(modiQuery).createCountQueryFor(null)).isEqualToIgnoringCase(modifyingQuery); + + // GH-2856: QueryEnhancer should throw exceptions for non-SELECT queries + assertThatThrownBy(() -> QueryEnhancer.create(modiQuery).createCountQueryFor(null)) + .isInstanceOf(IllegalStateException.class) + .hasMessageContaining("Cannot derive count query for UPDATE statement"); } @ParameterizedTest // GH-2593 @@ -649,11 +656,14 @@ void insertStatementIsProcessedSameAsDefault(String insertQuery) { assertThat(stringQuery.hasConstructorExpression()).isFalse(); // access over enhancer - assertThat(queryEnhancer.createCountQueryFor(null)).isEqualToIgnoringCase(queryUtilsCountQuery); - assertThat(queryEnhancer.rewrite(getRewriteInformation(sorting))).isEqualTo(insertQuery); // cant check with - // queryutils result since - // query utils appens order by which is not - // supported by sql standard. + // GH-2856: QueryEnhancer should throw exceptions for non-SELECT statements + assertThatThrownBy(() -> queryEnhancer.createCountQueryFor(null)) + .isInstanceOf(IllegalStateException.class) + .hasMessageContaining("Cannot derive count query for INSERT statement"); + + assertThatThrownBy(() -> queryEnhancer.rewrite(getRewriteInformation(sorting))) + .isInstanceOf(IllegalStateException.class) + .hasMessageContaining("Cannot apply sorting to INSERT statement"); assertThat(queryEnhancer.detectAlias()).isEqualToIgnoringCase(queryUtilsDetectAlias); assertThat(queryEnhancer.getProjection()).isEqualToIgnoringCase(queryUtilsProjection); assertThat(queryEnhancer.hasConstructorExpression()).isFalse(); @@ -688,6 +698,68 @@ private static void assertCountQuery(DefaultEntityQuery originalQuery, String co assertThat(getEnhancer(originalQuery).createCountQueryFor(null)).isEqualToIgnoringCase(countQuery); } + @Test // GH-2856 + void jpqlInsertQueryThrowsExceptionForCountQuery() { + + // Given: HQL INSERT query (HQL supports INSERT INTO ... SELECT) + DeclaredQuery query = DeclaredQuery.jpqlQuery("INSERT INTO Foo(a) SELECT b.a FROM Bar b"); + QueryEnhancer enhancer = QueryEnhancer.create(query); + + // When/Then: Should throw IllegalStateException for count query + assertThatThrownBy(() -> enhancer.createCountQueryFor(null)) + .isInstanceOf(IllegalStateException.class) + .hasMessageContaining("Cannot derive count query for INSERT statement") + .hasMessageContaining("SELECT"); + } + + @Test // GH-2856 + void jpqlUpdateQueryThrowsExceptionForSorting() { + + // Given: JPQL UPDATE query + DeclaredQuery query = DeclaredQuery.jpqlQuery("UPDATE User SET name = 'test'"); + QueryEnhancer enhancer = QueryEnhancer.create(query); + + // When/Then: Should throw IllegalStateException for sorting + Sort sort = Sort.by("id"); + QueryEnhancer.QueryRewriteInformation rewriteInfo = new DefaultQueryRewriteInformation( + sort, ReturnedType.of(Object.class, Object.class, new SpelAwareProxyProjectionFactory())); + + assertThatThrownBy(() -> enhancer.rewrite(rewriteInfo)) + .isInstanceOf(IllegalStateException.class) + .hasMessageContaining("Cannot apply sorting to UPDATE statement") + .hasMessageContaining("SELECT"); + } + + @Test // GH-2856 + void jpqlDeleteQueryThrowsExceptionForCountQuery() { + + // Given: JPQL DELETE query + DeclaredQuery query = DeclaredQuery.jpqlQuery("DELETE FROM User u WHERE u.id = :id"); + QueryEnhancer enhancer = QueryEnhancer.create(query); + + // When/Then: Should throw IllegalStateException for count query + assertThatThrownBy(() -> enhancer.createCountQueryFor(null)) + .isInstanceOf(IllegalStateException.class) + .hasMessageContaining("Cannot derive count query for DELETE statement") + .hasMessageContaining("SELECT"); + } + + @Test // GH-2856 + void jpqlAllowsUnsortedForNonSelectQueries() { + + // Given: JPQL UPDATE query + DeclaredQuery query = DeclaredQuery.jpqlQuery("UPDATE User SET name = 'test'"); + QueryEnhancer enhancer = QueryEnhancer.create(query); + + // When: Unsorted (no sorting) + QueryEnhancer.QueryRewriteInformation rewriteInfo = new DefaultQueryRewriteInformation( + Sort.unsorted(), ReturnedType.of(Object.class, Object.class, new SpelAwareProxyProjectionFactory())); + + // Then: Should not throw exception + String result = enhancer.rewrite(rewriteInfo); + assertThat(result).isEqualTo("UPDATE User SET name = 'test'"); + } + private static DefaultQueryRewriteInformation getRewriteInformation(Sort sort) { return new DefaultQueryRewriteInformation(sort, ReturnedType.of(Object.class, Object.class, new SpelAwareProxyProjectionFactory())); From a3289894f4e00b6ab71f77e8104e0c0a0003a4df Mon Sep 17 00:00:00 2001 From: Mark Paluch Date: Tue, 21 Oct 2025 10:47:25 +0200 Subject: [PATCH 2/3] Polishing. Update author tags. Refine introspection defaults instead of assuming SELECT statements by default. Extract code duplications from introspection to a mutable QueryInformationHolder. Retrofit AOT query derivation to skip count query creation for non-SELECT queries. Original pull request: #4052 See #2856 --- .../data/jpa/repository/aot/AotQueries.java | 28 +++++ .../jpa/repository/aot/QueriesFactory.java | 4 + .../query/EqlQueryIntrospector.java | 59 +++------- .../query/HibernateQueryInformation.java | 18 +-- .../query/HqlQueryIntrospector.java | 79 +++++-------- .../query/JSqlParserQueryEnhancer.java | 30 ++--- .../repository/query/JpaQueryEnhancer.java | 24 ++-- .../query/JpqlQueryIntrospector.java | 61 +++------- .../jpa/repository/query/QueryEnhancer.java | 14 ++- .../repository/query/QueryInformation.java | 20 +--- .../query/QueryInformationHolder.java | 105 ++++++++++++++++++ .../JSqlParserQueryEnhancerUnitTests.java | 27 ++--- .../query/JpaQueryEnhancerUnitTests.java | 25 +++++ .../query/QueryEnhancerUnitTests.java | 73 +++--------- 14 files changed, 303 insertions(+), 264 deletions(-) create mode 100644 spring-data-jpa/src/main/java/org/springframework/data/jpa/repository/query/QueryInformationHolder.java diff --git a/spring-data-jpa/src/main/java/org/springframework/data/jpa/repository/aot/AotQueries.java b/spring-data-jpa/src/main/java/org/springframework/data/jpa/repository/aot/AotQueries.java index 9ef9705bf2..7b223f2645 100644 --- a/spring-data-jpa/src/main/java/org/springframework/data/jpa/repository/aot/AotQueries.java +++ b/spring-data-jpa/src/main/java/org/springframework/data/jpa/repository/aot/AotQueries.java @@ -17,6 +17,7 @@ import java.util.LinkedHashMap; +import java.util.List; import java.util.Map; import java.util.function.Function; @@ -36,6 +37,10 @@ */ record AotQueries(AotQuery result, AotQuery count) { + public AotQueries(AotQuery query) { + this(query, AbsentCountQuery.INSTANCE); + } + /** * Derive a count query from the given query. */ @@ -45,6 +50,10 @@ public static AotQueries withDerivedCountQuery(T query, Fun DeclaredQuery underlyingQuery = queryMapper.apply(query); QueryEnhancer queryEnhancer = selector.select(underlyingQuery).create(underlyingQuery); + if (!queryEnhancer.isSelectQuery()) { + return new AotQueries(query); + } + String derivedCountQuery = queryEnhancer .createCountQueryFor(StringUtils.hasText(countProjection) ? countProjection : null); @@ -66,6 +75,25 @@ public QueryMetadata toMetadata(boolean paging) { return new AotQueryMetadata(paging); } + private static class AbsentCountQuery extends AotQuery { + + static final AbsentCountQuery INSTANCE = new AbsentCountQuery(); + + AbsentCountQuery() { + super(List.of()); + } + + @Override + public boolean isNative() { + return false; + } + + @Override + public boolean hasConstructorExpressionOrDefaultProjection() { + return false; + } + } + /** * String and Named Query-based {@link QueryMetadata}. */ diff --git a/spring-data-jpa/src/main/java/org/springframework/data/jpa/repository/aot/QueriesFactory.java b/spring-data-jpa/src/main/java/org/springframework/data/jpa/repository/aot/QueriesFactory.java index 9a764c255c..7dd293b313 100644 --- a/spring-data-jpa/src/main/java/org/springframework/data/jpa/repository/aot/QueriesFactory.java +++ b/spring-data-jpa/src/main/java/org/springframework/data/jpa/repository/aot/QueriesFactory.java @@ -180,6 +180,10 @@ public ReturnedType getReturnedType() { createNamedAotQuery(returnedType, selector, queryMethod.getNamedCountQueryName(), queryMethod, isNative)); } + if (queryMethod.isModifyingQuery()) { + + } + String countProjection = query.getString("countProjection"); return AotQueries.withDerivedCountQuery(aotStringQuery, StringAotQuery::getQuery, countProjection, selector); } diff --git a/spring-data-jpa/src/main/java/org/springframework/data/jpa/repository/query/EqlQueryIntrospector.java b/spring-data-jpa/src/main/java/org/springframework/data/jpa/repository/query/EqlQueryIntrospector.java index 5e78aebb8c..e8fb0a9fdc 100644 --- a/spring-data-jpa/src/main/java/org/springframework/data/jpa/repository/query/EqlQueryIntrospector.java +++ b/spring-data-jpa/src/main/java/org/springframework/data/jpa/repository/query/EqlQueryIntrospector.java @@ -15,79 +15,65 @@ */ package org.springframework.data.jpa.repository.query; -import static org.springframework.data.jpa.repository.query.QueryTokens.*; - -import java.util.ArrayList; -import java.util.Collections; -import java.util.List; - -import org.jspecify.annotations.Nullable; - /** * {@link ParsedQueryIntrospector} for EQL queries. * * @author Mark Paluch * @author Christoph Strobl - * @author kssumin + * @author Soomin Kim */ @SuppressWarnings("UnreachableCode") class EqlQueryIntrospector extends EqlBaseVisitor implements ParsedQueryIntrospector { private final EqlQueryRenderer renderer = new EqlQueryRenderer(); - - private @Nullable String primaryFromAlias = null; - private @Nullable List projection; - private boolean projectionProcessed; - private boolean hasConstructorExpression = false; - private QueryInformation.StatementType statementType = QueryInformation.StatementType.SELECT; + private final QueryInformationHolder introspection = new QueryInformationHolder(); @Override public QueryInformation getParsedQueryInformation() { - return new QueryInformation(primaryFromAlias, projection == null ? Collections.emptyList() : projection, - hasConstructorExpression, statementType); + return new QueryInformation(introspection); } @Override public Void visitSelectQuery(EqlParser.SelectQueryContext ctx) { - statementType = QueryInformation.StatementType.SELECT; + + introspection.setStatementType(QueryInformation.StatementType.SELECT); return super.visitSelectQuery(ctx); } @Override public Void visitFromQuery(EqlParser.FromQueryContext ctx) { - statementType = QueryInformation.StatementType.SELECT; + + introspection.setStatementType(QueryInformation.StatementType.SELECT); return super.visitFromQuery(ctx); } @Override public Void visitUpdate_statement(EqlParser.Update_statementContext ctx) { - statementType = QueryInformation.StatementType.UPDATE; + + introspection.setStatementType(QueryInformation.StatementType.UPDATE); return super.visitUpdate_statement(ctx); } @Override public Void visitDelete_statement(EqlParser.Delete_statementContext ctx) { - statementType = QueryInformation.StatementType.DELETE; + + introspection.setStatementType(QueryInformation.StatementType.DELETE); return super.visitDelete_statement(ctx); } @Override public Void visitSelect_clause(EqlParser.Select_clauseContext ctx) { - if (!projectionProcessed) { - projection = captureSelectItems(ctx.select_item(), renderer); - projectionProcessed = true; - } - + introspection.captureProjection(ctx.select_item(), renderer::visitSelect_item); return super.visitSelect_clause(ctx); } @Override public Void visitRange_variable_declaration(EqlParser.Range_variable_declarationContext ctx) { - if (primaryFromAlias == null && ctx.identification_variable() != null && !EqlQueryRenderer.isSubquery(ctx) + if (ctx.identification_variable() != null && !EqlQueryRenderer.isSubquery(ctx) && !EqlQueryRenderer.isSetQuery(ctx)) { - primaryFromAlias = ctx.identification_variable().getText(); + introspection.capturePrimaryAlias(ctx.identification_variable().getText()); } return super.visitRange_variable_declaration(ctx); @@ -96,23 +82,8 @@ public Void visitRange_variable_declaration(EqlParser.Range_variable_declaration @Override public Void visitConstructor_expression(EqlParser.Constructor_expressionContext ctx) { - hasConstructorExpression = true; + introspection.constructorExpressionPresent(); return super.visitConstructor_expression(ctx); } - private static List captureSelectItems(List selections, - EqlQueryRenderer itemRenderer) { - - List selectItemTokens = new ArrayList<>(selections.size() * 2); - for (EqlParser.Select_itemContext selection : selections) { - - if (!selectItemTokens.isEmpty()) { - selectItemTokens.add(TOKEN_COMMA); - } - - selectItemTokens.add(QueryTokens.token(QueryRenderer.from(itemRenderer.visitSelect_item(selection)).render())); - } - return selectItemTokens; - } - } diff --git a/spring-data-jpa/src/main/java/org/springframework/data/jpa/repository/query/HibernateQueryInformation.java b/spring-data-jpa/src/main/java/org/springframework/data/jpa/repository/query/HibernateQueryInformation.java index 1ea4e85ca8..e9e283d88b 100644 --- a/spring-data-jpa/src/main/java/org/springframework/data/jpa/repository/query/HibernateQueryInformation.java +++ b/spring-data-jpa/src/main/java/org/springframework/data/jpa/repository/query/HibernateQueryInformation.java @@ -15,16 +15,12 @@ */ package org.springframework.data.jpa.repository.query; -import java.util.List; - -import org.jspecify.annotations.Nullable; - /** * Hibernate-specific query details capturing common table expression details. * * @author Mark Paluch * @author Oscar Fanchin - * @author kssumin + * @author Soomin Kim * @since 3.5 */ class HibernateQueryInformation extends QueryInformation { @@ -33,16 +29,8 @@ class HibernateQueryInformation extends QueryInformation { private final boolean hasFromFunction; - public HibernateQueryInformation(@Nullable String alias, List projection, - boolean hasConstructorExpression, boolean hasCte, boolean hasFromFunction) { - super(alias, projection, hasConstructorExpression); - this.hasCte = hasCte; - this.hasFromFunction = hasFromFunction; - } - - public HibernateQueryInformation(@Nullable String alias, List projection, - boolean hasConstructorExpression, StatementType statementType, boolean hasCte, boolean hasFromFunction) { - super(alias, projection, hasConstructorExpression, statementType); + public HibernateQueryInformation(QueryInformationHolder introspection, boolean hasCte, boolean hasFromFunction) { + super(introspection); this.hasCte = hasCte; this.hasFromFunction = hasFromFunction; } diff --git a/spring-data-jpa/src/main/java/org/springframework/data/jpa/repository/query/HqlQueryIntrospector.java b/spring-data-jpa/src/main/java/org/springframework/data/jpa/repository/query/HqlQueryIntrospector.java index aa8065e06c..776746f79f 100644 --- a/spring-data-jpa/src/main/java/org/springframework/data/jpa/repository/query/HqlQueryIntrospector.java +++ b/spring-data-jpa/src/main/java/org/springframework/data/jpa/repository/query/HqlQueryIntrospector.java @@ -15,14 +15,6 @@ */ package org.springframework.data.jpa.repository.query; -import static org.springframework.data.jpa.repository.query.QueryTokens.*; - -import java.util.ArrayList; -import java.util.Collections; -import java.util.List; - -import org.jspecify.annotations.Nullable; - import org.springframework.data.jpa.repository.query.HqlParser.VariableContext; /** @@ -30,58 +22,61 @@ * * @author Mark Paluch * @author Oscar Fanchin - * @author kssumin + * @author Soomin Kim */ -@SuppressWarnings({ "UnreachableCode", "ConstantValue" }) +@SuppressWarnings({ "UnreachableCode" }) class HqlQueryIntrospector extends HqlBaseVisitor implements ParsedQueryIntrospector { private final HqlQueryRenderer renderer = new HqlQueryRenderer(); + private final QueryInformationHolder introspection = new QueryInformationHolder(); - private @Nullable String primaryFromAlias = null; - private @Nullable List projection; - private boolean projectionProcessed; - private boolean hasConstructorExpression = false; private boolean hasCte = false; private boolean hasFromFunction = false; - private QueryInformation.StatementType statementType = QueryInformation.StatementType.SELECT; @Override public HibernateQueryInformation getParsedQueryInformation() { - return new HibernateQueryInformation(primaryFromAlias, projection == null ? Collections.emptyList() : projection, - hasConstructorExpression, statementType, hasCte, hasFromFunction); + return new HibernateQueryInformation(introspection, hasCte, hasFromFunction); } @Override public Void visitSelectStatement(HqlParser.SelectStatementContext ctx) { - statementType = QueryInformation.StatementType.SELECT; + + introspection.setStatementType(QueryInformation.StatementType.SELECT); return super.visitSelectStatement(ctx); } + @Override + public Void visitFromQuery(HqlParser.FromQueryContext ctx) { + + introspection.setStatementType(QueryInformation.StatementType.SELECT); + return super.visitFromQuery(ctx); + } + @Override public Void visitInsertStatement(HqlParser.InsertStatementContext ctx) { - statementType = QueryInformation.StatementType.INSERT; + + introspection.setStatementType(QueryInformation.StatementType.INSERT); return super.visitInsertStatement(ctx); } @Override public Void visitUpdateStatement(HqlParser.UpdateStatementContext ctx) { - statementType = QueryInformation.StatementType.UPDATE; + + introspection.setStatementType(QueryInformation.StatementType.UPDATE); return super.visitUpdateStatement(ctx); } @Override public Void visitDeleteStatement(HqlParser.DeleteStatementContext ctx) { - statementType = QueryInformation.StatementType.DELETE; + + introspection.setStatementType(QueryInformation.StatementType.DELETE); return super.visitDeleteStatement(ctx); } @Override public Void visitSelectClause(HqlParser.SelectClauseContext ctx) { - if (!this.projectionProcessed) { - this.projection = captureSelectItems(ctx.selectionList().selection(), renderer); - this.projectionProcessed = true; - } + introspection.captureProjection(ctx.selectionList().selection(), renderer::visitSelection); return super.visitSelectClause(ctx); } @@ -95,9 +90,9 @@ public Void visitCte(HqlParser.CteContext ctx) { @Override public Void visitRootEntity(HqlParser.RootEntityContext ctx) { - if (this.primaryFromAlias == null && ctx.variable() != null && !HqlQueryRenderer.isSubquery(ctx) + if (ctx.variable() != null && !HqlQueryRenderer.isSubquery(ctx) && !HqlQueryRenderer.isSetQuery(ctx)) { - this.primaryFromAlias = capturePrimaryAlias(ctx.variable()); + capturePrimaryAlias(ctx.variable()); } return super.visitRootEntity(ctx); @@ -106,9 +101,9 @@ public Void visitRootEntity(HqlParser.RootEntityContext ctx) { @Override public Void visitRootSubquery(HqlParser.RootSubqueryContext ctx) { - if (this.primaryFromAlias == null && ctx.variable() != null && !HqlQueryRenderer.isSubquery(ctx) + if (ctx.variable() != null && !HqlQueryRenderer.isSubquery(ctx) && !HqlQueryRenderer.isSetQuery(ctx)) { - this.primaryFromAlias = capturePrimaryAlias(ctx.variable()); + capturePrimaryAlias(ctx.variable()); } return super.visitRootSubquery(ctx); @@ -117,9 +112,9 @@ public Void visitRootSubquery(HqlParser.RootSubqueryContext ctx) { @Override public Void visitRootFunction(HqlParser.RootFunctionContext ctx) { - if (this.primaryFromAlias == null && ctx.variable() != null && !HqlQueryRenderer.isSubquery(ctx) + if (ctx.variable() != null && !HqlQueryRenderer.isSubquery(ctx) && !HqlQueryRenderer.isSetQuery(ctx)) { - this.primaryFromAlias = capturePrimaryAlias(ctx.variable()); + capturePrimaryAlias(ctx.variable()); this.hasFromFunction = true; } @@ -129,27 +124,13 @@ public Void visitRootFunction(HqlParser.RootFunctionContext ctx) { @Override public Void visitInstantiation(HqlParser.InstantiationContext ctx) { - hasConstructorExpression = true; - + introspection.constructorExpressionPresent(); return super.visitInstantiation(ctx); } - private static String capturePrimaryAlias(VariableContext ctx) { - return ((ctx).nakedIdentifier() != null ? ctx.nakedIdentifier() : ctx.identifier()).getText(); + private void capturePrimaryAlias(VariableContext ctx) { + introspection + .capturePrimaryAlias((ctx.nakedIdentifier() != null ? ctx.nakedIdentifier() : ctx.identifier()).getText()); } - private static List captureSelectItems(List selections, - HqlQueryRenderer itemRenderer) { - - List selectItemTokens = new ArrayList<>(selections.size() * 2); - for (HqlParser.SelectionContext selection : selections) { - - if (!selectItemTokens.isEmpty()) { - selectItemTokens.add(TOKEN_COMMA); - } - - selectItemTokens.add(QueryTokens.token(QueryRenderer.from(itemRenderer.visitSelection(selection)).render())); - } - return selectItemTokens; - } } diff --git a/spring-data-jpa/src/main/java/org/springframework/data/jpa/repository/query/JSqlParserQueryEnhancer.java b/spring-data-jpa/src/main/java/org/springframework/data/jpa/repository/query/JSqlParserQueryEnhancer.java index 57294788ec..976b633b10 100644 --- a/spring-data-jpa/src/main/java/org/springframework/data/jpa/repository/query/JSqlParserQueryEnhancer.java +++ b/spring-data-jpa/src/main/java/org/springframework/data/jpa/repository/query/JSqlParserQueryEnhancer.java @@ -69,7 +69,7 @@ * @author Yanming Zhou * @author Christoph Strobl * @author Diego Pedregal - * @author kssumin + * @author Soomin Kim * @since 2.7.0 */ public class JSqlParserQueryEnhancer implements QueryEnhancer { @@ -311,6 +311,11 @@ private static ParsedType detectParsedType(Statement statement) { } } + @Override + public boolean isSelectQuery() { + return this.parsedType == ParsedType.SELECT; + } + @Override public boolean hasConstructorExpression() { return hasConstructorExpression; @@ -337,27 +342,21 @@ public QueryProvider getQuery() { @Override public String rewrite(QueryRewriteInformation rewriteInformation) { - return doApplySorting(rewriteInformation.getSort(), primaryAlias); - } - private String doApplySorting(Sort sort, @Nullable String alias) { + Sort sort = rewriteInformation.getSort(); String queryString = query.getQueryString(); - Assert.hasText(queryString, "Query must not be null or empty"); - if (this.parsedType != ParsedType.SELECT) { - if (!sort.isUnsorted()) { - throw new IllegalStateException(String.format( - "Cannot apply sorting to %s statement. Sorting is only supported for SELECT statements.", - this.parsedType)); - } - return queryString; + if (!isSelectQuery() && sort.isSorted()) { + throw new IllegalStateException( + "Cannot apply sorting to %s statement. Sorting is only supported for SELECT statements." + .formatted(this.parsedType)); } if (sort.isUnsorted()) { return queryString; } - return applySorting(deserializeRequired(this.serialized, Select.class), sort, alias); + return applySorting(deserializeRequired(this.serialized, Select.class), sort, primaryAlias); } private String applySorting(@Nullable Select selectStatement, Sort sort, @Nullable String alias) { @@ -393,8 +392,9 @@ private String applySorting(@Nullable Select selectStatement, Sort sort, @Nullab public String createCountQueryFor(@Nullable String countProjection) { if (this.parsedType != ParsedType.SELECT) { - throw new IllegalStateException(String.format( - "Cannot derive count query for %s statement. Count queries are only supported for SELECT statements.", + throw new IllegalStateException( + "Cannot derive count query for %s statement. Count queries are only supported for SELECT statements." + .formatted( this.parsedType)); } diff --git a/spring-data-jpa/src/main/java/org/springframework/data/jpa/repository/query/JpaQueryEnhancer.java b/spring-data-jpa/src/main/java/org/springframework/data/jpa/repository/query/JpaQueryEnhancer.java index 5bb082da78..24548c9164 100644 --- a/spring-data-jpa/src/main/java/org/springframework/data/jpa/repository/query/JpaQueryEnhancer.java +++ b/spring-data-jpa/src/main/java/org/springframework/data/jpa/repository/query/JpaQueryEnhancer.java @@ -41,7 +41,7 @@ * * @author Greg Turnquist * @author Mark Paluch - * @author kssumin + * @author Soomin Kim * @since 3.1 * @see JpqlQueryParser * @see HqlQueryParser @@ -184,6 +184,11 @@ Q getQueryInformation() { return queryInformation; } + @Override + public boolean isSelectQuery() { + return this.queryInformation.isSelectStatement(); + } + /** * Checks if the select clause has a new constructor instantiation in the JPA query. * @@ -222,14 +227,16 @@ public DeclaredQuery getQuery() { @Override public String rewrite(QueryRewriteInformation rewriteInformation) { - if (!queryInformation.isSelectStatement() && !rewriteInformation.getSort().isUnsorted()) { - throw new IllegalStateException(String.format( - "Cannot apply sorting to %s statement. Sorting is only supported for SELECT statements.", - queryInformation.getStatementType())); + Sort sort = rewriteInformation.getSort(); + + if (!queryInformation.isSelectStatement() && sort.isSorted()) { + throw new IllegalStateException( + "Cannot apply sorting to %s statement. Sorting is only supported for SELECT statements." + .formatted(queryInformation.getStatementType())); } return QueryRenderer.TokenRenderer.render( - sortFunction.apply(rewriteInformation.getSort(), this.queryInformation, rewriteInformation.getReturnedType()) + sortFunction.apply(sort, this.queryInformation, rewriteInformation.getReturnedType()) .visit(context)); } @@ -242,8 +249,9 @@ public String rewrite(QueryRewriteInformation rewriteInformation) { public String createCountQueryFor(@Nullable String countProjection) { if (!queryInformation.isSelectStatement()) { - throw new IllegalStateException(String.format( - "Cannot derive count query for %s statement. Count queries are only supported for SELECT statements.", + throw new IllegalStateException( + "Cannot derive count query for %s statement. Count queries are only supported for SELECT statements." + .formatted( queryInformation.getStatementType())); } diff --git a/spring-data-jpa/src/main/java/org/springframework/data/jpa/repository/query/JpqlQueryIntrospector.java b/spring-data-jpa/src/main/java/org/springframework/data/jpa/repository/query/JpqlQueryIntrospector.java index 21330fb6ec..7ccb450c06 100644 --- a/spring-data-jpa/src/main/java/org/springframework/data/jpa/repository/query/JpqlQueryIntrospector.java +++ b/spring-data-jpa/src/main/java/org/springframework/data/jpa/repository/query/JpqlQueryIntrospector.java @@ -15,79 +15,65 @@ */ package org.springframework.data.jpa.repository.query; -import static org.springframework.data.jpa.repository.query.QueryTokens.*; - -import java.util.ArrayList; -import java.util.Collections; -import java.util.List; - -import org.jspecify.annotations.Nullable; - /** * {@link ParsedQueryIntrospector} for JPQL queries. * * @author Mark Paluch * @author Christoph Strobl - * @author kssumin + * @author Soomin Kim */ -@SuppressWarnings({ "UnreachableCode", "ConstantValue" }) +@SuppressWarnings({ "UnreachableCode" }) class JpqlQueryIntrospector extends JpqlBaseVisitor implements ParsedQueryIntrospector { private final JpqlQueryRenderer renderer = new JpqlQueryRenderer(); - - private @Nullable String primaryFromAlias = null; - private @Nullable List projection; - private boolean projectionProcessed; - private boolean hasConstructorExpression = false; - private QueryInformation.StatementType statementType = QueryInformation.StatementType.SELECT; + private final QueryInformationHolder introspection = new QueryInformationHolder(); @Override public QueryInformation getParsedQueryInformation() { - return new QueryInformation(primaryFromAlias, projection == null ? Collections.emptyList() : projection, - hasConstructorExpression, statementType); + return new QueryInformation(introspection); } @Override public Void visitSelectQuery(JpqlParser.SelectQueryContext ctx) { - statementType = QueryInformation.StatementType.SELECT; + + introspection.setStatementType(QueryInformation.StatementType.SELECT); return super.visitSelectQuery(ctx); } @Override public Void visitFromQuery(JpqlParser.FromQueryContext ctx) { - statementType = QueryInformation.StatementType.SELECT; + + introspection.setStatementType(QueryInformation.StatementType.SELECT); return super.visitFromQuery(ctx); } @Override public Void visitUpdate_statement(JpqlParser.Update_statementContext ctx) { - statementType = QueryInformation.StatementType.UPDATE; + + introspection.setStatementType(QueryInformation.StatementType.UPDATE); return super.visitUpdate_statement(ctx); } @Override public Void visitDelete_statement(JpqlParser.Delete_statementContext ctx) { - statementType = QueryInformation.StatementType.DELETE; + + introspection.setStatementType(QueryInformation.StatementType.DELETE); return super.visitDelete_statement(ctx); } @Override public Void visitSelect_clause(JpqlParser.Select_clauseContext ctx) { - if (!projectionProcessed) { - projection = captureSelectItems(ctx.select_item(), renderer); - projectionProcessed = true; - } - + introspection.captureProjection(ctx.select_item(), renderer::visitSelect_item); return super.visitSelect_clause(ctx); } @Override public Void visitRange_variable_declaration(JpqlParser.Range_variable_declarationContext ctx) { - if (primaryFromAlias == null && ctx.identification_variable() != null && !JpqlQueryRenderer.isSubquery(ctx) + if (ctx.identification_variable() != null && !JpqlQueryRenderer.isSubquery(ctx) && !JpqlQueryRenderer.isSetQuery(ctx)) { - primaryFromAlias = ctx.identification_variable().getText(); + introspection.capturePrimaryAlias(ctx.identification_variable().getText()); } return super.visitRange_variable_declaration(ctx); @@ -96,23 +82,8 @@ public Void visitRange_variable_declaration(JpqlParser.Range_variable_declaratio @Override public Void visitConstructor_expression(JpqlParser.Constructor_expressionContext ctx) { - hasConstructorExpression = true; + introspection.constructorExpressionPresent(); return super.visitConstructor_expression(ctx); } - private static List captureSelectItems(List selections, - JpqlQueryRenderer itemRenderer) { - - List selectItemTokens = new ArrayList<>(selections.size() * 2); - for (JpqlParser.Select_itemContext selection : selections) { - - if (!selectItemTokens.isEmpty()) { - selectItemTokens.add(TOKEN_COMMA); - } - - selectItemTokens.add(QueryTokens.token(QueryRenderer.from(itemRenderer.visitSelect_item(selection)).render())); - } - return selectItemTokens; - } - } diff --git a/spring-data-jpa/src/main/java/org/springframework/data/jpa/repository/query/QueryEnhancer.java b/spring-data-jpa/src/main/java/org/springframework/data/jpa/repository/query/QueryEnhancer.java index 2810f957c0..510e15f5e1 100644 --- a/spring-data-jpa/src/main/java/org/springframework/data/jpa/repository/query/QueryEnhancer.java +++ b/spring-data-jpa/src/main/java/org/springframework/data/jpa/repository/query/QueryEnhancer.java @@ -21,7 +21,9 @@ import org.springframework.data.repository.query.ReturnedType; /** - * This interface describes the API for enhancing a given Query. + * This interface describes the API for enhancing a given Query. Query enhancers understand the syntax of the query and + * can introspect queries to determine aliases and projections. Enhancers can also rewrite queries to apply sorting and + * create count queries if the underlying query is a {@link #isSelectQuery() SELECT} query. * * @author Diego Krupitza * @author Greg Turnquist @@ -42,6 +44,14 @@ static QueryEnhancer create(DeclaredQuery query) { return QueryEnhancerFactory.forQuery(query).create(query); } + /** + * @return whether the underlying query is a SELECT query. + * @since 4.0 + */ + default boolean isSelectQuery() { + return true; + } + /** * Returns whether the given JPQL query contains a constructor expression. * @@ -77,6 +87,7 @@ static QueryEnhancer create(DeclaredQuery query) { * @param rewriteInformation the rewrite information to apply. * @return the modified query string. * @since 4.0 + * @throws IllegalStateException if the underlying query is not a {@link #isSelectQuery() SELECT} query. */ String rewrite(QueryRewriteInformation rewriteInformation); @@ -85,6 +96,7 @@ static QueryEnhancer create(DeclaredQuery query) { * * @param countProjection may be {@literal null}. * @return a query String to be used a count query for pagination. Guaranteed to be not {@literal null}. + * @throws IllegalStateException if the underlying query is not a {@link #isSelectQuery() SELECT} query. */ String createCountQueryFor(@Nullable String countProjection); diff --git a/spring-data-jpa/src/main/java/org/springframework/data/jpa/repository/query/QueryInformation.java b/spring-data-jpa/src/main/java/org/springframework/data/jpa/repository/query/QueryInformation.java index 0832f6166b..6d7b6c5c97 100644 --- a/spring-data-jpa/src/main/java/org/springframework/data/jpa/repository/query/QueryInformation.java +++ b/spring-data-jpa/src/main/java/org/springframework/data/jpa/repository/query/QueryInformation.java @@ -23,29 +23,21 @@ * Value object capturing introspection details of a parsed query. * * @author Mark Paluch - * @author kssumin + * @author Soomin Kim * @since 3.5 */ class QueryInformation { private final @Nullable String alias; - private final List projection; - private final boolean hasConstructorExpression; - private final StatementType statementType; - QueryInformation(@Nullable String alias, List projection, boolean hasConstructorExpression) { - this(alias, projection, hasConstructorExpression, StatementType.SELECT); - } - - QueryInformation(@Nullable String alias, List projection, boolean hasConstructorExpression, - StatementType statementType) { - this.alias = alias; - this.projection = projection; - this.hasConstructorExpression = hasConstructorExpression; - this.statementType = statementType; + QueryInformation(QueryInformationHolder introspection) { + this.alias = introspection.getAlias(); + this.projection = introspection.getProjection(); + this.hasConstructorExpression = introspection.hasConstructorExpression(); + this.statementType = introspection.getStatementType(); } /** diff --git a/spring-data-jpa/src/main/java/org/springframework/data/jpa/repository/query/QueryInformationHolder.java b/spring-data-jpa/src/main/java/org/springframework/data/jpa/repository/query/QueryInformationHolder.java new file mode 100644 index 0000000000..16fcac6b4e --- /dev/null +++ b/spring-data-jpa/src/main/java/org/springframework/data/jpa/repository/query/QueryInformationHolder.java @@ -0,0 +1,105 @@ +/* + * Copyright 2025 the original author or authors. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * https://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package org.springframework.data.jpa.repository.query; + +import static org.springframework.data.jpa.repository.query.QueryTokens.*; + +import java.util.ArrayList; +import java.util.Collection; +import java.util.List; +import java.util.function.Function; + +import org.jspecify.annotations.Nullable; + +/** + * Stateful object capturing introspection details of a parsed query. Introspection captures the first occurrence of a + * primary alias or projection items. + * + * @author Mark Paluch + * @since 4.0 + */ +class QueryInformationHolder { + + private @Nullable String primaryFromAlias = null; + private @Nullable List projection; + private boolean projectionProcessed; + private boolean hasConstructorExpression = false; + private QueryInformation.@Nullable StatementType statementType; + + public @Nullable String getAlias() { + return primaryFromAlias; + } + + public void capturePrimaryAlias(String alias) { + + if (primaryFromAlias != null) { + return; + } + + this.primaryFromAlias = alias; + } + + public boolean hasConstructorExpression() { + return hasConstructorExpression; + } + + public void constructorExpressionPresent() { + this.hasConstructorExpression = true; + } + + public QueryInformation.StatementType getStatementType() { + return statementType == null ? QueryInformation.StatementType.OTHER : statementType; + } + + public void setStatementType(QueryInformation.StatementType statementType) { + if (this.statementType == null) { + this.statementType = statementType; + } + } + + public List getProjection() { + return projection == null ? List.of() : List.copyOf(projection); + } + + /** + * Capture projection items if not already captured. + * + * @param selections collection of the selection items. + * @param tokenStreamFunction function that translates a selection item into a {@link QueryTokenStream} (i.e. a + * renderer). + */ + public void captureProjection(Collection selections, Function tokenStreamFunction) { + + if (projectionProcessed) { + return; + + } + List selectItemTokens = new ArrayList<>(selections.size() * 2); + + for (C selection : selections) { + + if (!selectItemTokens.isEmpty()) { + selectItemTokens.add(TOKEN_COMMA); + } + + selectItemTokens.add(QueryTokens.token(QueryRenderer.from(tokenStreamFunction.apply(selection)).render())); + } + + projection = selectItemTokens; + projectionProcessed = true; + } + +} diff --git a/spring-data-jpa/src/test/java/org/springframework/data/jpa/repository/query/JSqlParserQueryEnhancerUnitTests.java b/spring-data-jpa/src/test/java/org/springframework/data/jpa/repository/query/JSqlParserQueryEnhancerUnitTests.java index 80fe89366a..e5ae1b1da2 100644 --- a/spring-data-jpa/src/test/java/org/springframework/data/jpa/repository/query/JSqlParserQueryEnhancerUnitTests.java +++ b/spring-data-jpa/src/test/java/org/springframework/data/jpa/repository/query/JSqlParserQueryEnhancerUnitTests.java @@ -24,6 +24,7 @@ import org.junit.jupiter.params.ParameterizedTest; import org.junit.jupiter.params.provider.Arguments; import org.junit.jupiter.params.provider.MethodSource; + import org.springframework.data.domain.Sort; import org.springframework.data.projection.SpelAwareProxyProjectionFactory; import org.springframework.data.repository.query.ReturnedType; @@ -35,6 +36,7 @@ * @author Diego Krupitza * @author Geoffrey Deremetz * @author Christoph Strobl + * @author Soomin Kim */ class JSqlParserQueryEnhancerUnitTests extends QueryEnhancerTckTests { @@ -238,10 +240,11 @@ void truncateStatementShouldWork() { assertThat(query.getProjection()).isEmpty(); assertThat(query.hasConstructorExpression()).isFalse(); - // GH-2856: QueryEnhancer should throw exceptions for non-SELECT statements with sorting - assertThatThrownBy(() -> queryEnhancer.rewrite(getRewriteInformation(Sort.by("day").descending()))) + assertThatIllegalStateException() + .isThrownBy(() -> queryEnhancer.rewrite(getRewriteInformation(Sort.by("day").descending()))) .isInstanceOf(IllegalStateException.class) - .hasMessageContaining("Cannot apply sorting to OTHER statement"); + .withMessageContaining("Cannot apply sorting to OTHER statement"); + assertThat(queryEnhancer.detectAlias()).isNull(); assertThat(queryEnhancer.getProjection()).isEmpty(); assertThat(queryEnhancer.hasConstructorExpression()).isFalse(); @@ -287,21 +290,16 @@ static Stream mergeStatementWorksSource() { @Test // GH-2856 void nativeInsertQueryThrowsExceptionForCountQuery() { - // Given: Native INSERT query DeclaredQuery query = DeclaredQuery.nativeQuery("INSERT INTO users(name) VALUES('John')"); QueryEnhancer enhancer = new JSqlParserQueryEnhancer(query); - // When/Then: Should throw IllegalStateException for count query - assertThatThrownBy(() -> enhancer.createCountQueryFor(null)) - .isInstanceOf(IllegalStateException.class) - .hasMessageContaining("Cannot derive count query for INSERT statement") - .hasMessageContaining("SELECT"); + assertThatIllegalStateException().isThrownBy(() -> enhancer.createCountQueryFor(null)) + .withMessageContaining("Cannot derive count query for INSERT statement").withMessageContaining("SELECT"); } @Test // GH-2856 void nativeUpdateQueryThrowsExceptionForSorting() { - // Given: UPDATE query DeclaredQuery query = DeclaredQuery.nativeQuery("UPDATE users SET name = 'test'"); QueryEnhancer enhancer = new JSqlParserQueryEnhancer(query); @@ -310,24 +308,19 @@ void nativeUpdateQueryThrowsExceptionForSorting() { QueryEnhancer.QueryRewriteInformation rewriteInfo = new DefaultQueryRewriteInformation( sort, ReturnedType.of(Object.class, Object.class, new SpelAwareProxyProjectionFactory())); - assertThatThrownBy(() -> enhancer.rewrite(rewriteInfo)) - .isInstanceOf(IllegalStateException.class) - .hasMessageContaining("Cannot apply sorting to UPDATE statement") - .hasMessageContaining("SELECT"); + assertThatIllegalStateException().isThrownBy(() -> enhancer.rewrite(rewriteInfo)) + .withMessageContaining("Cannot apply sorting to UPDATE statement").withMessageContaining("SELECT"); } @Test // GH-2856 void nativeAllowsUnsortedForNonSelectQueries() { - // Given: UPDATE query DeclaredQuery query = DeclaredQuery.nativeQuery("UPDATE users SET name = 'test'"); QueryEnhancer enhancer = new JSqlParserQueryEnhancer(query); - // When: Unsorted (no sorting) QueryEnhancer.QueryRewriteInformation rewriteInfo = new DefaultQueryRewriteInformation( Sort.unsorted(), ReturnedType.of(Object.class, Object.class, new SpelAwareProxyProjectionFactory())); - // Then: Should not throw exception String result = enhancer.rewrite(rewriteInfo); assertThat(result).containsIgnoringCase("UPDATE users"); } diff --git a/spring-data-jpa/src/test/java/org/springframework/data/jpa/repository/query/JpaQueryEnhancerUnitTests.java b/spring-data-jpa/src/test/java/org/springframework/data/jpa/repository/query/JpaQueryEnhancerUnitTests.java index 43452f3977..f455e71be2 100644 --- a/spring-data-jpa/src/test/java/org/springframework/data/jpa/repository/query/JpaQueryEnhancerUnitTests.java +++ b/spring-data-jpa/src/test/java/org/springframework/data/jpa/repository/query/JpaQueryEnhancerUnitTests.java @@ -51,6 +51,31 @@ void shouldRemoveCommentsFromJpql(Function> enhancer .isEqualToIgnoringCase("SELECT some_alias FROM table_name some_alias"); } + @ParameterizedTest // GH-2856 + @MethodSource("queryEnhancers") + @SuppressWarnings("NullableProblems") + void detectsQueryType(Function> enhancerFunction) { + + JpaQueryEnhancer select = enhancerFunction.apply("SELECT some_alias FROM table_name some_alias"); + assertThat(select.getQueryInformation().getStatementType()).isEqualTo(QueryInformation.StatementType.SELECT); + + JpaQueryEnhancer from = enhancerFunction.apply("FROM table_name some_alias"); + assertThat(from.getQueryInformation().getStatementType()).isEqualTo(QueryInformation.StatementType.SELECT); + + JpaQueryEnhancer delete = enhancerFunction.apply("DELETE FROM table_name some_alias"); + assertThat(delete.getQueryInformation().getStatementType()).isEqualTo(QueryInformation.StatementType.DELETE); + + JpaQueryEnhancer update = enhancerFunction + .apply("UPDATE table_name some_alias SET some_alias.property = ?1"); + assertThat(update.getQueryInformation().getStatementType()).isEqualTo(QueryInformation.StatementType.UPDATE); + + if (((Object) select) instanceof JpaQueryEnhancer.HqlQueryParser) { + + JpaQueryEnhancer insert = enhancerFunction.apply("INSERT Person(name) VALUES(?1)"); + assertThat(insert.getQueryInformation().getStatementType()).isEqualTo(QueryInformation.StatementType.INSERT); + } + } + static Stream>> queryEnhancers() { return Stream.of(JpaQueryEnhancer::forHql, JpaQueryEnhancer::forEql, JpaQueryEnhancer::forJpql); } diff --git a/spring-data-jpa/src/test/java/org/springframework/data/jpa/repository/query/QueryEnhancerUnitTests.java b/spring-data-jpa/src/test/java/org/springframework/data/jpa/repository/query/QueryEnhancerUnitTests.java index cf691219a7..f341b63e11 100644 --- a/spring-data-jpa/src/test/java/org/springframework/data/jpa/repository/query/QueryEnhancerUnitTests.java +++ b/spring-data-jpa/src/test/java/org/springframework/data/jpa/repository/query/QueryEnhancerUnitTests.java @@ -18,9 +18,7 @@ import static org.assertj.core.api.Assertions.*; import static org.assertj.core.api.Assumptions.*; -import java.util.Arrays; import java.util.Collections; -import java.util.Set; import java.util.stream.Stream; import org.assertj.core.api.SoftAssertions; @@ -43,6 +41,7 @@ * @author Geoffrey Deremetz * @author Krzysztof Krason * @author Mark Paluch + * @author Soomin Kim */ class QueryEnhancerUnitTests { @@ -196,10 +195,8 @@ void createCountQueryFromDeleteQuery() { DefaultEntityQuery query = new TestEntityQuery("delete from some_table where id in :ids", true); - // GH-2856: QueryEnhancer should throw exceptions for non-SELECT queries - assertThatThrownBy(() -> getEnhancer(query).createCountQueryFor("p.lastname")) - .isInstanceOf(IllegalStateException.class) - .hasMessageContaining("Cannot derive count query for DELETE statement"); + assertThatIllegalStateException().isThrownBy(() -> getEnhancer(query).createCountQueryFor("p.lastname")) + .withMessageContaining("Cannot derive count query for DELETE statement"); } @Test // DATAJPA-456 @@ -626,13 +623,10 @@ void modifyingQueriesAreDetectedCorrectly() { assertThat(modiQuery.getProjection()).isEqualToIgnoringCase(projectionNotConsideringQueryType); assertThat(modiQuery.hasConstructorExpression()).isEqualTo(constructorExpressionNotConsideringQueryType); - // QueryUtils returns original query (not aware of statement type) assertThat(countQueryForNotConsiderQueryType).isEqualToIgnoringCase(modifyingQuery); - // GH-2856: QueryEnhancer should throw exceptions for non-SELECT queries - assertThatThrownBy(() -> QueryEnhancer.create(modiQuery).createCountQueryFor(null)) - .isInstanceOf(IllegalStateException.class) - .hasMessageContaining("Cannot derive count query for UPDATE statement"); + assertThatIllegalStateException().isThrownBy(() -> QueryEnhancer.create(modiQuery).createCountQueryFor(null)) + .withMessageContaining("Cannot derive count query for UPDATE statement"); } @ParameterizedTest // GH-2593 @@ -647,8 +641,6 @@ void insertStatementIsProcessedSameAsDefault(String insertQuery) { // queryutils results String queryUtilsDetectAlias = QueryUtils.detectAlias(insertQuery); String queryUtilsProjection = QueryUtils.getProjection(insertQuery); - String queryUtilsCountQuery = QueryUtils.createCountQueryFor(insertQuery); - Set queryUtilsOuterJoinAlias = QueryUtils.getOuterJoinAliases(insertQuery); // direct access assertThat(stringQuery.getAlias()).isEqualToIgnoringCase(queryUtilsDetectAlias); @@ -656,20 +648,17 @@ void insertStatementIsProcessedSameAsDefault(String insertQuery) { assertThat(stringQuery.hasConstructorExpression()).isFalse(); // access over enhancer - // GH-2856: QueryEnhancer should throw exceptions for non-SELECT statements - assertThatThrownBy(() -> queryEnhancer.createCountQueryFor(null)) - .isInstanceOf(IllegalStateException.class) - .hasMessageContaining("Cannot derive count query for INSERT statement"); - - assertThatThrownBy(() -> queryEnhancer.rewrite(getRewriteInformation(sorting))) - .isInstanceOf(IllegalStateException.class) - .hasMessageContaining("Cannot apply sorting to INSERT statement"); + assertThatIllegalStateException().isThrownBy(() -> queryEnhancer.createCountQueryFor(null)) + .withMessageContaining("Cannot derive count query for INSERT statement"); + + assertThatIllegalStateException().isThrownBy(() -> queryEnhancer.rewrite(getRewriteInformation(sorting))) + .withMessageContaining("Cannot apply sorting to INSERT statement"); assertThat(queryEnhancer.detectAlias()).isEqualToIgnoringCase(queryUtilsDetectAlias); assertThat(queryEnhancer.getProjection()).isEqualToIgnoringCase(queryUtilsProjection); assertThat(queryEnhancer.hasConstructorExpression()).isFalse(); } - public static Stream insertStatementIsProcessedSameAsDefaultSource() { + static Stream insertStatementIsProcessedSameAsDefaultSource() { return Stream.of( // Arguments.of("INSERT INTO FOO(A) VALUES('A')"), // @@ -677,19 +666,6 @@ public static Stream insertStatementIsProcessedSameAsDefaultSource() ); } - public static Stream detectsJoinAliasesCorrectlySource() { - - return Stream.of( // - Arguments.of("select p from Person p left outer join x.foo b2_$ar", Collections.singletonList("b2_$ar")), // - Arguments.of("select p from Person p left join x.foo b2_$ar", Collections.singletonList("b2_$ar")), // - Arguments.of("select p from Person p left outer join x.foo as b2_$ar, left join x.bar as foo", - Arrays.asList("b2_$ar", "foo")), // - Arguments.of("select p from Person p left join x.foo as b2_$ar, left outer join x.bar foo", - Arrays.asList("b2_$ar", "foo")) // - - ); - } - private static void assertCountQuery(String originalQuery, String countQuery, boolean nativeQuery) { assertCountQuery(new TestEntityQuery(originalQuery, nativeQuery), countQuery); } @@ -701,61 +677,46 @@ private static void assertCountQuery(DefaultEntityQuery originalQuery, String co @Test // GH-2856 void jpqlInsertQueryThrowsExceptionForCountQuery() { - // Given: HQL INSERT query (HQL supports INSERT INTO ... SELECT) DeclaredQuery query = DeclaredQuery.jpqlQuery("INSERT INTO Foo(a) SELECT b.a FROM Bar b"); QueryEnhancer enhancer = QueryEnhancer.create(query); - // When/Then: Should throw IllegalStateException for count query - assertThatThrownBy(() -> enhancer.createCountQueryFor(null)) - .isInstanceOf(IllegalStateException.class) - .hasMessageContaining("Cannot derive count query for INSERT statement") - .hasMessageContaining("SELECT"); + assertThatIllegalStateException().isThrownBy(() -> enhancer.createCountQueryFor(null)) + .withMessageContaining("Cannot derive count query for INSERT statement").withMessageContaining("SELECT"); } @Test // GH-2856 void jpqlUpdateQueryThrowsExceptionForSorting() { - // Given: JPQL UPDATE query DeclaredQuery query = DeclaredQuery.jpqlQuery("UPDATE User SET name = 'test'"); QueryEnhancer enhancer = QueryEnhancer.create(query); - // When/Then: Should throw IllegalStateException for sorting Sort sort = Sort.by("id"); QueryEnhancer.QueryRewriteInformation rewriteInfo = new DefaultQueryRewriteInformation( sort, ReturnedType.of(Object.class, Object.class, new SpelAwareProxyProjectionFactory())); - assertThatThrownBy(() -> enhancer.rewrite(rewriteInfo)) - .isInstanceOf(IllegalStateException.class) - .hasMessageContaining("Cannot apply sorting to UPDATE statement") - .hasMessageContaining("SELECT"); + assertThatIllegalStateException().isThrownBy(() -> enhancer.rewrite(rewriteInfo)) + .withMessageContaining("Cannot apply sorting to UPDATE statement").withMessageContaining("SELECT"); } @Test // GH-2856 void jpqlDeleteQueryThrowsExceptionForCountQuery() { - // Given: JPQL DELETE query DeclaredQuery query = DeclaredQuery.jpqlQuery("DELETE FROM User u WHERE u.id = :id"); QueryEnhancer enhancer = QueryEnhancer.create(query); - // When/Then: Should throw IllegalStateException for count query - assertThatThrownBy(() -> enhancer.createCountQueryFor(null)) - .isInstanceOf(IllegalStateException.class) - .hasMessageContaining("Cannot derive count query for DELETE statement") - .hasMessageContaining("SELECT"); + assertThatIllegalStateException().isThrownBy(() -> enhancer.createCountQueryFor(null)) + .withMessageContaining("Cannot derive count query for DELETE statement").withMessageContaining("SELECT"); } @Test // GH-2856 void jpqlAllowsUnsortedForNonSelectQueries() { - // Given: JPQL UPDATE query DeclaredQuery query = DeclaredQuery.jpqlQuery("UPDATE User SET name = 'test'"); QueryEnhancer enhancer = QueryEnhancer.create(query); - // When: Unsorted (no sorting) QueryEnhancer.QueryRewriteInformation rewriteInfo = new DefaultQueryRewriteInformation( Sort.unsorted(), ReturnedType.of(Object.class, Object.class, new SpelAwareProxyProjectionFactory())); - // Then: Should not throw exception String result = enhancer.rewrite(rewriteInfo); assertThat(result).isEqualTo("UPDATE User SET name = 'test'"); } From 64dcdc1fd5baed264b4d2a4308a36e024abe95ed Mon Sep 17 00:00:00 2001 From: Mark Paluch Date: Tue, 21 Oct 2025 11:04:54 +0200 Subject: [PATCH 3/3] Remove deprecated API that is marked for removal. Closes #4057 --- .../query/JSqlParserQueryEnhancer.java | 41 +++++++++- .../jpa/repository/query/JSqlParserUtils.java | 78 ------------------- .../jpa/repository/query/JpaEntityGraph.java | 16 +--- .../data/jpa/repository/query/QueryUtils.java | 11 --- .../CustomHsqlHibernateJpaVendorAdaptor.java | 43 ---------- 5 files changed, 42 insertions(+), 147 deletions(-) delete mode 100644 spring-data-jpa/src/main/java/org/springframework/data/jpa/repository/query/JSqlParserUtils.java delete mode 100644 spring-data-jpa/src/test/java/org/springframework/data/jpa/repository/CustomHsqlHibernateJpaVendorAdaptor.java diff --git a/spring-data-jpa/src/main/java/org/springframework/data/jpa/repository/query/JSqlParserQueryEnhancer.java b/spring-data-jpa/src/main/java/org/springframework/data/jpa/repository/query/JSqlParserQueryEnhancer.java index 976b633b10..ec5bbf729b 100644 --- a/spring-data-jpa/src/main/java/org/springframework/data/jpa/repository/query/JSqlParserQueryEnhancer.java +++ b/spring-data-jpa/src/main/java/org/springframework/data/jpa/repository/query/JSqlParserQueryEnhancer.java @@ -15,12 +15,12 @@ */ package org.springframework.data.jpa.repository.query; -import static org.springframework.data.jpa.repository.query.JSqlParserUtils.*; import static org.springframework.data.jpa.repository.query.QueryUtils.*; import net.sf.jsqlparser.expression.Alias; import net.sf.jsqlparser.expression.Expression; import net.sf.jsqlparser.expression.Function; +import net.sf.jsqlparser.expression.operators.relational.ExpressionList; import net.sf.jsqlparser.parser.CCJSqlParser; import net.sf.jsqlparser.parser.CCJSqlParserUtil; import net.sf.jsqlparser.parser.ParseException; @@ -590,4 +590,43 @@ private static T deserializeRequired(byte @Nullable [] bytes, Class type) throw new IllegalStateException("Failed to deserialize object type"); } + /** + * Generates a count function call, based on the {@code countFields}. + * + * @param countFields the non-empty list of fields that are used for counting + * @param distinct if it should be a distinct count + * @return the generated count function call + */ + private static Function getJSqlCount(List countFields, boolean distinct) { + + List countColumns = new ArrayList<>(countFields.size()); + for (String countField : countFields) { + Column column = new Column(countField); + countColumns.add(column); + } + + ExpressionList countExpression = new ExpressionList<>(countColumns); + + return new Function() // + .withName("count") // + .withParameters(countExpression) // + .withDistinct(distinct); + } + + /** + * Generates a lower function call, based on the {@code column}. + * + * @param column the non-empty column to use as param for lower + * @return the generated lower function call + */ + private static Function getJSqlLower(String column) { + + List expressions = Collections.singletonList(new Column(column)); + ExpressionList lowerParamExpression = new ExpressionList<>(expressions); + + return new Function() // + .withName("lower") // + .withParameters(lowerParamExpression); + } + } diff --git a/spring-data-jpa/src/main/java/org/springframework/data/jpa/repository/query/JSqlParserUtils.java b/spring-data-jpa/src/main/java/org/springframework/data/jpa/repository/query/JSqlParserUtils.java deleted file mode 100644 index 7c557b5c22..0000000000 --- a/spring-data-jpa/src/main/java/org/springframework/data/jpa/repository/query/JSqlParserUtils.java +++ /dev/null @@ -1,78 +0,0 @@ -/* - * Copyright 2022-2025 the original author or authors. - * - * Licensed under the Apache License, Version 2.0 (the "License"); - * you may not use this file except in compliance with the License. - * You may obtain a copy of the License at - * - * https://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, software - * distributed under the License is distributed on an "AS IS" BASIS, - * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. - * See the License for the specific language governing permissions and - * limitations under the License. - */ -package org.springframework.data.jpa.repository.query; - -import net.sf.jsqlparser.expression.Expression; -import net.sf.jsqlparser.expression.Function; -import net.sf.jsqlparser.expression.operators.relational.ExpressionList; -import net.sf.jsqlparser.schema.Column; - -import java.util.ArrayList; -import java.util.Collections; -import java.util.List; - -/** - * A utility class for JSqlParser. - * - * @author Diego Krupitza - * @author Greg Turnquist - * @since 2.7.0 - * @deprecated internal utility class. - */ -@Deprecated -public final class JSqlParserUtils { - - private JSqlParserUtils() {} - - /** - * Generates a count function call, based on the {@code countFields}. - * - * @param countFields the non-empty list of fields that are used for counting - * @param distinct if it should be a distinct count - * @return the generated count function call - */ - public static Function getJSqlCount(List countFields, boolean distinct) { - - List countColumns = new ArrayList<>(countFields.size()); - for (String countField : countFields) { - Column column = new Column(countField); - countColumns.add(column); - } - - ExpressionList countExpression = new ExpressionList<>(countColumns); - - return new Function() // - .withName("count") // - .withParameters(countExpression) // - .withDistinct(distinct); - } - - /** - * Generates a lower function call, based on the {@code column}. - * - * @param column the non-empty column to use as param for lower - * @return the generated lower function call - */ - public static Function getJSqlLower(String column) { - - List expressions = Collections.singletonList(new Column(column)); - ExpressionList lowerParamExpression = new ExpressionList<>(expressions); - - return new Function() // - .withName("lower") // - .withParameters(lowerParamExpression); - } -} diff --git a/spring-data-jpa/src/main/java/org/springframework/data/jpa/repository/query/JpaEntityGraph.java b/spring-data-jpa/src/main/java/org/springframework/data/jpa/repository/query/JpaEntityGraph.java index cf85c65d47..a40966156d 100644 --- a/spring-data-jpa/src/main/java/org/springframework/data/jpa/repository/query/JpaEntityGraph.java +++ b/spring-data-jpa/src/main/java/org/springframework/data/jpa/repository/query/JpaEntityGraph.java @@ -17,9 +17,9 @@ import java.util.List; -import org.springframework.data.jpa.repository.EntityGraph; - import org.jspecify.annotations.Nullable; + +import org.springframework.data.jpa.repository.EntityGraph; import org.springframework.data.jpa.repository.EntityGraph.EntityGraphType; import org.springframework.util.Assert; import org.springframework.util.StringUtils; @@ -95,18 +95,6 @@ public List getAttributePaths() { return attributePaths; } - /** - * Return {@literal true} if this {@link JpaEntityGraph} needs to be generated on-the-fly. - * - * @return {@literal true} if {@link #attributePaths} is not empty. - * @since 1.9 - * @deprecated since 3.5 as the used evaluation does not represent whether a {@link JpaEntityGraph} is dynamic or not. - */ - @Deprecated(since = "3.5", forRemoval = true) - public boolean isAdHocEntityGraph() { - return !attributePaths.isEmpty(); - } - @Override public String toString() { return "JpaEntityGraph [name=" + name + ", type=" + type + ", attributePaths=" + attributePaths.toString() + "]"; diff --git a/spring-data-jpa/src/main/java/org/springframework/data/jpa/repository/query/QueryUtils.java b/spring-data-jpa/src/main/java/org/springframework/data/jpa/repository/query/QueryUtils.java index 2a1d4b6b01..aee5df5cef 100644 --- a/spring-data-jpa/src/main/java/org/springframework/data/jpa/repository/query/QueryUtils.java +++ b/spring-data-jpa/src/main/java/org/springframework/data/jpa/repository/query/QueryUtils.java @@ -682,17 +682,6 @@ public static boolean hasNamedParameter(Query query) { return false; } - /** - * Returns whether the given query contains named parameters. - * - * @param query can be {@literal null} or empty. - * @return whether the given query contains named parameters. - */ - @Deprecated - static boolean hasNamedParameter(@Nullable String query) { - return StringUtils.hasText(query) && NAMED_PARAMETER.matcher(query).find(); - } - /** * Turns the given {@link Sort} into {@link jakarta.persistence.criteria.Order}s. * diff --git a/spring-data-jpa/src/test/java/org/springframework/data/jpa/repository/CustomHsqlHibernateJpaVendorAdaptor.java b/spring-data-jpa/src/test/java/org/springframework/data/jpa/repository/CustomHsqlHibernateJpaVendorAdaptor.java deleted file mode 100644 index 359bda9b5e..0000000000 --- a/spring-data-jpa/src/test/java/org/springframework/data/jpa/repository/CustomHsqlHibernateJpaVendorAdaptor.java +++ /dev/null @@ -1,43 +0,0 @@ -/* - * Copyright 2012-2025 the original author or authors. - * - * Licensed under the Apache License, Version 2.0 (the "License"); - * you may not use this file except in compliance with the License. - * You may obtain a copy of the License at - * - * https://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, software - * distributed under the License is distributed on an "AS IS" BASIS, - * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. - * See the License for the specific language governing permissions and - * limitations under the License. - */ -package org.springframework.data.jpa.repository; - -import org.hibernate.dialect.HSQLDialect; -import org.springframework.orm.jpa.vendor.Database; -import org.springframework.orm.jpa.vendor.HibernateJpaVendorAdapter; - -/** - * Fix for missing type declarations for HSQL. - * - * @see https://www.codesmell.org/blog/2008/12/hibernate-hsql-native-queries-and-booleans/ - * @author Oliver Gierke - * @deprecated since 3.0 without replacement as it's not needed anymore. - */ -@Deprecated -public class CustomHsqlHibernateJpaVendorAdaptor extends HibernateJpaVendorAdapter { - - @Override - protected Class determineDatabaseDialectClass(Database database) { - return super.determineDatabaseDialectClass(database); - } - - /** - * @deprecated since 3.0 without replacement as it's not needed anymore. - */ - @Deprecated - public static class CustomHsqlDialect extends HSQLDialect {} -}