DATAJPA-1652: Using || (pipes) along with named parameters in custom queries raises an exception

[reta]

• You have read the Spring Data contribution guidelines.
• There is a ticket in the bug tracker for the project in our JIRA.
• You use the code formatters provided here and have them applied to your changes. Don’t submit any formatting related changes.
• You submit test cases (unit or integration tests) that back your changes.
• You added yourself as author in the headers of the classes you touched. Amend the date range in the Apache license header if needed. For new types, add the license header (copy from another file and set the current year only).

Since the Lovelace release train, the usage of '||' along named parameters in custom queries ends up with Caused by: org.hibernate.QueryException: Named parameter [..] not set exception. For example,

@Query(value = "SELECT u FROM User u WHERE u.lastname LIKE '%'||:name||'%'")
Page<User> findByNamedQueryWithLike(@Param("name") String name, Pageable page); 

It caused by the fact that the identifier pattern (declared in QueryUtils) has changed in Lovelace and above, instead of capturing 'name', it captures 'name||'. The suggested fix is to exclude | from the identifier name pattern.

Fixes https://jira.spring.io/browse/DATAJPA-1652, could be also related to spring-projects/spring-framework#22450 and spring-projects/spring-data-commons#340.

Thank you.

[schauder]

This needs some more work before it can be merged, see the separate comments.

Please also make PR against master except when issues only appear in specific branches.

[schauder]

This adds a the | to the list of separators for an identifiers.
But the underlying problem lies deeper: the Unicode punctuation (\p{P})category doesn't contain many of the values in the Posix punctuation category (\{Punct}). For example < and > aren't in that list either.

I quickly tried \{Punct} but breaks some other test.

In any case we need a solution addressing the general problem.

[reta]

Thanks for the feedback @schauder , I have refactored the implementation a bit to address the issue at large, wondering what do you think, all tests are passing. Thank you!

[schauder]

While this fixes the problem at hand while not breaking any tests it just makes it obvious that something is really wrong here: an Identifier shouldn't contain a # and also probably not a $.

It seems IDENTIFIER isn't used for matching identifier, but something like identifier-or-spel
Therefore it

a) should be named as such
b) should be constructed as such, i.e. out of two regex that matches identifiers and spels respectively.
c) all the usages should be verified that they actually should use the more general regex vs the one that actually matches identifiers.

[reta]

So the $ seems to be a valid symbol within identifier name, there is even a dedicated test for it:

@Test // DATAJPA-1200
public void testHasNamedParameter() {
    ...
    checkHasNamedParameter(":$something", true, "dollar");
    ...
}

The # indeed is the SpelEx delimiter, in this context it turned out to be correct but redundant: it is explicitly included within NAMED_PARAMETER pattern. So I removed it from the more strict IDENTIFIER match.

[schauder]

Much better, but I'm still not satisfied.

Also, please add yourself as @author to all the files you change.

[schauder]

While this fixes the problem at hand while not breaking any tests it just makes it obvious that something is really wrong here: an Identifier shouldn't contain a # and also probably not a $.

It seems IDENTIFIER isn't used for matching identifier, but something like identifier-or-spel
Therefore it

a) should be named as such
b) should be constructed as such, i.e. out of two regex that matches identifiers and spels respectively.
c) all the usages should be verified that they actually should use the more general regex vs the one that actually matches identifiers.

[schauder]

Thanks, this is merged and backported.

[reta]

Thanks a lot, @schauder !