-
Notifications
You must be signed in to change notification settings - Fork 7
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
null values #1
Comments
Indeed! Nice addition! Feel free to give it a shot and submit a pr 👍 |
What should be done at case SqlDbType.Structured if value is null? If no special handling is needed there, then it should be enough to do in fact I'm preparing a PR for an implementation of the Type=Text case for SQLCommand so I'll add that change too (just need to consolidate your recent Roslynator changes to the code since I had done it on previous version) |
I've changed it to this:
and replaced all param.Value to paramValue below |
(note there are some more refactorings there like a renamed method parameter, using return instead of a local result variable to simplify code, minimize possibility of errors (in case one forgets a break or to set value to local variable compiler will say no all paths return a value) and to remove the breaks at switch |
sent the PR (#3) that should be fixing this issue too (just not sure if the fix is appropriate for the case where param.SqlDbType == SqlDbType.Structured - that is should I return just NULL in that case or something more complex with multiple NULLs depending on the internal structure of that stuctured type?) |
actually my suggested implementation above was naive, the resulting code is now:
that is it needs more work to implement the NULL issue correctly |
btw, I see https://github.com/jeroenpot/SqlHelper/blob/master/Source/Mirabeau.MsSql.Library/SqlGenerator.cs also has code similar to my naive 1st attempt of returning "null" at their "GetParameterValue" instead of also replacing "=[blanks]NULL" to "IS NULL" after that but only for non-Update queries |
error in the function ParameterValueForSQL, if the value is null,
there should be a test on the value.
The text was updated successfully, but these errors were encountered: