-
Notifications
You must be signed in to change notification settings - Fork 1.5k
Replace derived CriteriaQuery
with String-based queries
#3653
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
Conversation
f027352
to
b881020
Compare
switch (type) { | ||
case STARTING_WITH: | ||
return String.format("%s%%", escape.escape(value.toString())); | ||
case ENDING_WITH: | ||
return String.format("%%%s", escape.escape(value.toString())); | ||
case CONTAINING: | ||
case NOT_CONTAINING: | ||
return String.format("%%%s%%", escape.escape(value.toString())); | ||
default: | ||
return value; | ||
} |
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.
switch (type) { | |
case STARTING_WITH: | |
return String.format("%s%%", escape.escape(value.toString())); | |
case ENDING_WITH: | |
return String.format("%%%s", escape.escape(value.toString())); | |
case CONTAINING: | |
case NOT_CONTAINING: | |
return String.format("%%%s%%", escape.escape(value.toString())); | |
default: | |
return value; | |
} | |
String escapedValue = escape.escape(value.toString()); | |
return switch (type) { | |
case STARTING_WITH -> escapedValue + "%"; | |
case ENDING_WITH -> "%" + escapedValue; | |
case CONTAINING, NOT_CONTAINING -> "%" + escapedValue + "%"; | |
default -> value; | |
}; |
Consider leveraging the switch
expression for its readability and declaring escapedValue
once to avoid redundancy.
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 left a couple of minor complains and questions
spring-data-jpa/src/main/java/org/springframework/data/jpa/repository/query/JpqlUtils.java
Outdated
Show resolved
Hide resolved
...g-data-jpa/src/main/java/org/springframework/data/jpa/repository/query/ParameterBinding.java
Outdated
Show resolved
Hide resolved
...g-data-jpa/src/main/java/org/springframework/data/jpa/repository/query/PartTreeJpaQuery.java
Outdated
Show resolved
Hide resolved
lock.unlock(); | ||
protected JpqlQueryCreator createCreator(Sort sort, JpaParametersParameterAccessor accessor) { | ||
|
||
synchronized (cache) { |
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.
Wouldn't it be less error prone to make the cache implementation synchronized?
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.
It likely does make sense, also the count query requires a revision of its caching to consider nullable values. Good catch!
...a/src/test/java/org/springframework/data/jpa/repository/query/JpqlQueryBuilderUnitTests.java
Outdated
Show resolved
Hide resolved
...a/src/test/java/org/springframework/data/jpa/repository/query/JpqlQueryBuilderUnitTests.java
Outdated
Show resolved
Hide resolved
...org/springframework/data/jpa/repository/query/ParameterMetadataProviderIntegrationTests.java
Outdated
Show resolved
Hide resolved
Introduce new DSL to construct JPQL queries. Refactor ParameterMetadata to PartTreeParameterBinding. Disable Keyset pagination with projections for Eclipselink as Eclipselink doesn't consider type hints for JPQL queries.
Make usage of ParameterExpression more explicit. Add JPQL rendering tests. Favor Metamodel over From for building jpql queries. Align IsNull and IsNotNull handling. Support Derived Delete and Exists, consider null values when caching queries.
8969f54
to
87b700b
Compare
Remove method overloads accepting pure strings. Use switch-expressions. Correctly navigate nested joins. Introduce PathExpression interface, refine naming.
87b700b
to
801e007
Compare
CriteriaQuery
with String-based queriesCriteriaQuery
with String-based queries
Closes #3588