jphellemons/CommandAsSql

null values

Opened this issue ยท 7 comments

error in the function ParameterValueForSQL, if the value is null,

there should be a test on the value.

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
object paramValue = param.Value;
(followed by a check for null that returns the string "NULL")
before the switch statement and then use paramValue instead of param.Value in the ParameterValueForSQL method

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:

    public static String ParameterValueForSQL(this SqlParameter param)
    {
        object paramValue = param.Value; //assuming param isn't null

        if (paramValue == null)
            return "NULL";

        switch (param.SqlDbType)
        {
            case SqlDbType.Char:
            case SqlDbType.NChar:
            case SqlDbType.NText:
            case SqlDbType.NVarChar:
            case SqlDbType.Text:
            case SqlDbType.Time:
            case SqlDbType.VarChar:
            case SqlDbType.Xml:
            case SqlDbType.Date:
            case SqlDbType.DateTime:
            case SqlDbType.DateTime2:
            case SqlDbType.DateTimeOffset:
                return $"'{paramValue.ToString().Replace("'", "''")}'";
             
            //...

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:

    public static String ParameterValueForSQL(this SqlParameter param)
    {
        object paramValue = param.Value; //assuming param isn't null

        if (paramValue == null) //TODO: should probably use DBNull.Value instead or in combination with this
            return "NULL"; //TODO: naive code, won't work as is, need to replace later on = NULL with IS NULL at non-Update queries

        switch (param.SqlDbType)
        {

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