denisenkom / go-mssqldb

Microsoft SQL server driver written in go language
BSD 3-Clause "New" or "Revised" License
1.82k stars 495 forks source link

fix size reporting for typeBigBinary #686

Closed shueybubbles closed 3 years ago

shueybubbles commented 3 years ago

binary(x) is a variable size type, not fixed size. #685

shueybubbles commented 3 years ago

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:

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 commented 3 years ago

should testLogger methods put a lock around the l.t.Log calls? EDIT:a lock won't help since the tests themselves don't use testLogger.

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 commented 3 years ago

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