From a2bcdb311acb8aa4e268a2368c1b7ec2bd8f9d9a Mon Sep 17 00:00:00 2001
From: Sugu Sougoumarane <ssougou@gmail.com>
Date: Sat, 29 Apr 2017 10:58:44 -0700
Subject: [PATCH] insertID: address review comments

---
 go/vt/vtgate/engine/route.go    | 30 ++++++++++++++++++++----------
 go/vt/vtgate/router_dml_test.go |  4 ++--
 2 files changed, 22 insertions(+), 12 deletions(-)

diff --git a/go/vt/vtgate/engine/route.go b/go/vt/vtgate/engine/route.go
index d4d5ce669de..1503cbb2c6f 100644
--- a/go/vt/vtgate/engine/route.go
+++ b/go/vt/vtgate/engine/route.go
@@ -441,7 +441,7 @@ func (route *Route) execDeleteEqual(vcursor VCursor, queryConstruct *queryinfo.Q
 }
 
 func (route *Route) execInsertUnsharded(vcursor VCursor, queryConstruct *queryinfo.QueryConstruct) (*sqltypes.Result, error) {
-	insertid, err := route.handleGenerate(vcursor, queryConstruct)
+	insertID, err := route.handleGenerate(vcursor, queryConstruct)
 	if err != nil {
 		return nil, fmt.Errorf("execInsertUnsharded: %v", err)
 	}
@@ -456,14 +456,18 @@ func (route *Route) execInsertUnsharded(vcursor VCursor, queryConstruct *queryin
 		return nil, fmt.Errorf("execInsertUnsharded: %v", err)
 	}
 
-	if insertid != 0 {
-		result.InsertID = uint64(insertid)
+	// If handleGenerate generated new values, it supercedes
+	// any ids that MySQL might have generated. If both generated
+	// values, we don't return an error because this behavior
+	// is required to support migration.
+	if insertID != 0 {
+		result.InsertID = uint64(insertID)
 	}
 	return result, nil
 }
 
 func (route *Route) execInsertSharded(vcursor VCursor, queryConstruct *queryinfo.QueryConstruct) (*sqltypes.Result, error) {
-	insertid, err := route.handleGenerate(vcursor, queryConstruct)
+	insertID, err := route.handleGenerate(vcursor, queryConstruct)
 	if err != nil {
 		return nil, fmt.Errorf("execInsertSharded: %v", err)
 	}
@@ -478,8 +482,12 @@ func (route *Route) execInsertSharded(vcursor VCursor, queryConstruct *queryinfo
 		return nil, fmt.Errorf("execInsertSharded: %v", err)
 	}
 
-	if insertid != 0 {
-		result.InsertID = uint64(insertid)
+	// If handleGenerate generated new values, it supercedes
+	// any ids that MySQL might have generated. If both generated
+	// values, we don't return an error because this behavior
+	// is required to support migration.
+	if insertID != 0 {
+		result.InsertID = uint64(insertID)
 	}
 	return result, nil
 }
@@ -693,7 +701,9 @@ func (route *Route) deleteVindexEntries(vcursor VCursor, queryConstruct *queryin
 	return nil
 }
 
-func (route *Route) handleGenerate(vcursor VCursor, queryConstruct *queryinfo.QueryConstruct) (insertid int64, err error) {
+// handleGenerate generates new values using a sequence if necessary.
+// If no value was generated, it returns 0.
+func (route *Route) handleGenerate(vcursor VCursor, queryConstruct *queryinfo.QueryConstruct) (insertID int64, err error) {
 	if route.Generate == nil {
 		return 0, nil
 	}
@@ -733,12 +743,12 @@ func (route *Route) handleGenerate(vcursor VCursor, queryConstruct *queryinfo.Qu
 		}
 		// If no rows are returned, it's an internal error, and the code
 		// must panic, which will caught and reported.
-		insertid, err = qr.Rows[0][0].ParseInt64()
+		insertID, err = qr.Rows[0][0].ParseInt64()
 		if err != nil {
 			return 0, err
 		}
 	}
-	cur := insertid
+	cur := insertID
 	for i, v := range resolved {
 		if v != nil {
 			queryConstruct.BindVars[SeqVarName+strconv.Itoa(i)] = v
@@ -747,7 +757,7 @@ func (route *Route) handleGenerate(vcursor VCursor, queryConstruct *queryinfo.Qu
 			cur++
 		}
 	}
-	return insertid, nil
+	return insertID, nil
 }
 
 func (route *Route) handlePrimary(vcursor VCursor, vindexKeys []interface{}, colVindex *vindexes.ColumnVindex, bv map[string]interface{}) (keyspaceIDs [][]byte, err error) {
diff --git a/go/vt/vtgate/router_dml_test.go b/go/vt/vtgate/router_dml_test.go
index 695b652a3f1..e13883c98de 100644
--- a/go/vt/vtgate/router_dml_test.go
+++ b/go/vt/vtgate/router_dml_test.go
@@ -460,7 +460,7 @@ func TestInsertAutoincSharded(t *testing.T) {
 		Rows: [][]sqltypes.Value{{
 			sqltypes.MakeTrusted(sqltypes.Int64, []byte("1")),
 		}},
-		RowsAffected: 2,
+		RowsAffected: 1,
 		InsertID:     2,
 	}
 	sbc.SetResults([]*sqltypes.Result{wantResult})
@@ -516,7 +516,7 @@ func TestInsertAutoincUnsharded(t *testing.T) {
 		Rows: [][]sqltypes.Value{{
 			sqltypes.MakeTrusted(sqltypes.Int64, []byte("1")),
 		}},
-		RowsAffected: 2,
+		RowsAffected: 1,
 		InsertID:     2,
 	}
 	sbclookup.SetResults([]*sqltypes.Result{wantResult})