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

Table-valued parameters #1

Open
mausch opened this issue Oct 25, 2010 · 7 comments
Open

Table-valued parameters #1

mausch opened this issue Oct 25, 2010 · 7 comments

Comments

@mausch
Copy link
Owner

mausch commented Oct 25, 2010

Look into making table-valued parameters easy to use: http://msdn.microsoft.com/en-us/library/bb675163.aspx

@ascjones
Copy link
Contributor

ascjones commented Jun 9, 2014

I'll be needing this. I can go ahead and implement it, if you don't mind hinting how best to structure it.

Since it's Sql Server specific, we'll need to be dealing with an SqlParameter rather than the IDbDataParameter like so:

tvpParam.SqlDbType = SqlDbType.Structured;

I'll have a crack at it tomorrow.

@mausch
Copy link
Owner Author

mausch commented Jun 9, 2014

I've never used table-valued parameters myself so I'm not sure what it would take to make them usable. It seems that Parameter should have an additional TypeName property.
It would be nice to have this well-typed such that the TypeName property is only available for table-valued parameters. Make illegal states unrepresentable, etc.

@ascjones
Copy link
Contributor

Take a look at a first pass here:

geniussportsgroup@4bd4577

It only implements a single function execSPReader which is enough for now to get me going for my specific use case.

I just copied over some functions from FsSql.fs and modified them to deal with the SqlClient and TVP specific stuff just to keep it separate for now.

So still need to decide how the API is going to look for this and we can merge that stuff in, but the basic functionality is there. So would appreciate your input on that.

Let me know what you think!

@mausch
Copy link
Owner Author

mausch commented Jun 11, 2014

f282db0

Moved the table-valued parameter type to the general parameter type. This avoids the duplicate code/special casing in SqlClient.fs, while also keeping things API-compatible (I didn't have to change anything in the tests).

I also added the TypeName property, which was missing in your code.

Converting a record to a DataTable (your makeStructuredTableParam function) sounds interesting but I think it should be a separate thing, i.e. a function DataTable.ofRecords: 'a * 'a list -> DataTable
('a * 'a list is an encoding of a non-empty list, required because otherwise you can't build the DataTable schema)

What do you think?

@ascjones
Copy link
Contributor

Yep that looks like a much nicer solution - I wasn't sure how to make things API compatible so thanks a lot for that.

I've just merged your changes into my branch and added a DataTable extension as you suggest:

geniussportsgroup@d064b31

Although I'm not sure what you mean exactly with having to encode the non empty list...since with just the generic type parameter we can reflect over the properties to determine the schema? Which also allows the possibility of passing in an empty list. Like so (also in the commit)

let ofRecords<'a> (rs : 'a list) = 
    let fields = FSharpType.GetRecordFields typeof<'a>
    let dt = new DataTable()
    // add cols to DataTable
    for f in fields do
        if f.PropertyType.IsPrimitive || f.PropertyType = typeof<string> then
            dt.Columns.Add(f.Name, f.PropertyType) |> ignore
        else failwithf "Only primitive types and strings allowed for table valued params. Field %s was type %A" f.Name f.PropertyType
    // add rows to DataTable
    for r in rs do
        let rowVals = fields |> Array.map (fun f -> f.GetValue(r, null))
        dt.Rows.Add(rowVals) |> ignore
    dt

Anyway feel free to correct that if it is not what you meant.

If all looks good I will add a couple of extra tests and do a pull request.

Thanks again for all your help.

@mausch
Copy link
Owner Author

mausch commented Jun 13, 2014

Forget what I said about the non-empty list, it was all nonsense and you're right.
I pushed your changes, now you'd just have to make the "table valued parameter together with standard parameter" test work. I have no idea what the TypeName should be. Once that works, we can look into making the table parameter definition a bit shorter.

@ascjones
Copy link
Contributor

Cool just made a commit to get that test working. The type name should match that of the user defined table type in SQL Server - in this case my poorly named 'IntegerList':

CREATE TYPE [dbo].[IntegerList] AS TABLE([value] [int] NULL)

We have such a type in our databases just for passing in lists of ids to sprocs.

It was failing at first with an InvalidCastException because it was assigning the list of records directly into the Value property, without first converting it to a DataTable with DataTable.ofRecords. And the value can only be a DataTable or an IEnumerable for TVPs. Might it be a good idea to make our TVP type safe by restricting it to those two types?

Will look at it again on Monday.

EDIT: here's the commit: geniussportsgroup@47df14a

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

2 participants