Skip to content

Commit

Permalink
allow users to specify bind type (jmoiron#520)
Browse files Browse the repository at this point in the history
This is a thread-safe implementation of jmoiron#520, which adds a single
function `sqlx.BindDriver(driverName string, bindType int)` that allows
users of sqlx to specify the bind type for any driver.

This allows rebinding, and therefore named queries, to work in a lot
more cases:

* a new driver is released but not yet catalogued in SQLX
* users customize a driver and give it a new name

To do this, a registry had to be created so that it could be updated at
runtime.  This represents a synchronization problem, as it would be
written to and read from after compile time.  I tried two approaches:

* use `sync.Map`
* use `atomic.Value` and load/store the registry

Details within, but `sync.Map` ended up being 5x slower, and the
`atomic.Value` approach was ~2.5x slower:

```
goos: linux
goarch: amd64
pkg: github.com/jmoiron/sqlx

original:
BenchmarkBindSpeed/old-4                100000000               11.0 ns/op

sync.Map:
BenchmarkBindSpeed/new-4                24575726                50.8 ns/op

atomic.Value
BenchmarkBindSpeed/new-4                42535839                27.5 ns/op
```

Despite the slower showing, using `atomic.Value` in this way has
a correctness tradeoff.  Loads and Stores are guaranteed to be atomic,
but using Load+Store means that in cases of simultaneous writes, a write
could get lost.  This would be a _very_ difficult bug to find, so I've
opted for `sync.Map` despite the worse performance.

I think this is an acceptable trade-off as this is really unlikely to be
in any hot loops.

If this performance degredation does become a problem, another option
would be to hardcode the original registry in a switch as in the original
implementation, and only fallback on the custom registry.  I don't know
of a use case where people would want to _change_ the bindtype of a
driver whose bindtype is already known, but the flexibility seems worth
it as the performance lost doesn't seem like it's important.
  • Loading branch information
jmoiron committed Nov 20, 2020
1 parent 0794cb1 commit 00c6e74
Show file tree
Hide file tree
Showing 2 changed files with 107 additions and 12 deletions.
40 changes: 28 additions & 12 deletions bind.go
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ import (
"reflect"
"strconv"
"strings"
"sync"

"github.com/jmoiron/sqlx/reflectx"
)
Expand All @@ -20,21 +21,36 @@ const (
AT
)

var defaultBinds = map[int][]string{
DOLLAR: []string{"postgres", "pgx", "pq-timeouts", "cloudsqlpostgres", "ql"},
QUESTION: []string{"mysql", "sqlite3"},
NAMED: []string{"oci8", "ora", "goracle"},
AT: []string{"sqlserver"},
}

var binds sync.Map

func init() {
for bind, drivers := range defaultBinds {
for _, driver := range drivers {
BindDriver(driver, bind)
}
}

}

// BindType returns the bindtype for a given database given a drivername.
func BindType(driverName string) int {
switch driverName {
case "postgres", "pgx", "pq-timeouts", "cloudsqlpostgres", "ql":
return DOLLAR
case "mysql":
return QUESTION
case "sqlite3":
return QUESTION
case "oci8", "ora", "goracle", "godror":
return NAMED
case "sqlserver":
return AT
itype, ok := binds.Load(driverName)
if !ok {
return UNKNOWN
}
return UNKNOWN
return itype.(int)
}

// BindDriver sets the BindType for driverName to bindType.
func BindDriver(driverName string, bindType int) {
binds.Store(driverName, bindType)
}

// FIXME: this should be able to be tolerant of escaped ?'s in queries without
Expand Down
79 changes: 79 additions & 0 deletions bind_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,79 @@
package sqlx

import (
"math/rand"
"testing"
)

func oldBindType(driverName string) int {
switch driverName {
case "postgres", "pgx", "pq-timeouts", "cloudsqlpostgres", "ql":
return DOLLAR
case "mysql":
return QUESTION
case "sqlite3":
return QUESTION
case "oci8", "ora", "goracle", "godror":
return NAMED
case "sqlserver":
return AT
}
return UNKNOWN
}

/*
sync.Map implementation:
goos: linux
goarch: amd64
pkg: github.com/jmoiron/sqlx
BenchmarkBindSpeed/old-4 100000000 11.0 ns/op
BenchmarkBindSpeed/new-4 24575726 50.8 ns/op
async.Value map implementation:
goos: linux
goarch: amd64
pkg: github.com/jmoiron/sqlx
BenchmarkBindSpeed/old-4 100000000 11.0 ns/op
BenchmarkBindSpeed/new-4 42535839 27.5 ns/op
*/

func BenchmarkBindSpeed(b *testing.B) {
testDrivers := []string{
"postgres", "pgx", "mysql", "sqlite3", "ora", "sqlserver",
}

b.Run("old", func(b *testing.B) {
b.StopTimer()
var seq []int
for i := 0; i < b.N; i++ {
seq = append(seq, rand.Intn(len(testDrivers)))
}
b.StartTimer()
for i := 0; i < b.N; i++ {
s := oldBindType(testDrivers[seq[i]])
if s == UNKNOWN {
b.Error("unknown driver")
}
}

})

b.Run("new", func(b *testing.B) {
b.StopTimer()
var seq []int
for i := 0; i < b.N; i++ {
seq = append(seq, rand.Intn(len(testDrivers)))
}
b.StartTimer()
for i := 0; i < b.N; i++ {
s := BindType(testDrivers[seq[i]])
if s == UNKNOWN {
b.Error("unknown driver")
}
}

})
}

0 comments on commit 00c6e74

Please sign in to comment.