Upgrade to support H2 2.0. #204
Conversation
[resolves #203] Signed-off-by: Greg L. Turnquist <[email protected]>
gregturn
force-pushed
the
issue/h2-2.0
branch
from
0860b24
to
dd279a6
Compare
| @@ -146,9 +149,11 @@ private H2Statement addIndex(int index, Object value) { | |||
| ResultWithGeneratedKeys result = client.update(command, generatedColumns); | |||
| CommandUtil.clearForReuse(command); | |||
| if (GeneratedKeysMode.valueOf(generatedColumns) == GeneratedKeysMode.NONE) { | |||
| return H2Result.toResult(codecs, result.getUpdateCount()); | |||
| int updatedCountInt = Long.valueOf(result.getUpdateCount()).intValue(); | |||
There was a problem hiding this comment.
(int) result.getUpdateCount()does exactly the same, but without allocation ofjava.lang.Long.- H2 actually can return values larger than
Integer.MAX_VALUEfrom some operations, for example, fromTRUNCATE TABLE, so it would be better to handle it in some better way.
There was a problem hiding this comment.
Thanks @katzyn. I have applied (1), however (2) is governed by our current SPI requiring a Mono<Integer>. This topic is being tracked via r2dbc/r2dbc-spi#255.
| @@ -47,6 +47,6 @@ boolean doCanDecode(int dataType) { | |||
| Value doEncode(Object[] value) { | |||
| return ValueArray.get(Arrays.stream(Assert.requireNonNull(value, "value must not be null")) | |||
| .map(codecs::encode) | |||
| .toArray(Value[]::new)); | |||
| .toArray(Value[]::new), null); | |||
There was a problem hiding this comment.
H2 2.0 has only standard-compliant typed arrays.
CastDataProvider shouldn't be null here. If objects of different types are passed within the same array, they need to be converted to the same higher type and some conversions need to know properties of the session.
There was a problem hiding this comment.
I've opened #208 to track this.
| @@ -38,6 +38,6 @@ Byte doDecode(Value value, Class<? extends Byte> type) { | |||
|
|
|||
| @Override | |||
| Value doEncode(Byte value) { | |||
| return ValueByte.get(Assert.requireNonNull(value, "value must not be null")); | |||
| return ValueBinary.get(new byte[]{Assert.requireNonNull(value, "value must not be null")}); | |||
| return Value.STRING == dataType || | ||
| Value.STRING_FIXED == dataType || | ||
| Value.STRING_IGNORECASE == dataType; | ||
| return Value.VARCHAR == dataType || Value.VARCHAR_IGNORECASE == dataType; |
| @@ -30,7 +30,7 @@ | |||
|
|
|||
| @Override | |||
| boolean doCanDecode(int dataType) { | |||
| return dataType == Value.DECIMAL; | |||
| return dataType == Value.NUMERIC; | |||
There was a problem hiding this comment.
|| dataType == Value.DECFLOAT may be needed too. But DECFLOAT has three special values (NaN, Inf, and -Inf) not compatible with BigDecimal, so situation is complicated.
There was a problem hiding this comment.
I opened #209 to track this.
| @@ -34,6 +34,7 @@ | |||
| <properties> | |||
| <assertj.version>3.20.2</assertj.version> | |||
| <h2.version>1.4.200</h2.version> | |||
| <h2.version>2.0.202</h2.version> | |||
| return ValueBytes.get(Assert.requireNonNull(value, "value must not be null")); | ||
| return ValueBinary.get(Assert.requireNonNull(value, "value must not be null")); |
There was a problem hiding this comment.
When a binary string literal is specified, its data type BINARY or BINARY VARYING is implementation-defined in the SQL Standard. H2 uses BINARY VARYING and usage of BINARY may cause various unexpected effects.
So here ValueVarbinary.get() should be used for consistency.
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
[resolves #203]