Provide an option to disable `JSqlParserQueryEnhancer`

[onukristo]

trafficstars

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: https://github.com/transferwise/tw-entrypoints/pull/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.

[Danli741]

Any update for this issue and why is #3148 closed?

We are facing the same problem. Due to the functionalities of our app, we have plenty of native queries in repositories, and the temporary threads created by JsqlParser bother.

[pdodds]

Is there any news on the ability to disable JsqlParser, we have introduced it for another use-case but now Spring Data is failing due to the use of vector based queries in our SQL?

[mp911de]

For the time being, if JSqlParser hinders you, please implement custom repository fragments and use the EntityManager directly. We won't get to work on this one nearterm.

[oburgosm]

We had a SQL native query working fine without jsqlparser. But after an upgrade of spring-data, which come with jsqlparser due to spring-projects/spring-data-relational#1572, our code has started to fail. I think it's important to add an option to disable jsqlparser for this reason.

[marclallen]

Another person looking for a solution here. We finally decided to move from Sprintboot 2.6 to the latest and we ran into this. I don't know exactly what enhancements or functionality JSQLParser provides, but it breaks my code because, as has been noted, it's not SQL complete for every dialect. In my case, it's 'unnest' (and possibly others) in postgresql.

This is preventing us from moving to SB 3.2.3, and would very much like a solution.

[dhavalkumarthakkar]

Hi Team, we are moving from SB 3.1.x to SB 3.2.X and started getting error for existing query. Do we have any option to disable jsqlparser yet? It will block us to use new the version. Appreciate quick solution.

[marclallen]

Hi Team, we are moving from SB 3.1.x to SB 3.2.X and started getting error for existing query. Do we have any option to disable jsqlparser yet? It will block us to use new the version. Appreciate quick solution.

Here's what I did (and I got it from much higher in the thread, or possibly a different one... I forget). This removes the JSqlParser and, I think, uses the old method.

<dependency>
  <groupId>org.springframework.boot</groupId>
  <artifactId>spring-boot-starter-data-jdbc</artifactId>
  <exclusions>
    <exclusion>
      <groupId>com.github.jsqlparser</groupId>
      <artifactId>jsqlparser</artifactId>
    </exclusion>
  </exclusions>
</dependency>

[DmitriyBrashevets]

Hi Team, we are moving from SB 3.1.x to SB 3.2.X and started getting error for existing query. Do we have any option to disable jsqlparser yet? It will block us to use new the version. Appreciate quick solution.

Here's what I did (and I got it from much higher in the thread, or possibly a different one... I forget). This removes the JSqlParser and, I think, uses the old method.

<dependency>
  <groupId>org.springframework.boot</groupId>
  <artifactId>spring-boot-starter-data-jdbc</artifactId>
  <exclusions>
    <exclusion>
      <groupId>com.github.jsqlparser</groupId>
      <artifactId>jsqlparser</artifactId>
    </exclusion>
  </exclusions>
</dependency>

Exclusion helps to resolve the upsert issue with PG native query.

[mp911de]

We have a design to provide QueryEnhancer for a DeclaredQuery (query string, native true/false) from a QueryEnhancerSelector that could be configured via @EnableJpaRepositories(queryEnhancerSelector = …). Our design requires a bit of revision as DeclaredQuery represents actually an introspected query so we need to refine contracts on that end.

Applications can provide their own QueryEnhancerSelector class and make use of factory methods such as QueryEnhancerFactory.jsqlparser(), QueryEnhancerFactory.hql(), QueryEnhancerFactory.fallback() (the regex-variant) on a per-query basis.