Rewrite LIKE with wildcards using CONCAT function. #2944
Conversation
We support wrapping parameters (named or positional) with optional wildcards when doing LIKE patterns. This is out-of-band and requires moving the wildcards into the bindings. To stop doing this and causing race conditions, we can instead rewrite the queries using the CONCAT function. This function is standard across relational database (native queries) as well as JPA providers (Hibernate and EclipseLink). Resolves #2939. Related: #2760. Original Pull Request: #2944.
gregturn
force-pushed
the
issue/gh-2939-take2
branch
from
964b3f8 to
19832e0
Compare
We support wrapping parameters (named or positional) with optional wildcards when doing LIKE patterns. This is out-of-band and requires moving the wildcards into the bindings. To stop doing this and causing race conditions, we can instead rewrite the queries using the CONCAT function. This function is standard across relational database (native queries) as well as JPA providers (Hibernate and EclipseLink). Resolves #2939. Related: #2760. Original Pull Request: #2940. Superceding Pull Request: #2944
gregturn
force-pushed
the
issue/gh-2939-take2
branch
from
19832e0 to
086a109
Compare
There was a problem hiding this comment.
My understanding of our style guide for commit messages is for the references to issues:
- Don't end with a dot.
- Don't include a colon.
- refer to related issues with
See
Other than these minor issues this seems fine to me.
Another long standing issue finally fixed. 💛
|
|
||
| concatWrapper.append(")"); | ||
|
|
||
| LOGGER.warn( |
There was a problem hiding this comment.
I'm against logging this. It is a normal feature of SD JPA and we don't have a reason to believe it does not work.
If there are actually databases that don't support concat and need a workaround we can add a warning like this when we learn about them (and don't find a proper solution)
There was a problem hiding this comment.
We should consider this a valid feature and do not log a warning. Our docs explicitly mention:
and u.lastname like %:lastname%")
| @@ -102,6 +102,40 @@ void customQueryWithNullMatch() { | |||
| assertThat(Employees).extracting(EmployeeWithName::getName).isEmpty(); | |||
| } | |||
|
|
|||
| @Test | |||
There was a problem hiding this comment.
Tests should have a comment referencing the issue
// GH-2939 in this case.
Guess everyone is doing it differently, though. Everything is fine as long as we start ticket references with |
|
I guess that was a habit from Spring HATEOAS/Spring Data REST regarding formatting. I really like the |
We support wrapping parameters (named or positional) with optional wildcards when doing LIKE patterns. This is out-of-band and requires moving the wildcards into the bindings. To stop doing this and causing race conditions, we can instead rewrite the queries using the CONCAT function. This function is standard across relational database (native queries) as well as JPA providers (Hibernate and EclipseLink). See #2939 See #2760 Original Pull Request: #2940 Superceding Pull Request: #2944
We support wrapping parameters (named or positional) with optional wildcards when doing LIKE patterns. This is out-of-band and requires moving the wildcards into the bindings. To stop doing this and causing race conditions, we can instead rewrite the queries using the CONCAT function. This function is standard across relational database (native queries) as well as JPA providers (Hibernate and EclipseLink). See #2939 See #2760 Original Pull Request: #2940 Superceding Pull Request: #2944
We support wrapping parameters (named or positional) with optional wildcards when doing LIKE patterns. This is out-of-band and requires moving the wildcards into the bindings. To stop doing this and causing race conditions, we can instead rewrite the queries using the CONCAT function. This function is standard across relational database (native queries) as well as JPA providers (Hibernate and EclipseLink). See #2939 See #2760 Original Pull Request: #2940 Superceding Pull Request: #2944
Since queries like
select e from Employee e where e.name like %:name%as well asselect * from Employee AS e where e.name like %:name%both result in the wildcards being pulled out and migrated to the bindings, I have tuned that action to instead replace them with aCONCATfunction.CONCATappears to be supported on all major relational databases as well as on all JPA providers.This means we can eliminate the race condition that arises when you use the same binding more than once, and only one of them has a wildcard.
And because this doesn't impact any public-facing APIs nor does it involve the query parser, I believe this PR is a candidate for backporting to
3.0.xas well as2.7.x.I took the liberty of introducing a warning in the log files that can alert users to this, tipping them off that it may be to their advantage to rewrite their custom query using the same pattern, in case this feature is no longer supported in the future. If you think this is unneeded, we can remove it. However, I think it's fair to nudge people to in fact rewrite their queries and make them more standard.