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

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

Open
1 of 2 tasks
daniellittledev opened this issue Dec 14, 2021 · 8 comments
Open
1 of 2 tasks

Comments

@daniellittledev
Copy link

daniellittledev commented Dec 14, 2021

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:

if typeInfo.IsFixedLength then

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
  • I am willing to test the bug fix before next release
@smoothdeveloper
Copy link
Collaborator

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.

@suou-ryuu
Copy link

@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: #418

@smoothdeveloper
Copy link
Collaborator

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

@suou-ryuu
Copy link

@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
Copy link

@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

@smoothdeveloper
Copy link
Collaborator

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

@suou-ryuu
Copy link

@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

@smoothdeveloper
Copy link
Collaborator

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

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

No branches or pull requests

3 participants