Skip to content

Allow for the extension of the JdbcQueryLookupStrategy #1998

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
Closed
mipo256 opened this issue Feb 21, 2025 · 4 comments
Closed

Allow for the extension of the JdbcQueryLookupStrategy #1998

mipo256 opened this issue Feb 21, 2025 · 4 comments
Assignees
@schauder
Labels
type: enhancement A general enhancement
Milestone
4.0 M3 (2025.1.0)

Comments

@mipo256
Copy link
Contributor

mipo256 commented Feb 21, 2025 •
edited
Loading

We're writing our own dialect for the Spring Data JDBC aiming to support YDB. And we hit the problem: ydb-platform/ydb-java-dialects#173

The summary is that we seems to need to extend the QueryLookupStrategy. That is not a big deal, since it is public, however, we want to retian the logic of the RowMapper selection mechanism, as well as resolution of the StringBased/PartTree queries. This logic is outlined in the JdbcQueryLookupStrategy, which is apparently not public.

Proposal: Since the QueryLookupStrategy is constructed in JdbcRepositoryFactory, therefore it actually is intended to be overloaded, can we make the JdbcQueryLookupStrategy and its subclasses in JDBC module public as well?

I have the PR ready for this, waiting for the approval. CC: @schauder @mp911de
@mipo256 mipo256 mentioned this issue Feb 21, 2025
Open
@spring-projects-issues spring-projects-issues added the status: waiting-for-triage An issue we've not yet triaged label Feb 21, 2025
@schauder
Copy link
Contributor

I wonder if we could extract the RowMapper selection mechanism into a top level public class. Would that work for you?

@schauder schauder added the status: waiting-for-feedback We need additional information before we can continue label Feb 21, 2025
@mipo256
Copy link
Contributor Author

mipo256 commented Feb 21, 2025

That is great, but that is not enough.

We really need the logic encapsulated in the DeclaredQueryLookupStrategy and CreateQueryLookupStrategy. If we made them public as well, that would be great and that would work. May I provide the PR? I have the code ready.

@spring-projects-issues spring-projects-issues added status: feedback-provided Feedback has been provided and removed status: waiting-for-feedback We need additional information before we can continue labels Feb 21, 2025
@schauder
Copy link
Contributor

Sure, there is no harm in having a PR

@mipo256 mipo256 mentioned this issue Feb 21, 2025
Closed
@schauder schauder added type: enhancement A general enhancement and removed status: waiting-for-triage An issue we've not yet triaged status: feedback-provided Feedback has been provided labels Feb 24, 2025
schauder pushed a commit that referenced this issue May 13, 2025
@mipo256 @schauder
Polishing.
98ce382 
Uses new switch syntax.

Signed-off-by: mipo256 <[email protected]>

Commit message changed by Jens Schauder

Original pull request #2000
See #1998
schauder pushed a commit that referenced this issue May 13, 2025
@mipo256 @schauder
Extract RowMapper into top level classes.
6dab06a 
Signed-off-by: mipo256 <[email protected]>

Commit message changed by Jens Schauder

Original pull request #2000
Closes #1998
schauder added a commit that referenced this issue May 13, 2025
@schauder
Polishing.
ed4edba 
Formatting.

Original pull request #2000
See #1998
@schauder schauder added this to the 4.0 M3 (2025.1.0) milestone May 13, 2025
@schauder
Copy link
Contributor

Resolved by 6dab06a

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

@schauder schauder

Labels
type: enhancement A general enhancement
Projects
None yet
Milestone
4.0 M3 (2025.1.0)
Development

Successfully merging a pull request may close this issue.

Unified RowMapper infrastructure mipo256/spring-data-relational
3 participants
@schauder @spring-projects-issues @mipo256