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

Improve API for IN/OUT parameters #216

Closed
lukaseder opened this issue Apr 22, 2021 · 0 comments
Closed

Improve API for IN/OUT parameters #216

lukaseder opened this issue Apr 22, 2021 · 0 comments
Labels
for: team-attention An issue we need to discuss as a team to make progress type: enhancement A general enhancement

Comments

@lukaseder
Copy link
Contributor

Feature Request

As discussed here: r2dbc/r2dbc-mssql#199 (comment), the API to manage IN/OUT parameters of stored procedures may need some refinement.

Is your feature request related to a problem? Please describe

It is currently not quite clear how an IN/OUT parameter needs to be declared in R2DBC. There is io.r2dbc.spi.Parameters.out(Type, Object) that seems to allow for:

  • Providing an in value
  • Registering an out value

But it:

  1. Doesn't document the fact that this is / might be its intended use
  2. Implies that the input type and the output type are of the same type

I'm not sure if 2) is a really a problem, just pointing out that in JDBC, it is possible theoretically, though unlikely useful, to bind an IN value of type X and register it as an OUT value of type Y

Describe the solution you'd like

There are different possible solutions, and I don't have a strong preference:

  • The simplest solution is to just document the status quo and establish a contract about Parameters.out() methods really being about IN/OUT parameters. I'm not convinced this is the best solution, seee below
  • Add new Parameters.inOut() methods that make this clear. In many RDBMS (SQL Server being an exception), there's a clear distinction between an OUT parameter (no in value possible) and an IN/OUT parameter (in value and out value). The current implementation doesn't reflect this distinction, because:
    • Parameters.out(SomeClass.class) creates an implicit null value and there's no way of knowing whether that null value is bound as an IN parameter for the IN/OUT case, or simply ignored
    • Parameters.out()methods return an object of typeio.r2dbc.spi.Parameter.Out, which doesn't also implement io.r2dbc.spi.Parameter.In`

I will provide a PR for the second alternative.

Given that Version 0.9.0.M1 does not yet support all possible ways of calling stored procedures (see r2dbc/r2dbc-mssql#199 (comment)), I think we will be fine introducing a breaking change in this area. I think that at least these two methods should go:

  • io.r2dbc.spi.Parameters.out(Type, Object)
  • io.r2dbc.spi.Parameters.out(Object)
lukaseder added a commit to lukaseder/r2dbc-spi that referenced this issue Apr 22, 2021
Signed-off-by: Lukas Eder <lukas.eder@gmail.com>
@mp911de mp911de added type: enhancement A general enhancement for: team-attention An issue we need to discuss as a team to make progress labels Apr 22, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
for: team-attention An issue we need to discuss as a team to make progress type: enhancement A general enhancement
Projects
None yet
Development

No branches or pull requests

2 participants