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

Consider setting facets (size/precision/scale) systematically on DbParameters #18681

Closed
roji opened this issue Oct 31, 2019 · 2 comments
Closed

Comments

@roji
Copy link
Member

roji commented Oct 31, 2019

We don't currently systematically set facets on DbParameters to match the corresponding column's facets (we do set some facets in some cases). This hasn't been a problem up to now, as SQL Server accepted parameters for columns with differing facets. However, when Always Encrypted is on, it seems that parameter facets have to precisely match the columns (see dotnet/SqlClient#271).

Note that this is a provider-specific and probably also type-specific question. PostgreSQL for one does not support parameter facets at the protocol level, so the Npgsql implementation applies the facet client-side (e.g. truncating strings if Size is set).

@ajcvickers
Copy link
Contributor

@roji I believe if the facets are set on parameters, then SqlClient starts truncating the values, and it does so in a way that is inconsistent with how SQL Server would round otherwise. A quick search found #6538, although that may be more about issues with the temp table in the update pipeline--but that could be related here.

I think we need a good repro and then we need to really understand the SqlClient behavior here. If setting the parameters only works because of truncation such that application level truncation/rounding can be used instead, then that might be a better direction to go. Otherwise this may require changes in SqlClient.

@AndriySvyryd
Copy link
Member

If we decide to do this it would need to be opt-in to avoid the breaking change as we've seen many instances where the model facets don't match the database (and everything works).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants