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

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

Closed
roji opened this issue Aug 13, 2022 · 5 comments · Fixed by #28908
Closed

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

roji opened this issue Aug 13, 2022 · 5 comments · Fixed by #28908
Assignees
Labels
area-sprocs closed-fixed The issue has been fixed and is/will be included in the release indicated by the issue milestone. customer-reported type-enhancement
Milestone

Comments

@roji
Copy link
Member

roji commented Aug 13, 2022

In some scenarios, users may want to provide a value for some property, but also read back the property. This can be useful when the user-provided value is somehow transformed in the database, and then we want to propagate the transformed value back into the entity. With regular SQL this can be achieved via triggers, or by wrapping a sproc and having an input/output parameter for the property.

See conversation in #28553 (comment)

@ErikEJ
Copy link
Contributor

ErikEJ commented Aug 13, 2022

Not sure if it is relevant, but I ended up mapping all OUTPUT parameters as InputOutput (except the return value).

See related discussion here: ErikEJ/EFCorePowerTools#1069

@ajcvickers ajcvickers modified the milestones: Backlog, 7.0.0 Aug 16, 2022
@roji
Copy link
Member Author

roji commented Aug 16, 2022

This covers two things:

  • The same command both writing a value and reading it in the same command
  • Have a store-generated column which is written explicitly in one command, and read from another.

@roji
Copy link
Member Author

roji commented Aug 18, 2022

Some outputs from a discussion with @ajcvickers...

Problem 1

For the "same command" scenario, we're discussing writing the value when updating the entity, but also reading it back (if the property is ValueGeneratedOnUpdate).

However, there's also the case of inserts; just like a sproc/trigger/whatever may change the incoming value for an UPDATE, it may do so for an INSERT. In other words, to be consistent here, we'd need to read back ValueGeneratedOnAdd properties even when a value is provided by the user (since it may be modified db-side). We could start doing this, but that means we'd start sending back all IDs from the database even when they're explicitly given by the user, just to support this exotic scenario - which doesn't seem right.

Technically, the same is true of ValueGeneratedOnUpdate: the fact that a property is ValueGeneratedOnUpdate doesn't necessarily tell us if it's only generated/manipulated when no value is provided by the user, or also manipulated when the user provides a value. If we just start reading back values whenever the property is ValueGeneratedOnUpdate, we'd potentially be needlessly sending back values when we don't need to. This is admittedly really exotic, but there may be a case here for consistency with add.

In other words, to implement this we may be lacking additional metadata, which tells us to read back the property value on insert/update even when the user provides it.

Problem 2

For the non-same-command scenario, i.e. just allow the value to be either written or read (in different commands), there's still a problem with sprocs. Unlike regular SQL, sprocs require us to always provide all the parameters, making it difficult to do optional parameters (i.e. you can provide the value, but if you don't it'll get generated). The user could tell us about a sentinel value which we'd set when a property isn't set/changed by the user, and write their sprocs to recognize that value and act accordingly. Or there could be another flag parameter to say whether the parameter is set or not. But these seem like quite advanced features which we probably don't want to include for 7.0.

I'll bring this back to design, @AndriySvyryd if I've completely misunderstood things let me know.

@AndriySvyryd
Copy link
Member

We could start doing this, but that means we'd start sending back all IDs from the database even when they're explicitly given by the user, just to support this exotic scenario - which doesn't seem right.

It would only happen for InputOutput parameters, I don't think we need to do it for non-sproc mapping

Unlike regular SQL, sprocs require us to always provide all the parameters, making it difficult to do optional parameters (i.e. you can provide the value, but if you don't it'll get generated). The user could tell us about a sentinel value which we'd set when a property isn't set/changed by the user, and write their sprocs to recognize that value and act accordingly.

Currently the sentinel would be the CLR default value equivalent, I think this is part of the more general issue of lack of optional parameter support

@roji
Copy link
Member Author

roji commented Aug 24, 2022

Decision: when a parameter is defined with Direction=InputOutput, that's an opt-in to always write the value, and always read it back (and propagate it).

@roji roji removed the needs-design label Aug 25, 2022
@roji roji changed the title Support writing and reading of store-generated properties Support sproc input/output parameters on non-concurrency-token properties Aug 27, 2022
roji added a commit to roji/efcore that referenced this issue Aug 27, 2022
@roji roji added the closed-fixed The issue has been fixed and is/will be included in the release indicated by the issue milestone. label Aug 27, 2022
roji added a commit to roji/efcore that referenced this issue Sep 8, 2022
roji added a commit to roji/efcore that referenced this issue Sep 9, 2022
roji added a commit to roji/efcore that referenced this issue Sep 9, 2022
roji added a commit to roji/efcore that referenced this issue Sep 9, 2022
@ajcvickers ajcvickers modified the milestones: 7.0.0, 7.0.0-rc2 Sep 9, 2022
@ajcvickers ajcvickers modified the milestones: 7.0.0-rc2, 7.0.0 Nov 5, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area-sprocs closed-fixed The issue has been fixed and is/will be included in the release indicated by the issue milestone. customer-reported type-enhancement
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants