Return the option to clear the returning clause of a Query

[GeorgiPetkov]

Use case:

The functionality was implicitly removed as part of #11437.
I've described it here.
In short, this is preventing proper implementation and is a regression for https://github.com/gofabian/spring-boot-data-r2dbc-jooq.
This is only preventing queries using the RETURNING clause, but I doubt nowadays there's a project that's not making use of it at least once.
Seeing the commit would make it clear why the RETURNING clause can no longer be cleared through the provided API - 82c18b3.

Possible solution you'd like to see:

Add a method to reset/remove the returning clause in AbstractDMLQuery. The current setters like AbstractDMLQuery#setReturning now default to including all fields when an empty varargs/collection is passed instead of dropping it.

Possible workarounds:

AFAIK, the only workaround is to set the returning field of the query object through reflection which is obviously not a clear solution so for now I'm rather stuck with version 1.14.8.

Versions:

• jOOQ: 1.14.9+, caused by 82c18b3

[lukaseder]

Thanks for your report

In short, this is preventing proper implementation and is a regression for https://github.com/gofabian/spring-boot-data-r2dbc-jooq.

I'm assuming you're still referring to these lines here:

https://github.com/gofabian/spring-boot-data-r2dbc-jooq/blob/3e86bcbeb9cf70d45ddabb50a539890003a573d4/src/main/java/gofabian/r2dbc/jooq/ReactiveQueryExecutor.java#L140-L142

I can see how there might be a regression, but rest assured that none of the jOOQ QueryPart types were meant to be able to add/remove things at will, or even introspect them (as you've noticed and worked around using reflection). Sure, we could add
a clearReturning() method and many others, and write tons of tests to ensure this works. There had been numerous such feature requests in the past, all rejected, e.g. #7588. This is an all or nothing situation. We can't just add a clear() method for this single thing, and not add it everywhere, or we'll have 1000 issues in no time and a poorly designed API. So, it's a lot of work.

Irrespective of the above, this isn't where jOOQ wants to go, strategically, for these reasons:

1. Immutability is a long term goal, which will offer a lot of benefits to the DSL API
2. The model API will eventually be removed in the very long term. A first step towards this might be done soon: Reverse relationship between model and DSL APIs #11241
3. Once we're ready, a much better (hopefully immutable) query object model will offer access to its component parts, and you can read them safely, and transform a Query Q1 to a similar query Q2 (e.g. adding/dropping a RETURNING clause), without tampering with Q1 or any internals.

So, instead of focusing on this "regression" (it happened to work by accident, and because you read the implementation, not because the API made any guarantees), let's focus on these 2 things:

1. How you should call the jOOQ API instead, pretending it is immutable already today (your code will be cleaner), rather than adding things, and removing them again after further state transfer and/or mutation. (I don't really understand why something needs to be patched prior to the call to createR2dbcExecuteSpec and then restored again. It doesn't seem right to me)
2. I pinged you folks in the past: jOOQ 3.15 support for R2DBC gofabian/spring-boot-data-r2dbc-jooq#36. Is there still need for your module and API? Formal R2DBC support from within jOOQ is much cleaner than an external solution. Even if your module does something that jOOQ 3.15 doesn't do yet (e.g. reactive DAOs), the question at hand should resolve itself, because the DML RETURNING query will be executed via jOOQ, natively, I suspect?

AFAIK, the only workaround is to set the returning field of the query object through reflection which is obviously not a clear solution so for now I'm rather stuck with version 1.14.8.

It's a perfectly valid option, because it's as unclean as relying on an undocumented internal behaviour. Perhaps a bit more so. But how much worse can it get, given: https://github.com/gofabian/spring-boot-data-r2dbc-jooq/blob/fc144063f072b7187201ab697af56c94b4c68284/src/main/java/gofabian/r2dbc/jooq/JooqInternals.java 😀

[GeorgiPetkov]

I'm not a contributor to the spring-boot-data-r2dbc-jooq, just a user of it and I was willing to contribute after consulting with you on what path to take.
Yes, I was referring to the same lines.
Regarding the patching - it is to avoid the later added automatically RETURNING clause by the driver because of the used method. I still haven't gone deeper into the reasons why this was implemented that way in the first place.
I didn't know about the reactive changes, I'll definitely try them out. I've always been looking at spring-boot-data-r2dbc-jooq as a temporary solution (and I would guess the author sees it this way as well). I'll definitely look into adding feedback for missing features after having some experience already.

I agree with your vision for future development and I'll adapt accordingly.
Thank you for your quick and valuable responses.