FSharp.Data.SqlClient icon indicating copy to clipboard operation
FSharp.Data.SqlClient copied to clipboard

Fixed length binary columns in user defined table types fail when constructed

Open daniellittledev opened this issue 2 years ago • 8 comments

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

  1. Create a custom table type
CREATE TYPE [dbo].[CustomTableType] AS TABLE(
	[Value] [binary](16)
)
  1. 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

daniellittledev avatar Dec 14 '21 07:12 daniellittledev

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 avatar Dec 16 '21 09:12 smoothdeveloper

@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 avatar Apr 04 '22 13:04 suou-ryuu

@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 avatar Apr 04 '22 14:04 smoothdeveloper

@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 :)

suou-ryuu avatar Apr 04 '22 14:04 suou-ryuu

@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 avatar Apr 07 '22 00:04 suou-ryuu

@suou-ryuu there should be a 2.1.1 version with your PR changes upon validation from nuget.org.

smoothdeveloper avatar Apr 07 '22 21:04 smoothdeveloper

@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 avatar Apr 07 '22 23:04 suou-ryuu

@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.

smoothdeveloper avatar Apr 09 '22 08:04 smoothdeveloper