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

Allow large UDT buffers #340

Merged
merged 2 commits into from
Dec 5, 2019
Merged

Allow large UDT buffers #340

merged 2 commits into from
Dec 5, 2019

Conversation

Wraith2
Copy link
Contributor

@Wraith2 Wraith2 commented Dec 3, 2019

fixes #329

Changes the max size check so that it depends on the protocol version of the connection to the server. SQL Server 2008 and later use TDS verison >= 7.3 which can support large UDT types up to int.MaxValue not the previous short.MaxValue used.

I removed n byte[] allocation which wasn't being used. I reordered the type name check so it occurs before all the work to get and check the value. I changed the exception type from IndexOutOfRange exception with the default message to ArgumentOutOfRange exception with an explanation containing the size and max supported size, this will need localizing.

I'm hoping someone can do the dance required to point EF core at the PR build nuget version and see if the original problem reported by @capeseanis removed. I'm finding it rather difficult to try and work through. If the method is valid the same work can be done on the desktop framework version.

/cc @ajcvickers @aprognimak

@ajcvickers
Copy link
Member

Many thanks, @Wraith2. This looks good to me.

@Wraith2 Wraith2 changed the title WIP Allow large UDT buffers Allow large UDT buffers Dec 5, 2019
Copy link
Contributor

@David-Engel David-Engel left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good. Just needs the one small change.

Copy link
Contributor

@David-Engel David-Engel left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM. Thanks!

@cheenamalhotra cheenamalhotra added this to the 1.2.0-preview1 milestone Dec 5, 2019
@cheenamalhotra cheenamalhotra merged commit d38d554 into dotnet:master Dec 5, 2019
@Wraith2 Wraith2 deleted the udtbytes branch December 6, 2019 19:22
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.

UDT parameters are limited to 64K
4 participants