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

For reverse engineered procedures, it's not possible to set an initial value for an out parameter #1069

Closed
HimbeersaftLP opened this issue Aug 9, 2021 · 17 comments · Fixed by #1145

Comments

@HimbeersaftLP
Copy link

SQL Server procedures allow the executor to define an initial value for parameters declared as "OUTPUT".

The method generated by EF Core Power Tools' reverse engineering feature however, has the direction of the SqlParameter hardcoded to Output (as opposed to InputOutput).

_sb.AppendLine("Direction = System.Data.ParameterDirection.Output,");

Steps to reproduce

  1. Create a procedure that uses the value of one of its OUTPUT parameters.
  2. Use EF Core power Tools to generate the models
  3. You will notice, that you cannot provide an input value to the parameter

Further technical details

EF Core Power Tools version: 2.5.692

Database engine: SQL Server

Visual Studio version: Visual Studio 2019 16.10.4

@ErikEJ
Copy link
Owner

ErikEJ commented Aug 9, 2021

Please provide a full repro using for example Northwind, and I will have a look

@ErikEJ
Copy link
Owner

ErikEJ commented Aug 9, 2021

Are you proposing it should be always "InputOutput" ?

@ErikEJ
Copy link
Owner

ErikEJ commented Aug 10, 2021

Ping?

@HimbeersaftLP
Copy link
Author

Are you proposing it should be always "InputOutput" ?

I am not sure about the backwards compatibility of that (also is there a possible performance impact?). On the other hand, I also have no idea if there is another way to decide which one to use, short of making it a global option.

Please provide a full repro using for example Northwind, and I will have a look

I'll try to as soon as I can, if that is still needed.

Sorry for the late response.

@ErikEJ
Copy link
Owner

ErikEJ commented Aug 10, 2021

Does everything work as expected if you change all Output parameters to Input output?

@ErikEJ
Copy link
Owner

ErikEJ commented Aug 11, 2021

No need for repro - I will try to make it always InputOutput for now, and if issues arise, make a global switch to revert to current implementation.

@HimbeersaftLP
Copy link
Author

Does everything work as expected if you change all Output parameters to Input output?

It does for me and I can't think of any way it could cause a problem, but I am by no means an expert at SQL Server stuff.

(sorry again for the late response)

@ErikEJ
Copy link
Owner

ErikEJ commented Aug 13, 2021

Thanks, I also looked at a tool similar to mine, and it maps all non input parameters as input/output, so I will just change to that.

@ErikEJ ErikEJ closed this as completed in 36f96e6 Aug 13, 2021
@ErikEJ
Copy link
Owner

ErikEJ commented Aug 13, 2021

Thanks for reporting this!

@ErikEJ
Copy link
Owner

ErikEJ commented Aug 13, 2021

Fixed in latest daily

@HimbeersaftLP
Copy link
Author

Thanks ♥️

@ErikEJ
Copy link
Owner

ErikEJ commented Oct 9, 2021

@HimbeersaftLP If you would like this fixed, please provide a repro so I can understand the issue better.

@whschultz
Copy link

In looking through the code to open up my redundant bug report, I got to this point, thinking you'll have to retrieve the supplied value from the passed in via your auto-generated OutputParameter. However, that object throws an exception if you try to retrieve a value when it hasn't been set. Therefore, I was thinking that because of the possibility of default values in the T-SQL, you'll need the possibility to check if a value has been supplied on that OutputParameter, and if so, use it. You could potentially then set the direction to InputOutput if it has been set, but stick with Output if it hasn't.

I found this as a good discussion about the behavior when mixing these directions with default values in the T-SQL.

ErikEJ added a commit that referenced this issue Oct 19, 2021
ErikEJ added a commit that referenced this issue Oct 19, 2021
@ErikEJ
Copy link
Owner

ErikEJ commented Oct 20, 2021

I implemented a fix for this in the latest daily build, would be grateful if you could try it out.

@Durgesh132
Copy link

Yeah INPUT/OUTPUT parameter is required when we are creating and updating any record to table using same(single) stored procedure. So for create , no value will be set to input/output parameter and after saving record , id of new record will be returned through input/output parameter.
For update , value will be set to input/output parameter and based on that record will be updated.

Thanks Hank for adding the missing functionality to EF Core Power Tools.

@ErikEJ
Copy link
Owner

ErikEJ commented Oct 20, 2021

@Durgesh132 I created the PR - but have you verified the latest daily build works as expected?

@Durgesh132
Copy link

@ErikEJ @whschultz
I verified with Version 2.5.794 which was updated 20 hrs back and yeah It worked as expected.

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 a pull request may close this issue.

4 participants