Skip to content

Commit

Permalink
Initialise plans using sync.Once
Browse files Browse the repository at this point in the history
Previously, plans were initialised through a racy assignment. Now they
are initialised through a sync.Once, which is more clearly race free and
also makes the intent clear (it is initialisation).

With this change it's also necessary to make the plan more pointer-like,
since if you pass a struct around by value with a lock in it, you are
going to have a bad time. This was pointed out to me by the
gometalinter.

Fixes #296.
Supercedes #153.
  • Loading branch information
pwaller committed Nov 12, 2015
1 parent 75d359a commit 7f030ea
Showing 1 changed file with 16 additions and 21 deletions.
37 changes: 16 additions & 21 deletions table_bindings.go
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@ import (
"bytes"
"fmt"
"reflect"
"sync"
)

// CustomScanner binds a database column value to a Go type
Expand Down Expand Up @@ -44,9 +45,10 @@ type bindPlan struct {
versField string
autoIncrIdx int
autoIncrFieldName string
once sync.Once
}

func (plan bindPlan) createBindInstance(elem reflect.Value, conv TypeConverter) (bindInstance, error) {
func (plan *bindPlan) createBindInstance(elem reflect.Value, conv TypeConverter) (bindInstance, error) {
bi := bindInstance{query: plan.query, autoIncrIdx: plan.autoIncrIdx, autoIncrFieldName: plan.autoIncrFieldName, versField: plan.versField}
if plan.versField != "" {
bi.existingVersion = elem.FieldByName(plan.versField).Int()
Expand Down Expand Up @@ -100,8 +102,8 @@ type bindInstance struct {
}

func (t *TableMap) bindInsert(elem reflect.Value) (bindInstance, error) {
plan := t.insertPlan
if plan.query == "" {
plan := &t.insertPlan
plan.once.Do(func() {
plan.autoIncrIdx = -1

s := bytes.Buffer{}
Expand Down Expand Up @@ -154,16 +156,14 @@ func (t *TableMap) bindInsert(elem reflect.Value) (bindInstance, error) {
s.WriteString(t.dbmap.Dialect.QuerySuffix())

plan.query = s.String()
t.insertPlan = plan
}
})

return plan.createBindInstance(elem, t.dbmap.TypeConverter)
}

func (t *TableMap) bindUpdate(elem reflect.Value) (bindInstance, error) {
plan := t.updatePlan
if plan.query == "" {

plan := &t.updatePlan
plan.once.Do(func() {
s := bytes.Buffer{}
s.WriteString(fmt.Sprintf("update %s set ", t.dbmap.Dialect.QuotedTableForQuery(t.SchemaName, t.TableName)))
x := 0
Expand Down Expand Up @@ -212,16 +212,14 @@ func (t *TableMap) bindUpdate(elem reflect.Value) (bindInstance, error) {
s.WriteString(t.dbmap.Dialect.QuerySuffix())

plan.query = s.String()
t.updatePlan = plan
}
})

return plan.createBindInstance(elem, t.dbmap.TypeConverter)
}

func (t *TableMap) bindDelete(elem reflect.Value) (bindInstance, error) {
plan := t.deletePlan
if plan.query == "" {

plan := &t.deletePlan
plan.once.Do(func() {
s := bytes.Buffer{}
s.WriteString(fmt.Sprintf("delete from %s", t.dbmap.Dialect.QuotedTableForQuery(t.SchemaName, t.TableName)))

Expand Down Expand Up @@ -258,16 +256,14 @@ func (t *TableMap) bindDelete(elem reflect.Value) (bindInstance, error) {
s.WriteString(t.dbmap.Dialect.QuerySuffix())

plan.query = s.String()
t.deletePlan = plan
}
})

return plan.createBindInstance(elem, t.dbmap.TypeConverter)
}

func (t *TableMap) bindGet() bindPlan {
plan := t.getPlan
if plan.query == "" {

func (t *TableMap) bindGet() *bindPlan {
plan := &t.getPlan
plan.once.Do(func() {
s := bytes.Buffer{}
s.WriteString("select ")

Expand Down Expand Up @@ -299,8 +295,7 @@ func (t *TableMap) bindGet() bindPlan {
s.WriteString(t.dbmap.Dialect.QuerySuffix())

plan.query = s.String()
t.getPlan = plan
}
})

return plan
}

0 comments on commit 7f030ea

Please sign in to comment.