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: 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.

[MosheElisha]

Hi,

In our case, we want to use jsqlparser for a completely different use case so we need the dependency and cannot use exclusions but we still want to disable JSqlParserQueryEnhancer because is throws The query is not a valid SQL Query on valid queries.

Please allow disabling JSqlParserQueryEnhancer.

[MosheElisha]

Another option is perhaps JSqlParserQueryEnhancer (and dependent classes) should be in a separate jar (e.g., spring-data-jpa-query-enhancer-jsql) and only enable JSqlParserQueryEnhancer if JSqlParserQueryEnhancer is in the classpath.

In my project we want to use JSQLParser 5 for other purposes and we don't want it to be used by spring-data-jpa.

[MosheElisha]

@mp911de what do you think about the separate jar approach (e.g., spring-data-jpa-query-enhancer-jsql)? I think this will be best as it will solve the original issue and will decouple spring-data-jpa from jsqlparser.

If not, what about a flag to disable JSqlParserQueryEnhancer ?

I saw the work that was done on https://github.com/spring-projects/spring-data-jpa/pull/3148 . Personally, I don't want to define enhancer strategy per query and I don't want to have many warnings in case I decided not to use JSqlParserQueryEnhancer.

[mp911de]

@mp911de what do you think about the separate jar approach (e.g., spring-data-jpa-query-enhancer-jsql)?

It is one of the worse options.

3148 isn't a great way to solve the issue because of the mentioned shortcomings. There's an alternative with #3527 that requires some breaking changes around DeclaredQuery as the current design doesn't really allow for external customization. The constructor of StringQuery performs already a lookup of QueryEnhancerFactory via this.

[MosheElisha]

@mp911de :-) OK, thank you. https://github.com/spring-projects/spring-data-jpa/pull/3148 indeed looks better.

Looking forward to this! Thank you!

[mp911de]

After discussing the issue within the team, we concluded that we want to split efforts here. Short-term we would introduce a configuration property through SpringProperties (spring.properties on the class path) to provide a switch for disabling JSqlParser. The follow up #3622 would then address the proper design and allowing to select a QueryEnhancerStrategy through @EnableJpaRepositories.

[MosheElisha]

@mp911de Sounds good. Thank you for considering a short-term solution as I assume this will be delivered faster.

[MosheElisha]

@mp911de Thank you and the team. Much appreciated.

[MosheElisha]

@mp911de Thank you. In which release(s) can we expect this enhancement? Can we get it on the next 3.3.X?

[mp911de]

We assign the earliest milestone to our tickets to indicate since when a change will be/was available. In this case, the change is going to ship with service releases 3.2.11 and 3.3.5 and the upcoming 3.4 RC1.

[MosheElisha]

@mp911de Verified that spring.data.jpa.query.native.parser=regex is working as expected with version 3.3.5

Thanks a lot!