Skip to content

Use OFFSET/FETCH syntax in H2 dialect #1297

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
wants to merge 4 commits into from
Closed

Use OFFSET/FETCH syntax in H2 dialect #1297

wants to merge 4 commits into from

Conversation

mp911de
Copy link
Member

@mp911de mp911de commented Jul 21, 2022

As per the suggestion of the H2 maintainer.

Closes #1287

mp911de added 3 commits July 21, 2022 15:44
@mp911de
Prepare issue branch.
048ba4f
@mp911de
Use OFFSET/FETCH syntax in H2 dialect.
f643ced 
As per suggestion of the H2 maintainer.
@mp911de
Polishing.
b110d7b 
Reuse H2 dialect settings in R2DBC-specific H2 dialect.
@mp911de mp911de requested a review from schauder July 21, 2022 13:46
@spring-projects-issues spring-projects-issues added the status: waiting-for-triage An issue we've not yet triaged label Jul 21, 2022
schauder
schauder reviewed Jul 22, 2022
View reviewed changes
Copy link
Contributor

@schauder schauder left a comment

Choose a reason for hiding this comment

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

This raises two questions for me:

Why does the R2DBC H2 dialect extend from the Postgres dialect.
That seems rather counter intuitive and bound to cause problems every time we touch the H2 or the Postgres dialect, including extensions of the Dialect interfaces themselves.

I think if at all we should have maybe some ...DialectSupport class or something that both dialects delegate to.

Since the OP points out that what H2 is supporting the ANSI standard, shouldn't we reuse that, maybe even let H2 dialect extend from that?

@mp911de
Copy link
Member Author

mp911de commented Jul 22, 2022

I had the exact same feeling. I think the array handling is the only component that is different between JDBC and R2DBC so we should require only a R2DBC-specific subclass of the H2 dialect.

@mp911de
Polishing.
7384392 
Refactor ArrayColumns support classes, into toplevel-types. Let R2DBC H2 sublass the relational H2Dialect to decouple from Postgres.
schauder pushed a commit that referenced this pull request Aug 10, 2022
@mp911de @schauder
Use OFFSET/FETCH syntax in H2 dialect.
c2e163b 
As per suggestion of the H2 maintainer.

Closes #1287
Original pull request #1297
schauder pushed a commit that referenced this pull request Aug 10, 2022
@mp911de @schauder
Polishing.
0d1ba21 
Reuse H2 dialect settings in R2DBC-specific H2 dialect.
Refactor ArrayColumns support classes, into toplevel-types. Let R2DBC H2 sublass the relational H2Dialect to decouple from Postgres.

See #1287
Original pull request #1297
@schauder
Copy link
Contributor

That's merged.

@schauder schauder closed this Aug 10, 2022
@schauder schauder deleted the issue/1287 branch August 10, 2022 11:02
schauder pushed a commit that referenced this pull request Mar 30, 2023
@mp911de @schauder
Use OFFSET/FETCH syntax in H2 dialect.
2c865f4 
As per suggestion of the H2 maintainer.

Closes #1287
Original pull request #1297
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@schauder schauder schauder left review comments

Assignees

@schauder schauder

Labels
status: waiting-for-triage An issue we've not yet triaged
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

H2Dialect.LimitClause produces invalid SQL instead of standard OFFSET / FETCH
3 participants
@mp911de @schauder @spring-projects-issues