Handle nil params in ExecuteSql

[drewlesueur]

Issue #29

I have tested this with Sybase 12.5, However not with other versions of Sybase/MSSQL.

I took a tip from go-mssqldb where it associates null with "nvarchar(1)", though I am not exactly sure if it's the same context. https://github.com/denisenkom/go-mssqldb/blob/b91950f658ecd54342d783495e1aadf48a55e967/types.go#L1124

[daniel-redcat]

@drewlesueur You can test against a local docker instance of latest mssql to make sure it works for that too. docker pull mcr.microsoft.com/mssql/server:2019-latest docker run -e 'ACCEPT_EULA=Y' -e 'SA_PASSWORD=yourStrong(!)Password' -p 1433:1433 -d mcr.microsoft.com/mssql/server:2019-latest

[drewlesueur]

@daniel-redcat Thank you. I did that and added some more tests. Note that this feature does not work with these types: timestamp, binary, varbinary(max). But because of the convenience of it working with the other types, I still think this feature is worth adding.

[drewlesueur]

@ianic Does this feature seem like something you would consider approving? Thank you!

[gljubojevic]

@drewlesueur would you be so kind to provide a bit more context to a problem proposed PR should resolve. e.g. at least SQL example where you use ExecuteSql and golang code to execute that SQL including variable types.

Reason why I'm asking is following While this might work in some cases I would say that in many really can't, e.g. inserting null into int column would require implicit conversion on DB side, and not every DB type can be easily implicitly converted from nvarchar(1).

By further looking at proposed PR, it actually resolves only one use case when ExecuteSql is called explicitly with "nil" e.g.

ExecuteSql("select * from table where ID=?", nil)

This is crude example and maybe I'm missing something here, please provide example.

More importantly I would expect question more in direction like this:

var p *int
ExecuteSql("insert into table set p=?", p)

[drewlesueur]

@gljubojevic Yes, thank you for requesting this.

The use case that I am trying to target is when we use gofreetds as a driver for Go's database/sql package, specifically the DB.Exec function.

In a DB.Exec context this change will work if I use an explicit nil, a typed nil, or one of the sql.Null* types. I understand that this change is really only very helpful when we use this as a driver for database/sql, and not that helpful when using the underlying ExecuteSql call directly.

Also, I have added tests to show that the implicit conversion will work for all the types used in the freetds_types table in the tests, with the exception of timestamp, binary, and varbinary(max). Even though it will not work for those 3 types, because it works with all the other types in the freetds_types test table, I think it is still a worthwhile change.

Here is an example fo Go code that will now work after this change, that wouldn't work before:

package main

import (
    "database/sql"
    _ "github.com/minus5/gofreetds"
)

func main() {
     db, err := sql.Open("mssql", "Your Connection String Here...")
     if err != nil {
         panic(err.Error())
     }

     // Works with explicit nil.
     db.Exec("INSERT INTO some_table (some_col) VALUES (?)", nil)

     // Works with typed nil.
     var p *int
     db.Exec("INSERT INTO some_table (some_col) VALUES (?)", p)

     // Works with sql.Null* types.
     db.Exec("INSERT INTO some_table (some_col) VALUES (?)", sql.NullInt32{Int32: 0, Valid: false})
 }

You are right that this change is not very helpful when using ExecuteSql directly. To get that to work, we'd have to add some more type switching in go2SqlDataType which is doable, but I wanted to focus my change specifically on the database/sql driver use case.

I hope this clarifies my intention and benefits of this change. Thank you.