spring-projects / spring-data-jpa

Simplifies the development of creating a JPA-based data access layer.
https://spring.io/projects/spring-data-jpa/
Apache License 2.0
3.02k stars 1.42k forks source link

Pageable warnings incosistent or wrong #3660

Open dsuepke-swisscom opened 2 weeks ago

dsuepke-swisscom commented 2 weeks ago

Hi,

I have two basically equivalent queries. One of them (findFoo) gives a warning on startup, one (findFoo2) doesn't. The warning seems to be correct, because in our project the query gets very slow.

Specific issues:

Minimal example:

Log output

 :: Spring Boot ::                (v3.3.5)

2024-11-06T08:53:52.872+01:00  WARN 63873 --- [demo] [           main] JpaBaseConfiguration$JpaWebConfiguration : spring.jpa.open-in-view is enabled by default. Therefore, database queries may be performed during view rendering. Explicitly configure spring.jpa.open-in-view to disable this warning
2024-11-06T08:53:53.107+01:00  WARN 63873 --- [demo] [           main] o.s.d.jpa.repository.query.NamedQuery    : Finder method public abstract org.springframework.data.domain.Page com.example.demo.FooRepository.findFoo(java.lang.String,org.springframework.data.domain.Pageable) is backed by a NamedQuery but contains a Pageable parameter; Sorting delivered via this Pageable will not be applied

FooRepository

@Repository
public interface FooRepository extends JpaRepository<FooEntity, Long>  {

    @Query(name = "FooEntity.findFoo", countName = "FooEntity.findFoo.count", nativeQuery = true)
    Page<FooEntity> findFoo(@Param("foo") String foo, Pageable pageable);

    @Query(value = "SELECT * FROM foo WHERE foo='foo'", countQuery = "SELECT count(*) FROM foo WHERE foo='foo'", nativeQuery = true)
    Page<FooEntity> findFoo2(@Param("foo") String foo, Pageable pageable);
}

FooEntity

@Entity
@Getter
@Setter
@NamedNativeQuery(name = "FooEntity.findFoo", query = "SELECT * FROM foo WHERE foo='foo'")
@NamedNativeQuery(name = "FooEntity.findFoo.count", query = "SELECT count(*) FROM foo WHERE foo='foo'")
public class FooEntity {

    @Id
    @Column
    @GeneratedValue(strategy = GenerationType.SEQUENCE)
    private long id;

    @Column
    private String foo;

}

build.gradle

plugins {
    id 'java'
    id 'org.springframework.boot' version '3.3.5'
    id 'io.spring.dependency-management' version '1.1.6'
}

group = 'com.example'
version = '0.0.1-SNAPSHOT'

java {
    toolchain {
        languageVersion = JavaLanguageVersion.of(21)
    }
}

configurations {
    compileOnly {
        extendsFrom annotationProcessor
    }
}

repositories {
    mavenCentral()
}

dependencies {
    implementation 'org.springframework.boot:spring-boot-starter-data-jpa'
    implementation 'org.springframework.boot:spring-boot-starter-web'
    compileOnly 'org.projectlombok:lombok'
    runtimeOnly 'com.h2database:h2'
    annotationProcessor 'org.projectlombok:lombok'
    testImplementation 'org.springframework.boot:spring-boot-starter-test'
    testRuntimeOnly 'org.junit.platform:junit-platform-launcher'
}

tasks.named('test') {
    useJUnitPlatform()
}
Seol-JY commented 2 weeks ago

I’ve already completed the development to address this issue, and the changes are ready to be submitted in a PR. If the issue is valid, would it be okay for me to propose the changes via the PR?

mp911de commented 2 weeks ago

@Seol-JY How do you plan to address the issue? What changes do you want to propose? Let's please discuss these upfront.

@dsuepke-swisscom to your question:

One query method uses named queries while the other declares a query string. That is a significant difference for us. To be able to apply sorting, we need to be in control over the actual query string. JPA's EntityManager allows us only calling EntityManager.createNamedQuery(queryName) after which we can only bind parameters and set limit/offset. We cannot alter the query itself.

In the second case, where you provide a SQL string, we are parsing the SQL query and injecting the given sort order.

Another aspect is that named queries are often associated times with additional information (e.g., @NamedNativeQuery(resultSetMapping = …, fetchSize = …,) so we cannot extract the query and run the query as it was a pure String-based query.

Let me know whether this makes sense.

Seol-JY commented 2 weeks ago

Thank you for your clarification, @mp911de

I had initially developed a solution assuming this was a valid issue, with changes ready for implementation. However, after reviewing your explanation of how named queries and string-based queries are handled differently - particularly regarding sort order capabilities and metadata handling - I now understand that the current warning behavior is intentional and technically correct. I'll withdraw my proposed changes as they would have added unnecessary warnings that don't align with the actual technical implementation. Thank you for clarifying the architectural reasoning behind this difference.

dsuepke-swisscom commented 2 weeks ago

Hi, thanks to the both of you. I understand the technical limitations and it makes sense, was kinda expecting this to be the underlying cause. So the warning itself is correct and I appreciate it. Possibly, there is still room for improvement:

In summary, I think two follow-up issues arise from your helpful explanations: a) The warning should indicate at a solution rather than the problem, and b) there must be a way to get rid of the warning once it has been solved to keep a clean log.

mp911de commented 2 weeks ago

In that regard, what actually is the recommended course of action? In the real application we are indeed using a resultSetMapping, inlining does not seem possible as the normal Query annotation does not support it.

Thanks for the detail. Have you tried switching to our new @NativeQuery annotation (available in the latest milestones and RC1 release). It allows to specify @NativeQuery(sqlResultSetMapping=…).

Alternatively, you could switch to Limit and ScrollPosition parameters while returning Window<T>.

Once you can confirm that @NativeQuery(sqlResultSetMapping=…) is working for you, we could update our log message to include this suggestion.

dsuepke-swisscom commented 1 week ago

Sorry, had to work on another project for a few days. I would like to try @NativeQuery, but I cannot figure out how to import that into our gradle project. We have these two, which seem relevant:

plugins {
    id 'org.springframework.boot' version '3.3.0'
    ...
}
dependencies {
    implementation 'org.springframework.boot:spring-boot-starter-data-jpa'
    ...
}

I cannot find an RC on mavencentral. I found some release notes on the JPA site, but no instructions on how to use it. Could you kindly help me on how to use the mentioned RC for NativeQuery?

EDIT: Changed the plugin, copied the wrong line.

mp911de commented 1 week ago

The easiest might be going to start.spring.io and generating a new Gradle project with Boot 3.4 RC1. That allows you to copy the milestone repo declaration. Am 15. Nov. 2024, 08:37 +0100 schrieb dsuepke-swisscom @.***>:

Sorry, had to work on another project for a few days. I would like to try @NativeQuery, but I cannot figure out how to import that into our gradle project. We have these two, which seem relevant: plugins { id 'com.github.node-gradle.node' version '3.6.0 ... } dependencies { implementation 'org.springframework.boot:spring-boot-starter-data-jpa' ... } I cannot find an RC on mavencentral. I found some release notes on the JPA site, but no instructions on how to use it. Could you kindly help me on how to use the mentioned RC for NativeQuery? — Reply to this email directly, view it on GitHub, or unsubscribe. You are receiving this because you were mentioned.Message ID: @.***>