Repository: denisenkom/go-mssqldb
Pull request #686 (2021-07-30 13:42:10)

fix size reporting for typeBigBinary
binary(x) is a variable size type, not fixed size. #685 

shueybubbles: If I'm reading this log correctly, I think the tests can't reliably use testing.T's log as the driver logger because testing.T's logger isn't thread/goroutine-safe. Is that accurate?

```
WARNING: DATA RACE
Read at 0x00c0006403a3 by goroutine 68:
  testing.(*common).logDepth()
      c:/go114/src/testing/testing.go:727 +0xab
  testing.(*common).log()
      c:/go114/src/testing/testing.go:720 +0x96
  testing.(*common).Logf()
      c:/go114/src/testing/testing.go:766 +0x28
  testing.(*T).Logf()
      <autogenerated>:1 +0x7c
  github.com/denisenkom/go-mssqldb.testLogger.Printf()
      C:/gopath/src/github.com/denisenkom/go-mssqldb/tds_test.go:253 +0xa2
  github.com/denisenkom/go-mssqldb.(*testLogger).Printf()
      <autogenerated>:1 +0x35
  github.com/denisenkom/go-mssqldb.optionalLogger.Printf()
      C:/gopath/src/github.com/denisenkom/go-mssqldb/log.go:18 +0xb5
  github.com/denisenkom/go-mssqldb.processSingleResponse()
      C:/gopath/src/github.com/denisenkom/go-mssqldb/token.go:706 +0x1e88
Previous write at 0x00c0006403a3 by goroutine 73:
  testing.tRunner.func1()
      c:/go114/src/testing/testing.go:1036 +0x46e
  testing.tRunner()
      c:/go114/src/testing/testing.go:1054 +0x214
Goroutine 68 (running) created at:
  github.com/denisenkom/go-mssqldb.startReading()
      C:/gopath/src/github.com/denisenkom/go-mssqldb/token.go:774 +0xab
  github.com/denisenkom/go-mssqldb.(*Stmt).processQueryResponse()
      C:/gopath/src/github.com/denisenkom/go-mssqldb/mssql.go:638 +0x170
  github.com/denisenkom/go-mssqldb.(*Stmt).queryContext()
      C:/gopath/src/github.com/denisenkom/go-mssqldb/mssql.go:633 +0x1c7
  github.com/denisenkom/go-mssqldb.(*Stmt).QueryContext()
```

This is what open does:

```go
func open(t *testing.T) *sql.DB {
	checkConnStr(t)
	SetLogger(testLogger{t})
	connector, err := NewConnector(makeConnStr(t).String())
	if err != nil {
		t.Error("Open connection failed:", err.Error())
		return nil
	}
	return sql.OpenDB(connector)
}
```

shueybubbles: should testLogger methods put a lock around the l.t.Log calls?
```go
type testLogger struct {
	t testing.TB
}

func (l testLogger) Printf(format string, v ...interface{}) {
	l.t.Logf(format, v...)
}

func (l testLogger) Println(v ...interface{}) {
	l.t.Log(v...)
}
```

tc-hib: Isn't it the same as #680? I think you can safely ignore it for this PR :)

Patch:
diff --git a/types.go b/types.go
index cae1992..822a7d8 100644
--- a/types.go
+++ b/types.go
@@ -1464,7 +1464,7 @@ func makeGoLangTypeLength(ti typeInfo) (int64, bool) {
 	case typeVariant:
 		return 0, false
 	case typeBigBinary:
-		return 0, false
+		return int64(ti.Size), true
 	default:
 		panic(fmt.Sprintf("not implemented makeGoLangTypeLength for type %d", ti.TypeId))
 	}


Test patch:
diff --git a/queries_test.go b/queries_test.go
index 5711787..39dea63 100644
--- a/queries_test.go
+++ b/queries_test.go
@@ -2589,3 +2589,28 @@ func TestClose(t *testing.T) {
 
 	conn.Close()
 }
+
+func TestTypeSizesFromQuery(t *testing.T) {
+	conn := open(t)
+	defer conn.Close()
+	r, err := conn.Query("SELECT CAST('1' AS CHAR(10)), CAST(0x02 AS BINARY(20)), CAST(0x03 AS VARBINARY(30))")
+	if err != nil {
+		t.Fatal(err)
+	}
+	c, err := r.ColumnTypes()
+	if err != nil {
+		t.Fatal(err)
+	}
+	l, ok := c[0].Length()
+	if !ok || l != 10 {
+		t.Errorf("CHAR(10) reported as (variable, length): (%v, %d)", ok, l)
+	}
+	l, ok = c[1].Length()
+	if !ok || l != 20 {
+		t.Errorf("BINARY(20) reported as (variable, length): (%v, %d)", ok, l)
+	}
+	l, ok = c[2].Length()
+	if !ok || l != 30 {
+		t.Errorf("VARBINARY(30) reported as (variable, length): (%v, %d)", ok, l)
+	}
+}
diff --git a/types_test.go b/types_test.go
index eba8957..fa7e648 100644
--- a/types_test.go
+++ b/types_test.go
@@ -71,15 +71,18 @@ func TestMakeGoLangTypeLength(t *testing.T) {
 		typeVarLen bool
 		typeLen    int64
 		typeID     uint8
+		size       int
 	}{
-		{"typeDateTime", false, 0, typeDateTime},
-		{"typeDateTim4", false, 0, typeDateTim4},
-		{"typeBigBinary", false, 0, typeBigBinary},
+		{"typeDateTime", false, 0, typeDateTime, 0},
+		{"typeDateTim4", false, 0, typeDateTim4, 0},
+		{"typeBigVarChar", true, 2147483645, typeBigVarChar, 0xffff},
+		{"typeBigVarChar", true, 10, typeBigVarChar, 10},
+		{"typeBigBinary", true, 30, typeBigBinary, 30},
 		//TODO: Add other supported types
 	}
 
 	for _, tt := range tests {
-		n, v := makeGoLangTypeLength(typeInfo{TypeId: tt.typeID})
+		n, v := makeGoLangTypeLength(typeInfo{TypeId: tt.typeID, Size: tt.size})
 		if v != tt.typeVarLen {
 			t.Errorf("invalid type length variability returned for %s", tt.typeName)
 		}