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

Make H2SequenceMaxValueIncrementer compatible with H2 database 2.0.x #27870

Closed

Conversation

hpoettker
Copy link
Contributor

Spring JDBC is currently not compatible with 2.0.x of H2. This creates problems for users that want to upgrade. See e.g.

I've adjusted H2SequenceMaxValueIncrementer such that it works with both 1.4 and 2.0 and adjusted some test DDL scripts such that they work with both H2 versions and HSQL.

I've also written a test for the incrementer as it is currently seems not to be covered by any test. I was uncertain whether it should be added to DataFieldMaxValueIncrementerTests as that class uses mocks for all data sources instead of embedded databases.

The complete build is successful for the currently used H2 version 1.4.200. With version 2.0.204, the tests in spring-jdbc and spring-test also pass. However, the tests in spring-r2dbc fail with 2.0.204. But that is not related to the changes in the test DDL scripts.

@hpoettker
Copy link
Contributor Author

The build failed with

OrderedMessageSendingIntegrationTests > exceedTimeLimit() FAILED
    java.lang.AssertionError at OrderedMessageSendingIntegrationTests.java:171

I don't think this is related to my changes.

@sbrannen sbrannen added in: data Issues in data modules (jdbc, orm, oxm, tx) status: waiting-for-triage An issue we've not yet triaged or decided on labels Jan 3, 2022
@sbrannen sbrannen changed the title Compatibility of spring-jdbc with 2.0.x of H2 Make H2SequenceMaxValueIncrementer compatible with version 2.0.x of the H2 database Jan 3, 2022
@sbrannen sbrannen changed the title Make H2SequenceMaxValueIncrementer compatible with version 2.0.x of the H2 database Make H2SequenceMaxValueIncrementer compatible with H2 database 2.0.x Jan 3, 2022
@sbrannen sbrannen added type: enhancement A general enhancement and removed status: waiting-for-triage An issue we've not yet triaged or decided on labels Jan 3, 2022
@sbrannen sbrannen modified the milestones: 6.0.0-M2, 5.3.15 Jan 3, 2022
@sbrannen sbrannen self-assigned this Jan 3, 2022
@sbrannen
Copy link
Member

sbrannen commented Jan 6, 2022

Hi @hpoettker,

Thanks for submitting your first PR to the Spring Framework, and thanks for the detailed description! 👍

I've adjusted H2SequenceMaxValueIncrementer such that it works with both 1.4 and 2.0 and adjusted some test DDL scripts such that they work with both H2 versions and HSQL.

Thanks for checking both versions.

I've also written a test for the incrementer as it is currently seems not to be covered by any test. I was uncertain whether it should be added to DataFieldMaxValueIncrementerTests as that class uses mocks for all data sources instead of embedded databases.

It's better to have a dedicated integration test using a real, embedded database for this purpose.


The H2 documentation for Sequence value expression indeed states that we should be using NEXT VALUE FOR <sequence> instead of <sequence>.nextval from dual. So thanks for bringing that to our attention, @katzyn!

As mentioned elsewhere, the syntax <sequence>.nextval from dual can be used with H2 when using the Legacy or Oracle compatibility modes.

@sbrannen
Copy link
Member

sbrannen commented Jan 7, 2022

However, the tests in spring-r2dbc fail with 2.0.204. But that is not related to the changes in the test DDL scripts.

This will be addressed in the following PR for r2dbc-h2:

So, for the time being we cannot fully upgrade to H2 2.x in the build.

sbrannen added a commit that referenced this pull request Jan 7, 2022
sbrannen added a commit to sbrannen/spring-framework that referenced this pull request Jan 7, 2022
@sbrannen sbrannen closed this in ed4e228 Jan 7, 2022
@sbrannen
Copy link
Member

sbrannen commented Jan 7, 2022

This has been merged into 5.3.x and main in ed4e228 and further refined in 18781e5 and 75e1811.

Thanks

@sbrannen
Copy link
Member

sbrannen commented Jan 7, 2022

sbrannen added a commit to sbrannen/spring-framework that referenced this pull request Jan 7, 2022
Commit ed4e228 introduced support for H2 2.0.x but did not upgrade
the H2 dependency.

This commit upgrades the H2 dependency to version 2.0.206 but also
adds an explicit test dependency on version 1.4.200 in spring-r2dbc,
since r2dbc-h2 does not yet support H2 2.0.x.

Once r2dbc/r2dbc-h2#204 has been included in a
released version of r2dbc-h2 we will be able to upgrade spring-r2dbc's
test dependency on the H2 database to 2.0.x as well.

See spring-projectsgh-27870
See spring-projectsgh-27902
@hpoettker hpoettker deleted the h2-v2-compatible-jdbc branch January 7, 2022 18:34
sbrannen added a commit that referenced this pull request Jan 8, 2022
@hpoettker made me aware of the ModeEnum in H2 that allows the
parameterized test to be simplified, which was the primary impetus for
this commit.

See gh-27870
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
in: data Issues in data modules (jdbc, orm, oxm, tx) type: enhancement A general enhancement
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants