Try to optionally use JSqlParser for parsing SQL

[schauder]

https://github.com/JSQLParser/JSqlParser Seems to be a decent solution to the problem of SQL parsing.
We also have a lot of issue related to SQL and JPQL parsing.

Therefore we want to:

• implement everything related to SQL parsing using JSqlParser
• use that implementation when JSqlParser is on the classpath.

The downside of having two parser implementations is, that it might get confusing, what is going on for the user.
Therefore we should:

• During startup log an INFO message what parser we are using.
• If possible define well defined limits of the standard parser like: We can't parse anything with nested parenthesis and mention that information in documentation and in error messages.

[DiegoKrupitza]

I think having two different parsers, as you already said, might confuse users. That's why I was thinking to first introduces the new parsers and deprecate the old one.
So far as I can see the old parser could be "translated" into the new framework (I did not went into details but it seems possible).
Therefore if it can really replace the old custom implementation deprecation and later removal should be considered.

[schauder]

There are two problems with this:

1. We also have to parse JPQL, which is a completely different story.
2. We don't want to require an external library. It is a basic principle we follow to minimise required dependencies to third party libraries.

[DiegoKrupitza]

ok this makes sense.
I will try to take a look at this enhancement. I would say I start with something simple such as org.springframework.data.jpa.repository.query.QueryUtils. Since here we do a lot of Regex matching etc just to get and detect certain parts of an SQL query and introducing JSqlParser here would bring a big benefit in terms of fault prevention, readability etc.

It will take some time since right now it is exam season.

[DiegoKrupitza]

So I now translated the most important methods from org.springframework.data.jpa.repository.query.QueryUtils into their JSqlParser counterpart. All the changes are inside org.springframework.data.jpa.repository.query.JSqlParserQueryUtils.

So far I can see I did not miss anything SQL related but it would be nice if someone could look over it.
Furthermore, it is needed to somehow use JSqlParserQueryUtils over QueryUtils when it is a SQL query. I could not figure out how to do this within the spring-data-jpa ecosystem (prob due to missing project related knowledge). Maybe someone can help me there (or if it is faster maybe hook into my current state and "finish" this part).

[DiegoKrupitza]

Although JSqlParser is great for parsing the SQL there are still some limitations. I think this should be included into the final documentation. Due to this "bug" I had to adjust few test cases.

JSQLParser/JSqlParser#1462

[schauder]

Furthermore, it is needed to somehow use JSqlParserQueryUtils over QueryUtils when it is a SQL query.
Here is how I would approach this:

1. For every method in QueryUtils accessed from outside the class, create a new variant, that takes a StringQuery class.
2. StringQuery combines just two fields: The query string and a flag denoting if it is a native query.
3. deprecate the old methods that use strings directly.
4. cascade the change through the code base. This will lead you to all the places where query strings come into the system.
5. In QueryUtils delegate to JSqlParserQueryUtils for native queries.
6. extract the actual work of QueryUtils into another class (SimpleParserQueryUtils ???).

HTH

[schauder]

Although JSqlParser is great for parsing the SQL there are still some limitations.
As long as it isn't worse then what we use now this shouldn't be to much of a problem.
Thanks for raising an issue with them.

[schauder]

So far I can see I did not miss anything SQL related but it would be nice if someone could look over it.
Could you create PR for that?

[DiegoKrupitza]

Furthermore, it is needed to somehow use JSqlParserQueryUtils over QueryUtils when it is a SQL query.
Here is how I would approach this:

1. For every method in QueryUtils accessed from outside the class, create a new variant, that takes a StringQuery class.
2. StringQuery combines just two fields: The query string and a flag denoting if it is a native query.
3. deprecate the old methods that use strings directly.
4. cascade the change through the code base. This will lead you to all the places where query strings come into the system.
5. In QueryUtils delegate to JSqlParserQueryUtils for native queries.
6. extract the actual work of QueryUtils into another class (SimpleParserQueryUtils ???).

HTH

Ok thank you! I will take a look at this later! 👍

PS: the first line of your response is still within the "quote" of my message

[DiegoKrupitza]

Although JSqlParser is great for parsing the SQL there are still some limitations.
As long as it isn't worse then what we use now this shouldn't be to much of a problem.
Thanks for raising an issue with them.

I think we should clearly state somewhere that parsing errors could still happen, although the query is valid sql and that this could be due to some internals of the JSqlParser. So that the user knows that a possible short term fix could be to use a different query or use the "custom parser".

[DiegoKrupitza]

So far I can see I did not miss anything SQL related but it would be nice if someone could look over it.
Could you create PR for that?

Ok I can open a PR later but it is still a draft.