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.

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

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

Closed
wants to merge 4 commits into from

Conversation

schauder
Copy link
Contributor

@schauder schauder commented Aug 22, 2024

Closes #1856

schauder added a commit that referenced this pull request Aug 28, 2024
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 added a commit that referenced this pull request Aug 28, 2024
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
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 marked this pull request as ready for review August 28, 2024 14:20
* @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.

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.

Fixing a test used for performance reasons.

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

Original pull request #1863
See #1856
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
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
mp911de pushed a commit that referenced this pull request Sep 4, 2024
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
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
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
Labels
type: enhancement A general enhancement
Projects
None yet
Development

Successfully merging this pull request may close these issues.

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