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

Add support for TVP [WIP] #49

Open
wants to merge 11 commits into
base: master
Choose a base branch
from
Open

Add support for TVP [WIP] #49

wants to merge 11 commits into from

Conversation

drowzy
Copy link

@drowzy drowzy commented Nov 30, 2017

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.

@Deathklok-97
Copy link

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

@Deathklok-97
Copy link

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

@drowzy
Copy link
Author

drowzy commented Aug 19, 2021

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

@Deathklok-97
Copy link

Deathklok-97 commented Aug 19, 2021

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)

@Loopers97
Copy link

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

@drowzy
Copy link
Author

drowzy commented Aug 20, 2021

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

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

Successfully merging this pull request may close these issues.

4 participants