Provide an option to disable `JSqlParserQueryEnhancer`

[onukristo]

When JSqlParser is in classpath, the org.springframework.data.jpa.repository.query.QueryEnhancerFactory enables org.springframework.data.jpa.repository.query.JSqlParserQueryEnhancer.

However JSqlParser is quite CPU and memory allocation expensive, and it creates a new thread every time a query gets parsed.
Also, while the JSqlParser is very useful for various use cases, it is not perfect, and can not parse all the queries. We have encountered even bugs, when parsing a more complex SQL, goes to endless loop.

One of our services reported 15% more CPU total, just from this component.

Essentially, we have JSqlParser in classpath for other reasons, but don't want all the negative effects from JSqlParserQueryEnhancer.

Currently we disable it via byte-code manipulation:

new ByteBuddy()
          .redefine(clazz)
          .method(named("isJSqlParserInClassPath"))
          .intercept(FixedValue.value(false))
          .make()
          .load(clazz.getClassLoader(), ClassReloadingStrategy.fromInstalledAgent());

However, it is not maintainable solution as we would most likely need to revisit this every time spring-data-jpa gets upgraded.

Please provide another mechanism, preferably a Spring Boot property, to disable this mechanism.

[mp911de]

Do you happen to have a profiler recording that you could attach to this ticket? I'm wondering whether we could generally have some caching to avoid parsing the query over and over again.

[onukristo]

We have, but unfortunately I can not share any internals of our services. Corporate rules :)

But looking the profile, the caching would help greatly for sure and would fix the issue for us, I think.

For example we have a public library for measuring application side database access, via JSqlParser. We use cache there.

I coincidentally just added a mechanism against the JSqlParser creating new threads and against those endless loops on rare queries as well: transferwise/tw-entrypoints#31

A similar endless loops/long parsing time protection, could also implemented together with the caching in spring-data-jpa, just in case.

[gregturn]

I'm wondering if there isn't a suitable way to "opt out" via a Spring Boot property setting that in turn could feed into Spring Data JPA?

If we're needing a method-by-method switch on when to use it, I'm not really a fan of that. But simply disabling it for native query handling in general. Then perhaps.

[spring-projects-issues]

If you would like us to look at this issue, please provide the requested information. If the information is not provided within the next 7 days this issue will be closed.

[llehtinen]

@gregturn The request was for a property to disable it altogether. As far as we know, it does not exist today.

As @onukristo mentioned in the description, resorting to bytecode seems to be the only way (unless one removes JSqlParser from classpath).

[mp911de]

#3039 should remove some pressure as we now use the parsed statement for detection operations. It should be possible to construct count and sorting query enhancements from the parsed AST as we're creating a query derived (projection, ordering) from the original query without actually changing things.

If we copy over immutable parts and render from the updated query, then we should see parsing only once.

[onukristo]

There is still a problem, that jsqlparser is not complete and is not able to parse all queries.

For example in one of our services we have the following repository code:

    @Modifying
    @Query(value = "INSERT INTO table(a, b, c, d)" +
        "VALUES (:a, :b, :c, :d)" +
        "ON CONFLICT (a, b)" +
        "DO UPDATE " +
        "SET c = :c, d = :d, last_updated = NOW(), version = a.version + 1", nativeQuery = true)
    void saveA(@Param("a") String a, @Param("b") long b, @Param("c") String c, @Param(value = "d") String d);

Now, because JSqlParser can not parse a query where "ON CONFLICT" has two parameters, the spring-data is throwing an error and preventing the application to start up.