Some feedback on R2DBC

[Lukas Eder]

Hey folks,

Congrats to your M5 release. I've finally had some time to review the SPI. I must say, I find it *much* easier to understand and use than ADBA, despite having never worked with Flux and Mono before, so this Spring approach is all essentially news to me.

Some feedback from my side:

Generated Keys

I've already commented on the relevant issue here:

https://github.com/r2dbc/r2dbc-spi/issues/17

While I understand that this is a very messy area in JDBC, I do strongly suggest you do not drop this SPI method. While clients (like jOOQ) can work around native driver support for fetching generated keys / default values / trigger generated values, etc., the best approach is still to have actual native driver support to avoid the additional server roundtrip, if the driver / RDBMS support DML and fetching in a single roundtrip. Many do.

Of course, such an SPI can be postponed to a future release too, if you're currently unsure about the best way forward here. But I can assure you, when this feature doesn't work in jOOQ (as linked from above issue 17), I get tons of complaints from users. This seems to be very essential to everyone doing CRUD, and by extension, probably doing REST as well. Being able to produce an ID after an INSERT

Reactor and Reactive Streams dependency

Given that R2DBC is a strategic API in the Spring ecosystem, I obviously understand the dependency on reactor and reactive streams, given that this is deployed everywhere in Spring. But I do wonder if in the particular case of R2DBC, it would be possible to work with the JDK Flow API only. This would make it much easier for non-Spring tools (oh, say, jOOQ) to adopt R2DBC in the future. What's your take here?

ColumnMetadata

This SPI feels largely incomplete. Is there any further work planned for release 1.0.0?

Also, I find io.r2dbc.spi.RowMetadata#getColumnMetadatas()'s return type Iterable a bit unusual. Is this common practice in Spring? A List<ColumnMetadata> seems more appropriate / convenient in this case.

Savepoints

The JDBC API supports "unnamed" savepoints, which is very convenient for clients. Has retaining that functionality been discussed? Also, before the API is set in stone, I would have expected Statement.createSavepoint(...) to return a Publisher<Savepoint> type, and the release and rollback operations would also reference the Savepoint type, not the String, given that the name could be optional.

Exceptions and warnings (most important criticism last)

I've discussed this on the jdbc-spec-discuss list as well in this lengthy thread:

http://mail.openjdk.java.net/pipermail/jdbc-spec-discuss/2017-October/000098.html

Some databases (e.g. MySQL and SQL Server, since 12c also Oracle) allow for statement batches to produce multiple consecutive results. A result can be any of:

- An update count

- A result set

- An exception

This is a bit of an obscure JDBC feature, but some people do depend on this, especially if they make heavy use of SQL Server stored procedures. jOOQ supports this as well, and I think this should not be overlooked in R2DBC. Streaming such consecutive results through Flux seems quite elegant in fact. I currently do not see how such a stream of events can be retrieved from io.r2dbc.spi.Result, or from io.r2dbc.spi.Statement#execute().

In particular, exceptions can be seen as ordinary results, e.g. in SQL Server, as a signal rather than something that should abort a transaction. Take this snippet for example (from the linked ADBA discussion):

DECLARE @t TABLE(i INT);
INSERT INTO @t VALUES (1),(2),(3);
RAISERROR('message 1', 16, 2, 3);
RAISERROR('message 2', 16, 2, 3);
SELECT * FROM @t
RAISERROR('message 3', 16, 2, 3);

It should produce a result like this:

Update Count: 3
Exception   : message 1
Exception   : message 2
Result      :
  i: 1
  i: 2
  i: 3
Exception   : message 3

So, on an SPI level, there should probably be a Flux<Result> type returned from execute(), or an alternative execute() method that allows for this execution mode, where exceptions can be treated as signals, not as exceptions, and where interleaved update counts and result sets are possible.

I hope this helps,

Lukas

[Mark Paluch]

Hi Lukas and welcome to the R2DBC community.

Thanks a lot for your feedback. I added some responses/comments (inline). Please note that R2DBC is not a Spring project. It is just built by people that typically contribute to Spring projects.

Cheers, 

Mark

Am 23. Nov. 2018, 11:14 +0100 schrieb Lukas Eder <lukas...@datageekery.com>:

Hey folks,

Congrats to your M5 release. I've finally had some time to review the SPI. I must say, I find it *much* easier to understand and use than ADBA, despite having never worked with Flux and Mono before, so this Spring approach is all essentially news to me.

Some feedback from my side:

Generated Keys

I've already commented on the relevant issue here:

https://github.com/r2dbc/r2dbc-spi/issues/17

While I understand that this is a very messy area in JDBC, I do strongly suggest you do not drop this SPI method. While clients (like jOOQ) can work around native driver support for fetching generated keys / default values / trigger generated values, etc., the best approach is still to have actual native driver support to avoid the additional server roundtrip, if the driver / RDBMS support DML and fetching in a single roundtrip. Many do.

Of course, such an SPI can be postponed to a future release too, if you're currently unsure about the best way forward here. But I can assure you, when this feature doesn't work in jOOQ (as linked from above issue 17), I get tons of complaints from users. This seems to be very essential to everyone doing CRUD, and by extension, probably doing REST as well. Being able to produce an ID after an INSERT 

I see your point. Given the initial support we’ve seen from the currently supported databases (Postgres, H2, Microsoft SQL Server) we did not see a point in abstracting away SQL. For these databases, it’s just a matter of altering the query to return generated keys. We’d need to get examples from databases that implement a different scheme. For now, we can get away with moving dialect-specifics into client libraries.

Reactor and Reactive Streams dependency

Given that R2DBC is a strategic API in the Spring ecosystem, I obviously understand the dependency on reactor and reactive streams, given that this is deployed everywhere in Spring. But I do wonder if in the particular case of R2DBC, it would be possible to work with the JDK Flow API only. This would make it much easier for non-Spring tools (oh, say, jOOQ) to adopt R2DBC in the future. What's your take here? 

R2DBC only depends on Reactive Streams. We intentionally stay on Reactive Streams and not the Java Flow API to simplify integration and not raise barriers to entry by staying on Java 8. There are currently no reactive libraries (these are required to build client drivers) that implement JDK’s Flow API.

With the whole ecosystem moving forward, it makes sense to migrate to JDK’s Flow API at a point we have sufficient Java 9+ adoption and libraries that implement the Flow API.

You can use R2DBC with RxJava2, Ratpack, Akka Streams and other libraries that integrate with Reactive Streams without requiring to use Project Reactor.

Dependencies on Project Reactor arise from an driver-implementation and R2DBC Client perspective. Drivers require a reactive composition library and hence you see Project Reactor being pulled in. Drivers can use what ever library/own implementation they need to expose a specific driver functionality. We built R2DBC Client, that is an example how a humane Client could look like using Project Reactor as that’s the tool we’re used to.

ColumnMetadata

This SPI feels largely incomplete. Is there any further work planned for release 1.0.0?

Also, I find io.r2dbc.spi.RowMetadata#getColumnMetadatas()'s return type Iterable a bit unusual. Is this common practice in Spring? A List<ColumnMetadata> seems more appropriate / convenient in this case. 

This is also our feeling. Happy to start a discussion around metadata to get an impression what’s really needed here. Can you file a ticket in our SPI project along some things you’d like to see there?

Savepoints

The JDBC API supports "unnamed" savepoints, which is very convenient for clients. Has retaining that functionality been discussed? Also, before the API is set in stone, I would have expected Statement.createSavepoint(...) to return a Publisher<Savepoint> type, and the release and rollback operations would also reference the Savepoint type, not the String, given that the name could be optional. 

R2DBC requires for sure a few API iterations until we’re happy to release a 1.0 GA. Can you file a ticket for this improvement? That way we could potentially move save-point-based release operations into a Savepoint type and also let createSavepoint(String) return Publisher<Savepoint>.

execute() can emit multiple Result objects which encapsulate individual streams for the returned results. A Result can terminate exceptionally without affecting the other Result’s. Typically, you’d flatMap(…) multiple streams into a single one. That’s a good scenario to be tested at least in each driver project.

I hope this helps,

Lukas

--
You received this message because you are subscribed to the Google Groups "r2dbc" group.
To unsubscribe from this group and stop receiving emails from it, send an email to r2dbc+un...@googlegroups.com.
To post to this group, send email to r2...@googlegroups.com.
To view this discussion on the web visit https://groups.google.com/d/msgid/r2dbc/88a20853-b3c2-4134-ae44-44a46c01713f%40googlegroups.com.
For more options, visit https://groups.google.com/d/optout.

[Lukas Eder]

Hi Mark,

Thanks a lot for the quick reply.

Comments inline...

On Friday, November 23, 2018 at 1:09:33 PM UTC+1, Mark Paluch wrote:

Generated Keys

I've already commented on the relevant issue here:

https://github.com/r2dbc/r2dbc-spi/issues/17

While I understand that this is a very messy area in JDBC, I do strongly suggest you do not drop this SPI method. While clients (like jOOQ) can work around native driver support for fetching generated keys / default values / trigger generated values, etc., the best approach is still to have actual native driver support to avoid the additional server roundtrip, if the driver / RDBMS support DML and fetching in a single roundtrip. Many do.

Of course, such an SPI can be postponed to a future release too, if you're currently unsure about the best way forward here. But I can assure you, when this feature doesn't work in jOOQ (as linked from above issue 17), I get tons of complaints from users. This seems to be very essential to everyone doing CRUD, and by extension, probably doing REST as well. Being able to produce an ID after an INSERT 

I see your point. Given the initial support we’ve seen from the currently supported databases (Postgres, H2, Microsoft SQL Server) we did not see a point in abstracting away SQL. For these databases, it’s just a matter of altering the query to return generated keys. We’d need to get examples from databases that implement a different scheme. For now, we can get away with moving dialect-specifics into client libraries.

Indeed, PostgreSQL and SQL Server have purely SQL based alternatives. But H2 does not. Other databases that have SQL based approaches include:

- DB2 (only database that implements the SQL standard data change table)

- Firebird (mimicks PostgreSQL's syntax)

- Oracle (with some trickery involving PL/SQL, not very convenient - should be provided by the driver instead)

Note also that SQL Server's OUTPUT clause has its own caveats, so using a second query fetching the identity variable may be inevitable. However, a second query in a statement batch raises new questions, as per my last criticism about statement batches and exceptions / warnings.

 

Reactor and Reactive Streams dependency

Given that R2DBC is a strategic API in the Spring ecosystem, I obviously understand the dependency on reactor and reactive streams, given that this is deployed everywhere in Spring. But I do wonder if in the particular case of R2DBC, it would be possible to work with the JDK Flow API only. This would make it much easier for non-Spring tools (oh, say, jOOQ) to adopt R2DBC in the future. What's your take here? 

R2DBC only depends on Reactive Streams. We intentionally stay on Reactive Streams and not the Java Flow API to simplify integration and not raise barriers to entry by staying on Java 8. There are currently no reactive libraries (these are required to build client drivers) that implement JDK’s Flow API.

With the whole ecosystem moving forward, it makes sense to migrate to JDK’s Flow API at a point we have sufficient Java 9+ adoption and libraries that implement the Flow API.

You can use R2DBC with RxJava2, Ratpack, Akka Streams and other libraries that integrate with Reactive Streams without requiring to use Project Reactor.

Dependencies on Project Reactor arise from an driver-implementation and R2DBC Client perspective. Drivers require a reactive composition library and hence you see Project Reactor being pulled in. Drivers can use what ever library/own implementation they need to expose a specific driver functionality. We built R2DBC Client, that is an example how a humane Client could look like using Project Reactor as that’s the tool we’re used to.

I did not realise that R2DBC client vs SPI distinction. Thanks for the clarification. I guess depending on reactive streams is sufficient for now - I see your points.

 

ColumnMetadata

This SPI feels largely incomplete. Is there any further work planned for release 1.0.0?

Also, I find io.r2dbc.spi.RowMetadata#getColumnMetadatas()'s return type Iterable a bit unusual. Is this common practice in Spring? A List<ColumnMetadata> seems more appropriate / convenient in this case. 

This is also our feeling. Happy to start a discussion around metadata to get an impression what’s really needed here. Can you file a ticket in our SPI project along some things you’d like to see there?

Sure, here you go: https://github.com/r2dbc/r2dbc-spi/issues/22

 

Savepoints

The JDBC API supports "unnamed" savepoints, which is very convenient for clients. Has retaining that functionality been discussed? Also, before the API is set in stone, I would have expected Statement.createSavepoint(...) to return a Publisher<Savepoint> type, and the release and rollback operations would also reference the Savepoint type, not the String, given that the name could be optional. 

R2DBC requires for sure a few API iterations until we’re happy to release a 1.0 GA. Can you file a ticket for this improvement? That way we could potentially move save-point-based release operations into a Savepoint type and also let createSavepoint(String) return Publisher<Savepoint>.

Sure thing: https://github.com/r2dbc/r2dbc-spi/issues/23

Right, of course. That's where my understanding of Flux / Publisher failed me, and you're correct about the SPI. I was perhaps misguided by looking at the client library, which declares:

    public Flux<Integer> execute() {

        return Flux

            .from(this.statement.execute())

            .flatMap(Result::getRowsUpdated);

    }

So, it is the intent of the client library to offer only minimal functionality, and for an edge case like this, people should resort to using the SPI directly?

Thanks,

Lukas

[Greg Turnquist]

Yes.

In essence, you COULD write your app using the SPI, but it would probably be very...bulky.

The idea is to bundle up common paradigms into a humane client. The r2dbc-client library is just one take at doing things, inspired by JDBI. In other words, if you were to take JDBI and rewrite it from scratch, hooking into the SPI, you could implement something very analogous and easy for the JDBI community to pick regarding flows. If people are keenly interested, expand r2dbc-client to further implement parts of JDBI not yet covered. (I know nothing about JDBI, so I don't know what's missing.)

I could imagine doing the same for jOOQ. Throw out JDBC and using R2DBC and Reactive Streams, how far can you go with building an alternative client?

 

Thanks,

Lukas

--
You received this message because you are subscribed to the Google Groups "r2dbc" group.
To unsubscribe from this group and stop receiving emails from it, send an email to r2dbc+un...@googlegroups.com.
To post to this group, send email to r2...@googlegroups.com.

To view this discussion on the web visit https://groups.google.com/d/msgid/r2dbc/6815c8c3-d5bb-4b5c-8b50-bdf32fed2ce2%40googlegroups.com.

For more options, visit https://groups.google.com/d/optout.

--

Greg Turnquist | Principal Software Engineer | Spring team at Pivotal
Mobile: +1 321-759-1143 | twitter/github/skype: gregturn | gturn...@pivotal.io

Pivotal.io

[Lukas Eder]

On Sunday, November 25, 2018 at 4:09:07 AM UTC+1, Greg Turnquist wrote:

So, it is the intent of the client library to offer only minimal functionality, and for an edge case like this, people should resort to using the SPI directly?

Yes.

In essence, you COULD write your app using the SPI, but it would probably be very...bulky.

The idea is to bundle up common paradigms into a humane client. The r2dbc-client library is just one take at doing things, inspired by JDBI. In other words, if you were to take JDBI and rewrite it from scratch, hooking into the SPI, you could implement something very analogous and easy for the JDBI community to pick regarding flows. If people are keenly interested, expand r2dbc-client to further implement parts of JDBI not yet covered. (I know nothing about JDBI, so I don't know what's missing.)

Excellent, thanks for the explanations.

 

I could imagine doing the same for jOOQ. Throw out JDBC and using R2DBC and Reactive Streams, how far can you go with building an alternative client?

I do have plans to eventually build an internal SPI in jOOQ as well, based on what I'm seeing in other SPIs like ADBA and R2DBC. That way, ADBA and R2DBC (and possibly other) drivers can be hooked into jOOQ directly, just like you're describing. Quite possibly, this abstraction might even be used to abstract over JDBC as well, as there shouldn't be any issues accessing a synchronous API via an asynchronous one (apart from the complexity and longer stack traces)... 

 

[Ben Hale]

Lukas,

Thanks for the awesome feedback, it’ll be really helpful to have someone with your experience giving us guidance on what can make client implementations more powerful.  The team has responded to most of your questions and we’ll move a lot of the discussion into GH issues for rapid inclusion, but one point I wanted to reinforce is the following. The SPI is very much intended to be the simplest possible thing for driver implementors even at the expense of being inhumane for direct usage.  The idea is that client implementations, jOOQ, Jdbi, Spring Data, etc. build humane APIs on top of it, tailoring those APIs to their communities and usage paradigms.  `r2dbc-client` is simply an example of one of those clients to demonstrate how one might do it, but given how quickly Spring Data has surpassed it, and the hope that jOOQ and others do the same, I would not be surprised if it never has very much adoption.

Thanks again for the feedback and I look forward to really digging into it with you.

-Ben

--

You received this message because you are subscribed to the Google Groups "r2dbc" group.
To unsubscribe from this group and stop receiving emails from it, send an email to r2dbc+un...@googlegroups.com.
To post to this group, send email to r2...@googlegroups.com.

To view this discussion on the web visit https://groups.google.com/d/msgid/r2dbc/ebcc7e52-59eb-4206-9c46-096273c64d9a%40googlegroups.com.

[Lukas Eder]

Hi Ben,

On Monday, November 26, 2018 at 4:52:09 PM UTC+1, Ben Hale wrote:

Lukas,

Thanks for the awesome feedback, it’ll be really helpful to have someone with your experience giving us guidance on what can make client implementations more powerful.

In fact, Oliver once reached out to me to ask me to join, which I gladly did (finally) :)

 

[...] but one point I wanted to reinforce is the following. The SPI is very much intended to be the simplest possible thing for driver implementors even at the expense of being inhumane for direct usage.  The idea is that client implementations, jOOQ, Jdbi, Spring Data, etc. build humane APIs on top of it, tailoring those APIs to their communities and usage paradigms.  `r2dbc-client` is simply an example of one of those clients to demonstrate how one might do it, but given how quickly Spring Data has surpassed it, and the hope that jOOQ and others do the same, I would not be surprised if it never has very much adoption.

Yes, it all makes sense now, thanks to you and Greg for the explanations.