-
Notifications
You must be signed in to change notification settings - Fork 1.4k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Properly handle native queries with SpEL #3109
Conversation
Native queries with SpEL expressions should be properly parsed instead of converted to non-native queries. See #3096
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Generally, this PR isn't fitting to our existing code. AbstractStringBasedJpaQuery
already creates a ExpressionBasedStringQuery
that we IMO should render properly to avoid downstream issues.
Adding more context to a DeclaredQuery
factory adds more dependencies that aren't within the actual scope of the query wrapper.
We should rather eagerly render the query so that downstream components work with the already resolved expressions.
* @param method is a {@link JpaQueryMethod} that has the related metadata | ||
* @return a {@literal DeclaredQuery} instance even for a {@literal null} or empty argument. | ||
*/ | ||
static DeclaredQuery of(@Nullable String query, Boolean nativeQuery, @Nullable JpaQueryMethod method) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This method signature introduces a cycle as JpaQueryMethod
uses DeclaredQuery
already. Why don't we resort to EntityInformation
as parameter?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I have tweaked that method signature to instead pass in JpaEntityMetadata
.
The following repository definition:
...wil fail.
We have code that will explicitly switch it from native to non-native, and hence force it into one of the query parsers, which will cause it to fail.
However, it's not as simple as simply retaining the
nativeQuery
value. It also causes theQueryEnhancerFactory
to kick in before parsing the SpEL expression inside. Tools like JSqlParser will fail. Thus, we need to detect it's SpEL nature sooner and properly mark it, so it's handled correctly.This PR alters the visibility of a couple private items, but only makes them package-private.