Skip to content

Conversation

@roji
Copy link
Member

@roji roji commented Aug 27, 2022

@AndriySvyryd @ajcvickers one last design thought on this... This PR makes in/out parameters means "always both send and receive", as discussed. If, in the future, we implement optional sproc parameters with sentinel values, it would be a breaking change to change the meaning of in/out to "write only if the user set the property, read only if they didn't" (which is our regular SQL behavior). In other words, in order to allow the user to sometimes provide a value and sometimes not (in different commands) - today, without optional params/sentinels - we're making it so that their value is always sent and read back (in the same command).

I'm a bit concerned that we're painting ourselves into a corner; ideally, the write-and-read within the same command would be triggered by a separate opt-in, since it's probably really rare. Not merging this PR would mean users currently must choose either input or output, but not both, which doesn't seem too bad. If/when we have optional/sentinel parameters, they'd be able to do input/output without reading back values when they provide them.

But we've already discussed this too much... If you're guys this we should do this, I'm OK with it.

Closes #28704

@AndriySvyryd
Copy link
Member

Ok, how about this: We change the Metadata API to use a different enum for Direction that contains InputAndOutput and InputOrOutput, the current Fluent API will configure it as InputOrOutput and we can decide later how to expose InputAndOutput in the Fluent API.

@roji
Copy link
Member Author

roji commented Sep 8, 2022

@AndriySvyryd did the changes as we discussed, take a look.

(Note that this doesn't include creating our own ParameterDirection, I'll do that in a separate PR)

@roji roji mentioned this pull request Sep 8, 2022
Copy link
Member

@AndriySvyryd AndriySvyryd left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Docs/samples will need to be updated

@roji roji force-pushed the InputOutput branch 2 times, most recently from a63bde5 to deebb2d Compare September 9, 2022 09:15
@roji roji merged commit a7b82d0 into dotnet:release/7.0 Sep 9, 2022
@roji roji deleted the InputOutput branch September 9, 2022 10:56
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Support sproc input/output parameters on non-concurrency-token properties

2 participants