Skip to content

Fix: non primitive out types#340

Merged
smoothdeveloper merged 2 commits intofsprojects:masterfrom
thinkbeforecoding:fix-non-primitive-out-types
May 9, 2019
Merged

Fix: non primitive out types#340
smoothdeveloper merged 2 commits intofsprojects:masterfrom
thinkbeforecoding:fix-non-primitive-out-types

Conversation

@thinkbeforecoding
Copy link
Copy Markdown
Contributor

This is a fix for non primitive out types in stored procedures.

I had a case with a output parameter of type Guid, the type provider could not use the Expr.Value(Activator.CreateInstance(…)) since it can do so only for primive types.
In the case of a non primite type, we use Expr.DefaultValue(t) which calls the default ctor.

The change in SetRef<'t> was necessary to set the output value to default<'t> when the parameter value is actually DBNull. This raise the question of actually making it optional…

@smoothdeveloper
Copy link
Copy Markdown
Collaborator

@thinkbeforecoding thanks for reporting the issue / fix.

Could you add a non regression test and a comment around the added check with a bit of context?

@thinkbeforecoding
Copy link
Copy Markdown
Contributor Author

For the tests, I added a storedproc that pass or not the input Guid to out parameter depending on a bool parameter.
There is a tests where it's called with true, to check that everything is ok when there is a value, and a test with false to test when the out parameter is not set.

@smoothdeveloper smoothdeveloper self-assigned this May 9, 2019
@thinkbeforecoding
Copy link
Copy Markdown
Contributor Author

Is it ok like this ?

@smoothdeveloper smoothdeveloper merged commit 4450560 into fsprojects:master May 9, 2019
@thinkbeforecoding
Copy link
Copy Markdown
Contributor Author

No prb, I pushed a version to our local nuget feed.
Thx !!

smoothdeveloper added a commit that referenced this pull request May 27, 2019
…ooling for maintenance of this project)

* release note / fix for #340
* version number
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.

2 participants