JSqlParserQueryEnhancer fails to parse select query with except or union operator

[gauravjhs]

Hello Team.

Upgrading to Spring Data 2.7.x and having com.github.jsqlparser in the classpath prevents the application to start if repositories use native queries.

Caused by: java.lang.ClassCastException: class net.sf.jsqlparser.statement.select.SetOperationList cannot be cast to class net.sf.jsqlparser.statement.select.PlainSelect (net.sf.jsqlparser.statement.select.SetOperationList and net.sf.jsqlparser.statement.select.PlainSelect


A query similar to

select SOME_COLUMN from SOME_TABLE where REPORTING_DATE = :REPORTING_DATE  
except 
select SOME_COLUMN from SOME_OTHER_TABLE where REPORTING_DATE = :REPORTING_DATE

fails to parse because jsqlparser returns a SetOperationList object instead of PlainSelect
This is directly typecasted in class JSqlParserQueryEnhancer

private String detectAlias(String query) {
  if (this.parsedType != ParsedType.SELECT) {
	  return null;
  }
 
  Select selectStatement = parseSelectStatement(query);
  PlainSelect selectBody = (PlainSelect) selectStatement.getSelectBody();
  return detectAlias(selectBody);
}

It would be useful if we can give a flag to choose which QueryEnhancer user wants to use. There will be multiple cases where jsqlparser is on the classpath for historical project but either they won't be able to upgrade because of breaking changes in jsqlparser or they do not intend to use is as QueryEnhancer.

[DiegoKrupitza]

Thank you for raising that issue!

I identified two things that need fixing in my opinion.

1. Enhancing the JSqlParserQueryEnhancer

In my first implementation I did not notice that there are more implementations of "SelectBody". In total there are 4 of them (see screenshot 1).

Right now we only support one of them (PlainSelect) as you pointed out. We should definitely support all of them although for SetOperationList it might be a bit tricky. When I take your query example from above and for example feed it to QueryUtils.createCountQuery(...) (which is basically what the default implementation DefaultQueryEnhancer will use) a wrong count query will be generated (see following code snippet). So when implementing the JSQLParser equivalent we have to define a correct behaviour.

select count(SOME_COLUMN) from SOME_TABLE where REPORTING_DATE = :REPORTING_DATE  
except 
select SOME_COLUMN from SOME_OTHER_TABLE where REPORTING_DATE = :REPORTING_DATE

2. Make Selection of QueryEnhancer Configurable

You are totally right we should make it configurable. Another prime example here is #2564 . I think that this configuration could happen per repository (maybe an annotation like @PreferedQueryEnhancer(QueryEnhancerPreference.DEFAULT) on the repository) or on a global level (some property in the application.yaml). I think this design decision needs to be made by the maintainers.

[gauravjhs]

Makes sense.
Request to the maintainers to add a flag to turn off this feature as it will help people currently using JSQLParser to upgrade easily.

[gregturn]

From a general standpoint, we don't want to introduce lots of configuration options. JSqlParser is supposed to be a single option, chosen by using the native=true flag.

If it's easy to add support for this operation @DiegoKrupitza, then fine. We can review that.

But if this is going to involve "options", then I'm inclined to say this moves beyond the scope of anything we want to maintain directly inside Spring Data JPA.

I realize that you have a hard-coded cast a cited earlier in this ticket. If you wish to add a defensive if-clause that checks selectStatement instanceof PlainSelect, and then throw a RuntimeException because we don't support any of the more complex, we could at least build up a better error message as to the problem at hand.

[gauravjhs]

Thanks, @gregturn for your insight.
Just to get more understanding on the issue, if JSqlParser is the only option for native queries and if we cannot support all types of selectStatement options and throws a RuntimeException, what options do we have to resolve the issue as we won't be able to get away from native queries in certain scenarios.

[DiegoKrupitza]

With the new PR union and except should be supported, which means a query as stated above should not face any issues anymore. Few of the QueryEnhancer methods return empty string or null since it does not make sense to return values here. All in all the "important" features that may be needed in further development are covered.