tds icon indicating copy to clipboard operation
tds copied to clipboard

Add support for TVP [WIP]

Open drowzy opened this issue 8 years ago • 6 comments

This PR adds support for Table Valued Parameters. It's functional but can be considered a WIP. I would like some feedback and suggestions before I progress further.

Most functionality required for tvp is already present, there are few additions required:

The spec lists type restrictions and the corresponding replacement types. Implemented here.

As the columns does not include a value, the attributes can not be calculated when encoding the column. Due to this the attributes needs to be provided in the opts part of the column. For most types, encoding using encode_<type>_type function works with a %Paramter{value: nil}. With the exception of varchar & varbinary (probably more but I haven't encountered them yet).

I've added a new function for running stored procedures https://github.com/drowzy/tds/blob/b1991bce00d50131b83b034cb2bddfba547470e6/lib/tds.ex#L21 as I couldn't get it to work using Tds.query. It passes the statement in opts so that it can be used in a handle_execute callback (This is a bit dirty and I would like a suggestion for a cleaner solution), as it is required for the rcp name to be that of the SP you intend to run. This is how other tds libraries does I believe Also the only parameters passed is the ones specified as input (i.e no @handle).

Known shortcomings

  • encode_tvp_type does not take database & table name into consideration. According to the spec: "f the TVP is a parameter to a stored procedure or function where parameter metadata is available on the server side, the client can send all zero-length strings for TVP_TYPENAME".

  • No TVP_ORDER_UNIQUE

  • All types not thoroughly tested.

  • uniqueidentifiers does not seem to be parsed to a uuid when receiving a result.

Feedback is very much appreciated.

drowzy avatar Nov 30 '17 08:11 drowzy

This is a good addition to functionality. What's the hold-up on the acceptance? Anything I can do to help?

Deathklok-97 avatar Aug 10 '21 20:08 Deathklok-97

@drowzy, this is pretty close, are you going to pick it back up?

Deathklok-97 avatar Aug 19 '21 15:08 Deathklok-97

It’s been a long time! I can try to rebase and fix conflicts, when I get the time.

drowzy avatar Aug 19 '21 20:08 drowzy

Awesome, I forked and brought your stuff over. I ran into a little snafu with defaults of no count in the sp. It looks like the messages that came back were for affected rows, not just the Tds result set. I was going a little nuts chasing this down.

2021-08-19 16_36_52-(1)

Deathklok-97 avatar Aug 19 '21 21:08 Deathklok-97

I think I can make this return multiple result sets too. I don't know how common needing that functionality is. The company I'm currently at has dbas that maintain their own layer of sps as an api

Loopers97 avatar Aug 19 '21 23:08 Loopers97

@Deathklok-97 Oh if you already applied my changes to an up to date fork, then go for it! You can just reference and I'll close this PR. I haven't been working with SQL server for a long time and don't remember much from this implementation either so :).

drowzy avatar Aug 20 '21 12:08 drowzy