Let `Result.getRowsUpdated()` return `Long`
elefeint opened this issue · 3 comments
Currently, Result.getRowsUpdated() returns a Publisher<Integer>.
Cloud Spanner naturally returns Long (internally known as INT64) values, and the Cloud Spanner R2DBC driver works around the mismatch by converting too-large numbers with a warning and loss of original value.
Ideally, we would be able to pass the original value through to the end user. This would require the result specification to change from Publisher<Integer> getRowsUpdated() to Publisher<Long> getRowsUpdated().
This may also help future-proof the spec as datasets grow ever-larger.
This is also an issue for H2 because they now return a Long. I am force fitting it into an Integer to comply.
It's also an issue for Oracle R2DBC. We throw an ArithmeticException if an update count exceeds Integer.MAX_VALUE.
Publishing Long, rather than Integer, seems like the right thing to do. This way, it will be consistent with Result.UpdateCount.value():
The only reason I can think of not to do this would be a significant performance benefit from using Integer instead of Long. If that can be demonstrated, then there might be case for SPI methods that expose int-valued updated counts.
I'm wondering what's the best way to go about it without creating too much trouble. The SPI could specify Publisher<? extends Number> for the time being, even for 0.9 without breaking existing implementations and for 1.0 we could migrate to Long. That could give implementations some time to adopt.
Switching with 1.0 directly from Integer to Number would render implementations incompatible. What other possibilities do we have?
Likely, regardless of what we do, either drivers or clients will break.