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

Always use TypeHandler for parameters if available #339

Closed
wants to merge 1 commit into from
Closed

Always use TypeHandler for parameters if available #339

wants to merge 1 commit into from

Conversation

kashifsoofi
Copy link

TypeHandlers can override the logic for built-in SQL data types (int,
byte, DateTime etc.) when these are return values via
ITypeHandler.Parse, but ITypeHandler.SetValue was never being used for
parameters. Moved the typeHandlers.TryGetValue() call up to make sure a
type handler is always used if available.

TypeHandlers can override the logic for built-in SQL data types (int,
byte, DateTime etc.) when these are return values via
ITypeHandler.Parse, but ITypeHandler.SetValue was never being used for
parameters. Moved the typeHandlers.TryGetValue() call up to make sure a
type handler is always used if available.
@kashifsoofi
Copy link
Author

Credit goes to Matt Wilkinson for original change (#177)

@drusellers
Copy link

👍 want

@NickCraver
Copy link
Member

While I can see the use here, unfortunately this change can't really happen without a lot more love. There are no handlers for other types in the pipe such as most IEnumerable types, or any custom types where no such mapping actually exists. If you run the unit tests programs (e.g. build and \Tests\bin\Debug\Smackdown.exe), you can see a great many downstream breaks due to how parameters are handled in general.

@NickCraver
Copy link
Member

Since we can't merge either - I'll consolidate this into #177 so we can track the issue in one place.

@NickCraver NickCraver closed this Oct 15, 2015
@kwaclaw
Copy link

kwaclaw commented Sep 22, 2016

It seems #177 has been closed as well, without any resolution to the issue.

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