Skip to content
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.

Sign up for GitHub

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

Jump to bottom

Support for SpEL expressions containing table names in annotated queries #1863

Closed
wants to merge 4 commits into from
Closed

Support for SpEL expressions containing table names in annotated queries #1863

wants to merge 4 commits into from

Conversation

schauder
Copy link
Contributor

@schauder schauder commented Aug 22, 2024 •
edited
Loading

Closes #1856

schauder added a commit that referenced this pull request Aug 28, 2024
@schauder
Support for table names in SpEL expressions.
33786ab 
SpEL expressions in queries get processed in two steps:

1. First SpEL expressions outside parameters are detected and processed.
This is done with a `StandardEvaluationContext` with the variables `tableName` and `qualifiedTableName` added.
This step is introduced by this commit.

2. Parameters made up by SpEL expressions are processed as usual.

Closes #1856
Originial pull request #1863
@schauder schauder force-pushed the issue/1856-tablename-spel branch from d0127fe to 33786ab Compare August 28, 2024 08:48
schauder added a commit that referenced this pull request Aug 28, 2024
@schauder
Polishing.
fd6d4e6 
Fixing a test used for performance reasons.

Formatting a test.
Removing public modifier.
Separating test methods from infrastructure.

Original pull request #1863
See #1856
schauder added a commit that referenced this pull request Aug 28, 2024
@schauder
Support for table names in SpEL expressions.
21a5f74 
SpEL expressions in queries get processed in two steps:

1. First SpEL expressions outside parameters are detected and processed.
This is done with a `StandardEvaluationContext` with the variables `tableName` and `qualifiedTableName` added.
This step is introduced by this commit.

2. Parameters made up by SpEL expressions are processed as usual.

Closes #1856
Originial pull request #1863
@schauder schauder force-pushed the issue/1856-tablename-spel branch from 33786ab to 21a5f74 Compare August 28, 2024 14:16
@schauder schauder marked this pull request as ready for review August 28, 2024 14:20
mp911de
mp911de requested changes Aug 30, 2024
View reviewed changes
...-jdbc/src/main/java/org/springframework/data/jdbc/repository/query/StringBasedJdbcQuery.java
* @param queryPreprocessor must not be {@literal null}.
* @since 3.4
*/
public StringBasedJdbcQuery(JdbcQueryMethod queryMethod, NamedParameterJdbcOperations operations,
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

With already the constructor being changed, I wonder what difference it makes handing in the processed query vs. a QueryPreprocessor that we're going to remove from the signature anyways.

...in/java/org/springframework/data/relational/repository/query/TableNameQueryPreprocessor.java Outdated
private final SqlIdentifier qualifiedTableName;
private final Dialect dialect;

public TableNameQueryPreprocessor(SqlIdentifier tableName, SqlIdentifier qualifiedTableName, Dialect dialect) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Introducing concepts of QueryPreprocessing exposes this utility quite prominently. I wonder whether we can avoid repeating table name extraction across JDBC and R2DBC modules and I wonder whether we could have a RelationalQueryLookupStrategy. Looking at JdbcQueryLookupStrategy and R2dbcQueryLookupStrategy, these seem to have sufficient context already for that purpose.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done.

I kept the TableNameQueryPreprocessor since putting tons of String manipulation into RelationalQueryLookupStrategy directly feels wrong.

@schauder
1856-tablename-spel - Prepare branch
2c4c4c9
@schauder
Polishing.
22b8795 
Fixing a test used for performance reasons.

Formatting a test.
Removing public modifier.
Separating test methods from infrastructure.

Original pull request #1863
See #1856
@schauder
Support for table names in SpEL expressions.
f87866c 
SpEL expressions in queries get processed in two steps:

1. First SpEL expressions outside parameters are detected and processed.
This is done with a `StandardEvaluationContext` with the variables `tableName` and `qualifiedTableName` added.
This step is introduced by this commit.

2. Parameters made up by SpEL expressions are processed as usual.

Closes #1856
Originial pull request #1863
@schauder
Move query extraction into QueryLookupStrategy.
c563c2c 
JdbcStringBasedQuery no longer extracts the query from the `QueryMethod`.

The logic of applying table name based SpEL expressions moves into the new base class `RelationalQueryLookupStrategy`.

See #1856
Originial pull request #1863
@schauder schauder force-pushed the issue/1856-tablename-spel branch from 21a5f74 to c563c2c Compare September 3, 2024 14:56
mp911de pushed a commit that referenced this pull request Sep 4, 2024
@schauder @mp911de
Polishing.
4221840 
Fixing a test used for performance reasons.

Formatting a test.
Removing public modifier.
Separating test methods from infrastructure.

Original pull request #1863
See #1856
mp911de pushed a commit that referenced this pull request Sep 4, 2024
@schauder @mp911de
Support for table names in SpEL expressions.
f937738 
SpEL expressions in queries get processed in two steps:

1. First SpEL expressions outside parameters are detected and processed.
This is done with a `StandardEvaluationContext` with the variables `tableName` and `qualifiedTableName` added.
This step is introduced by this commit.

2. Parameters made up by SpEL expressions are processed as usual.

Closes #1856
Original pull request #1863
mp911de added a commit that referenced this pull request Sep 4, 2024
@mp911de
Polishing.
7277413 
Remove Preprocessor interface. Add property accessors to RelationalQueryLookupStrategy. Reuse property accessors instead of loosely coupled object access.

See #1856
Original pull request #1863
@mp911de mp911de added the type: enhancement A general enhancement label Sep 4, 2024
@mp911de mp911de self-assigned this Sep 4, 2024
@mp911de mp911de added this to the 3.4 M1 (2024.1.0) milestone Sep 4, 2024
@mp911de mp911de closed this Sep 4, 2024
@mp911de mp911de deleted the issue/1856-tablename-spel branch September 4, 2024 08:08
@mp911de
Copy link
Member

mp911de commented Sep 4, 2024

That's merged and polished now.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@mp911de mp911de mp911de requested changes

Assignees

@mp911de mp911de

Labels
type: enhancement A general enhancement
Projects
None yet
Milestone
3.4 M1 (2024.1.0)
Development

Successfully merging this pull request may close these issues.

Add support for table name placeholder for @Query in JDBC and R2DBC
2 participants
@schauder @mp911de