allow users to specify bind type (jmoiron#520)

jmoiron · jmoiron · commit 00c6e74d816a · 2020-11-20T11:44:27.000-05:00
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.
diff --git a/bind.go b/bind.go
@@ -7,6 +7,7 @@ import (
 	"reflect"
 	"strconv"
 	"strings"
+	"sync"
 
 	"github.com/jmoiron/sqlx/reflectx"
 )
@@ -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
diff --git a/bind_test.go b/bind_test.go
@@ -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")
+			}
+		}
+
+	})
+}
