OGM-1472 Rework of rendering JPQL into Ignite SQL #19
Conversation
Salauyou
changed the title
|
Thanks @Salauyou. It looks very interesting. I will have a look at it soon. @schernolyas Maybe you can have a look at this as well? |
|
Thanks a lot |
|
@schernolyas As I know, you're working on supporting querying by associations (which requires joins), so I'd like you to review if it would be easy to add such support. I tried not to change |
| for ( Map.Entry<String, TypedGridValue> entry : queryParameters.getNamedParameters().entrySet() ) { | ||
| indexedParameters.add( entry.getValue().getValue() ); | ||
| List<Object> parameterValues; | ||
| if ( !queryParameters.getPositionalParameters().isEmpty() ) { |
There was a problem hiding this comment.
I see you remove support of named parameters. I think it is correct because Ignite SQL supports positional parameters only.
There was a problem hiding this comment.
Not exactly, I just changed the way how they are processeed.
When a named paremeter is met in query tree, it is rendered as positional parameter (?1, ?2,...) and its value is put into indexedParameters list, converted into required type if needed:
There was a problem hiding this comment.
Simply saying, when some :param is met in JPQL, you print it in SQL as ?n and put its value n-th in the list of values. So when SQL is executed, parameters' positions exactly match those in initial JPQL.
There was a problem hiding this comment.
Ok @Salauyou . Thanks for comment
| EntityManager em = getFactory().createEntityManager(); | ||
| em.getTransaction().begin(); | ||
|
|
||
| List<Movie> movies = em.createQuery( "from Movie where upper(title) = :title", Movie.class ) |
There was a problem hiding this comment.
You use named parameter, aren't you? Where do you support it?
There was a problem hiding this comment.
Named params are fully supported in JPQL, especially since JPA encourages to prefer named params over positional.
|
Hi @DavideD ! Could you please review the PR? |
|
Thanks @Salauyou Could you rebase this on the current master? There are some conflicts It bothers me a bit that this dialect is using a different approach for parsing the query instead of using the AstParser provided by the hibernate HQL parser project. In theory you should have updated that project so that these changes could be used by the other dialects as well. |
|
@DavideD Thanks for feedback. I endorse your doubts. Regarding HQL parser -- it is still used, partially. Parsing JPQL into tree is made with built-in parser, but rendering into SQL is made by own code because of For me, HQL parser project needs development. But I'm not sure about making proposals because I don't participate in discussion, and many other sides should be involved in such discussion, at least developers of other OGM dialects. Also, I don't know whether other dialects require and can support query functionality that Ignite supports. |
|
Hi @Salauyou, I would like to merge this but because I applied the other PR first now there are some conflicts. Could you rebase to the latest branch and resolve them? Thanks |
|
@DavideD @schernolyas Conflicts resolved, ready to merge. Sorry for late reply. |
|
Hi @DavideD ! Could you please review and merge the PR ? |
|
@Salauyou THanks for solving the conflicts, one thing to keep in mind for the future is that in our projects we always rebase the branch on master and never have a merge commit in the log. If you are not familiar with rebasing, you can find an explanation here: https://git-scm.com/book/en/v2/Git-Branching-Rebasing I will take care of it this time, but keep it in mind if you plan to send future PR, please. |
|
The benefinit of rebasing is that the log looks much nicer and easier to read |
|
@DavideD very useful remark, thanks. I've got it |
|
Rebased and merged #19 |
Instead of using built-in renderer with
QueryRendererDelegateand predicate factory, let's take query tree produced by HQL parser and build Ignite SQL by recursively traversing this tree. This would allow to use in JPQL queries expressions of any complexity, not limited to simple "[not] path-operator-literal/parameter" predicates (see https://hibernate.atlassian.net/browse/OGM-750).In this pull request, is also added support of standard JPQL functions: concat, upper, lower, trim etc.