From 7f030ea5b94b7ca756c19c24c58dd2a8b6b275d2 Mon Sep 17 00:00:00 2001
From: Peter Waller <p@pwaller.net>
Date: Wed, 11 Nov 2015 18:41:26 +0000
Subject: [PATCH] Initialise plans using sync.Once

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 go-gorp/gorp#296.
Supercedes go-gorp/gorp#153.
---
 table_bindings.go | 37 ++++++++++++++++---------------------
 1 file changed, 16 insertions(+), 21 deletions(-)

diff --git a/table_bindings.go b/table_bindings.go
index c343206c..25a94af4 100644
--- a/table_bindings.go
+++ b/table_bindings.go
@@ -15,6 +15,7 @@ import (
 	"bytes"
 	"fmt"
 	"reflect"
+	"sync"
 )
 
 // CustomScanner binds a database column value to a Go type
@@ -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()
@@ -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{}
@@ -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
@@ -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)))
 
@@ -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 ")
 
@@ -299,8 +295,7 @@ func (t *TableMap) bindGet() bindPlan {
 		s.WriteString(t.dbmap.Dialect.QuerySuffix())
 
 		plan.query = s.String()
-		t.getPlan = plan
-	}
+	})
 
 	return plan
 }