Skip to content

Conversation

kssumin
Copy link
Contributor

@kssumin kssumin commented Oct 18, 2025

Changes

This PR addresses issue #2856 by adding proper validation to QueryEnhancer implementations to prevent invalid operations on non-SELECT statements.

Summary

Previously, QueryEnhancer would silently return default values or apply inappropriate operations when attempting to:

  • Create count queries for INSERT/UPDATE/DELETE/MERGE statements
  • Apply sorting to INSERT/UPDATE/DELETE/MERGE statements

This behavior was problematic as it could lead to unexpected results or silent failures.

What Changed

  1. Statement Type Detection

    • Added StatementType enum to QueryInformation class
    • Implemented statement type detection using regex pattern matching in DefaultQueryEnhancer
    • Extended introspector classes (JpqlQueryIntrospector, HqlQueryIntrospector, EqlQueryIntrospector) to detect and propagate statement types
  2. Validation Logic

    • QueryEnhancer.rewrite(): Now throws IllegalStateException when attempting to apply sorting to non-SELECT statements
    • QueryEnhancer.createCountQueryFor(): Now throws IllegalStateException when attempting to create count queries for non-SELECT statements
    • Unsorted rewriting is still allowed for all statement types (no-op behavior)
  3. Affected Classes

    • DefaultQueryEnhancer: Added statement type detection and validation
    • JpaQueryEnhancer: Added validation for sorting and count queries
    • JSqlParserQueryEnhancer: Changed from returning original query to throwing exceptions
    • QueryInformation: Added StatementType enum and helper methods
    • HibernateQueryInformation: Extended to support statement type
  4. Test Coverage

    • Added test cases to QueryEnhancerUnitTests for JPQL queries:
      • INSERT/UPDATE/DELETE statements throwing exceptions for count queries
      • INSERT/UPDATE/DELETE statements throwing exceptions for sorting
      • Unsorted operations working for all statement types
    • Added test cases to JSqlParserQueryEnhancerUnitTests for native queries:
      • INSERT/UPDATE statements throwing exceptions for count queries and sorting
      • Unsorted operations working for all statement types

Closes: #2856

  • You have read the Spring Data contribution guidelines.
  • You use the code formatters provided here and have them applied to your changes. Don’t submit any formatting related changes.
  • You submit test cases (unit or integration tests) that back your changes.
  • You added yourself as author in the headers of the classes you touched. Amend the date range in the Apache license header if needed. For new types, add the license header (copy from another file and set the current year only).

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 <ksoomin25@gmail.com>
@mp911de mp911de self-assigned this Oct 20, 2025
@mp911de mp911de added type: task A general task and removed status: waiting-for-triage An issue we've not yet triaged labels Oct 20, 2025
@mp911de mp911de added this to the 4.0 GA (2025.1.0) milestone Oct 21, 2025
mp911de pushed a commit that referenced this pull request Oct 21, 2025
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 <ksoomin25@gmail.com>
Original pull request: #4052
Closes #2856
mp911de added a commit that referenced this pull request Oct 21, 2025
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
@mp911de
Copy link
Member

mp911de commented Oct 21, 2025

Thank you for your contribution. That's merged and polished now.

The summary above looks as if it was AI-generated from the changeset which is basically repeating all technical aspects that are immediately visible from the change itself. For the future, descriptions should rather talk about the motivation and aspects that are introduced by a change as repeating what the diff is going to contain is rather noise and distracting from the actual change.

@mp911de mp911de closed this Oct 21, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

type: task A general task

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Revisit QueryEnhancer exception handling

3 participants