Issue Inserting NULL Into Database

[blaskovicz]

I'm using this driver with mssql and sqlx.DB and have run into an issue where when I try to insert a null, it gets marshalled to the wrong column type during runtime and outputs the following failure:

Failure Message:

--- FAIL: TestNULLs (0.95s)
        Error:          Received unexpected error "mssql: Implicit conversion from data type nvarchar to binary is not allowed. Use the CONVERT function to run this query."
        Messages:       failed to insert null binary column

Reproducer code:

import (
  "testing"
  "github.com/jmoiron/sqlx"
  mssql "github.com/denisenkom/go-mssqldb"
  "github.com/stretchr/testify/require"
)

func TestNULLs(t *testing.T) {
  var db *sqlx.DB
  db = connectTestDB()
  if _, ok := db.Driver().(*mssql.MssqlDriver); !ok {
    t.Skip("tests are only implemented for mssql, currently")
  }
  db.MustExec("DROP TABLE IF EXISTS [dbo].[test]")
  db.MustExec("CREATE TABLE [dbo].[test] ([id] int not null primary key, [sample_col] binary(16))")
  r, err := db.Exec("INSERT INTO [dbo].[test] ([id], [sample_col]) values (?, ?)", 1, nil)
  require.NoError(t, err, "failed to insert null binary column")
  rows, err := r.RowsAffected()
  require.NoError(t, err, "failed to get rows affected for insert null")
  require.Equal(t, 1, int(rows), "expected to have inserted 1 null row")
}

[kardianos]

Are you able to reproduce this without sqlx or testify?

[blaskovicz]

Refactored to use just database/sql, and it still fails with the same error:

Error

--- FAIL: TestNULLs (0.04s)
        repro_test.go:26: Failed to insert data: mssql: Implicit conversion from data type nvarchar to binary is not allowed. Use the CONVERT function to run this query.

Simplified Code

import (
  "database/sql"
  "testing"

  _ "github.com/denisenkom/go-mssqldb"
)

func TestNULLs(t *testing.T) {
  db, err := sql.Open("mssql", "server=somehost;user id=someuser;password=somepass;database=somedb")
  if err != nil {
    t.Fatalf("Failed to connect to db: %s", err)
  }

  if _, err = db.Exec("DROP TABLE IF EXISTS [dbo].[test]"); err != nil {
    t.Fatalf("Failed to drop table: %s")
  }

  if _, err = db.Exec("CREATE TABLE [dbo].[test] ([id] int not null primary key, [sample_col] binary(16))"); err != nil {
    t.Fatalf("Failed to create table: %s", err)
  }

  res, err := db.Exec("INSERT INTO [dbo].[test] ([id], [sample_col]) values (?, ?)", 1, nil)
  if err != nil {
    t.Fatalf("Failed to insert data: %s", err)
  }

  if rowCount, err := res.RowsAffected(); err != nil {
    t.Fatalf("Failed to get affected rows: %s", err)
  } else if int(rowCount) != 1 {
    t.Fatalf("Expected 1 row affected")
  }
}

[kardianos]

Thanks. This can be part of a unit test, easier for me to see the problem. I'll look into it.

[dimdin]

A NULLTYPE = 0x1f is defined here: https://msdn.microsoft.com/en-us/library/dd341171.aspx It can be used in makeParam (in mssql.go) instead of typeNVarChar

[nemec784]

Any news?

[kardianos]

Thanks for the ping. This fell of my radar. Assigning to myself now.

[nemec784]

@denisenkom, @kardianos, any news?

[denisenkom]

So I've changed parameter type to NULLTYPE but there is still a problem.

We use sp_executesql to run queries with parameters, and it requires all parameters to have types. Those types are passed into a second parameter for sp_executesql called @params. I tried passing different types, e.g. int, date, char, binary, but none of them work for all receiving types. Only nvarchar works for most of types except binary.

[denisenkom]

basically we need to find a type to replace string SOMETHING below, which would work for all types:

sp_executesql('declare @x binary = @p1; select @x', '@p1 SOMETHING', null) sp_executesql('declare @x date = @p1; select @x', '@p1 SOMETHING', null) sp_executesql('declare @x int = @p1; select @x', '@p1 SOMETHING', null) ...

[kardianos]

We can fix this in go1.9. We can define top level types (mssql.Binary, mssql.Text, ...) and implement https://godoc.org/database/sql/driver#NamedValueChecker The NamedValueChecker. They pass the parameter into the driver like db.Exec("SQL", mssql.Binary(nil)) which will get verified and passed on by the custom NamedValueChecker, and any of these custom types need to be handled internally as well.

@denisenkom Does this sound workable to you? I have a branch in the wings that already implements the NamedValueChecker for OUPUT parameter support kardianos-poc-go1.9 that I can bring up to speed and merge in at some point in the near future. Go is still at the beginning of the go1.10 merge window so I need to work on these type of issues in the next couple of weeks.

[neilotoole]

@kardianos @denisenkom As discussed in your comments above, is this issue still considered to be solvable via NamedValueChecker?

In the case of an app I'm working on, I can report that this still happens for BINARY and VARBINARY cols, recently tested using go version go1.14.3 darwin/amd64 and go-mssqldb v0.0.0-20200428022330-06a60b6afbbc against Microsoft SQL Server 2017 (RTM-CU19) (KB4535007) - 14.0.3281.6 (X64).

[jeffdupont]

this is the same as issue #530

[blaskovicz]

this is the same as issue #530

m4ver1k opened this issue on Oct 14, 2019

blaskovicz opened this issue on Dec 16, 2016

[jeffdupont]

I'm not saying that issue was first, I'm just drawing a line that whatever is underlying, they're connected.