FSharp.Data.SqlClient
FSharp.Data.SqlClient copied to clipboard
Fixed length binary columns in user defined table types fail when constructed
Issue Summary
When using a user defined table type with a fixed length binary column the wrong constructor for SqlMetaData
is used resulting in the following exception.
System.ArgumentException: 'The dbType Binary is invalid for this constructor.'
This line looks like it's causing the issue, which incorrectly assumes if it is a fixed length that it can safely use the two parameter constructor:
https://github.com/fsprojects/FSharp.Data.SqlClient/blob/0bc5129586512316570da1f27e588287d7e8be35/src/SqlClient.DesignTime/DesignTime.fs#L544
To Reproduce
- Create a custom table type
CREATE TYPE [dbo].[CustomTableType] AS TABLE(
[Value] [binary](16)
)
- Use the type in a query
type StaticProvider =
SqlCommandProvider<"
Declare @Updates as CustomTableType = @Inputs;
" , staticConnectionString>
use cmd = new StaticProvider(context.connection, transaction = context.transaction)
cmd.AsyncExecute(
data.inputs
|> List.map(fun x ->
StaticProvider.CustomTableType( // <--- Error in constructor
Value = x.value
)
)
Error
System.ArgumentException: 'The dbType Binary is invalid for this constructor.'
Expected behavior
There should not be an error in this case.
What you can do
- [ ] I am willing to contribute a PR with a unit test showcasing the issue
- [x] I am willing to test the bug fix before next release
Thanks for the issue report @daniellittledev.
In #362, a workaround was to use nvarchar(max)
instead of a specified length, I'm not sure if it would work or if you can use that in meantime the bug get addressed.
I'll try to put this under test and fix the call to SqlMetaData constructor accordingly.
@smoothdeveloper I've submitted a PR with what I think may be the fix for this issue. Please check it out when you have a minute: https://github.com/fsprojects/FSharp.Data.SqlClient/pull/418
@suou-ryuu thanks a lot for the fix PR, with added test.
I'll take a moment to review and run the test locally, and issue a new release when possible.
@daniellittledev, if you happen to test the fix on your own and confirm in meantime, please let us know.
Thanks again all!
@smoothdeveloper Thanks for the quick reply.
I'll take a moment to review and run the test locally, and issue a new release when possible.
Superb. Looking forward to it :)
@smoothdeveloper Sorry to pester, but do you have an ETA for when you'll be able to confirm this fix and issue a new release? We currently need this as it's blocking progress on a few projects.
Thank you in advance
@suou-ryuu there should be a 2.1.1 version with your PR changes upon validation from nuget.org.
@smoothdeveloper Fantastic. Thank you. I see the 2.1.1 on nuget.org; I've tested it locally and it's all working as expected 😄 Thank you for handling so quickly
@suou-ryuu thanks for confirming on your end and no problems.
Just so you know, once you've got the main library to build, you can generate the nuget package yourself: build NuGet -st
This allows you to drop a new version locally before a release is made.
@daniellittledev if you have a chance to try the new release and if it fixes this issue, we can close it?
Thanks again @suou-ryuu for contributing the fix.