Pass RPCErrors by pointer instead of value

2015-06-04 20:26:00 -07:00 · 2015-06-04 20:26:00 -07:00 · d339c32c49
--- a/go/mysql/proto/bson_test.go
+++ b/go/mysql/proto/bson_test.go
@ -34,7 +34,7 @@ func TestQueryResult(t *testing.T) {
 		Rows: [][]sqltypes.Value{
 			{{sqltypes.Numeric("1")}, {sqltypes.String("aa")}},
 		},
-		Err: RPCError{1000, "failed due to err"},
+		Err: &RPCError{1000, "failed due to err"},
 	}
 	encoded, err := bson.Marshal(&custom)
 	if err != nil {
@ -50,8 +50,12 @@ func TestQueryResult(t *testing.T) {
 	if err != nil {
 		t.Error(err)
 	}
-	if custom.Err != unmarshalled.Err {
+	if unmarshalled.Err == nil {
 		t.Errorf("want %#v, got %#v", custom.Err, unmarshalled.Err)
+	} else {
+		if *custom.Err != *unmarshalled.Err {
+			t.Errorf("want %#v, got %#v", custom.Err, unmarshalled.Err)
+		}
 	}
 	if custom.RowsAffected != unmarshalled.RowsAffected {
 		t.Errorf("want %v, got %#v", custom.RowsAffected, unmarshalled.RowsAffected)
--- a/go/mysql/proto/qr_bson_test.go
+++ b/go/mysql/proto/qr_bson_test.go
@ -20,7 +20,7 @@ var testcases = []TestCase{
 	// Empty
 	{
 		qr:      QueryResult{},
-		encoded: "k\x00\x00\x00\x04Fields\x00\x05\x00\x00\x00\x00?RowsAffected\x00\x00\x00\x00\x00\x00\x00\x00\x00?InsertId\x00\x00\x00\x00\x00\x00\x00\x00\x00\x04Rows\x00\x05\x00\x00\x00\x00\x03Err\x00!\x00\x00\x00\x12Code\x00\x00\x00\x00\x00\x00\x00\x00\x00\x05Message\x00\x00\x00\x00\x00\x00\x00\x00",
+		encoded: "J\x00\x00\x00\x04Fields\x00\x05\x00\x00\x00\x00?RowsAffected\x00\x00\x00\x00\x00\x00\x00\x00\x00?InsertId\x00\x00\x00\x00\x00\x00\x00\x00\x00\x04Rows\x00\x05\x00\x00\x00\x00\nErr\x00\x00",
 	},
 	// Only fields set
 	{
@ -29,7 +29,7 @@ var testcases = []TestCase{
 				{Name: "foo", Type: 1},
 			},
 		},
-		encoded: "\x9e\x00\x00\x00\x04Fields\x008\x00\x00\x00\x030\x000\x00\x00\x00\x05Name\x00\x03\x00\x00\x00\x00foo\x12Type\x00\x01\x00\x00\x00\x00\x00\x00\x00\x12Flags\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00?RowsAffected\x00\x00\x00\x00\x00\x00\x00\x00\x00?InsertId\x00\x00\x00\x00\x00\x00\x00\x00\x00\x04Rows\x00\x05\x00\x00\x00\x00\x03Err\x00!\x00\x00\x00\x12Code\x00\x00\x00\x00\x00\x00\x00\x00\x00\x05Message\x00\x00\x00\x00\x00\x00\x00\x00",
+		encoded: "}\x00\x00\x00\x04Fields\x008\x00\x00\x00\x030\x000\x00\x00\x00\x05Name\x00\x03\x00\x00\x00\x00foo\x12Type\x00\x01\x00\x00\x00\x00\x00\x00\x00\x12Flags\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00?RowsAffected\x00\x00\x00\x00\x00\x00\x00\x00\x00?InsertId\x00\x00\x00\x00\x00\x00\x00\x00\x00\x04Rows\x00\x05\x00\x00\x00\x00\nErr\x00\x00",
 	},
 	// Only rows, no fields
 	{
@ -38,7 +38,7 @@ var testcases = []TestCase{
 				{sqltypes.MakeString([]byte("abcd")), sqltypes.MakeNumeric([]byte("1234")), sqltypes.MakeFractional([]byte("1.234"))},
 			},
 		},
-		encoded: "\x98\x00\x00\x00\x04Fields\x00\x05\x00\x00\x00\x00?RowsAffected\x00\x00\x00\x00\x00\x00\x00\x00\x00?InsertId\x00\x00\x00\x00\x00\x00\x00\x00\x00\x04Rows\x002\x00\x00\x00\x040\x00*\x00\x00\x00\x050\x00\x04\x00\x00\x00\x00abcd\x051\x00\x04\x00\x00\x00\x001234\x052\x00\x05\x00\x00\x00\x001.234\x00\x00\x03Err\x00!\x00\x00\x00\x12Code\x00\x00\x00\x00\x00\x00\x00\x00\x00\x05Message\x00\x00\x00\x00\x00\x00\x00\x00",
+		encoded: "w\x00\x00\x00\x04Fields\x00\x05\x00\x00\x00\x00?RowsAffected\x00\x00\x00\x00\x00\x00\x00\x00\x00?InsertId\x00\x00\x00\x00\x00\x00\x00\x00\x00\x04Rows\x002\x00\x00\x00\x040\x00*\x00\x00\x00\x050\x00\x04\x00\x00\x00\x00abcd\x051\x00\x04\x00\x00\x00\x001234\x052\x00\x05\x00\x00\x00\x001.234\x00\x00\nErr\x00\x00",
 	},
 	// one row and one field
 	{
@ -66,6 +66,13 @@ var testcases = []TestCase{
 		},
 		encoded: "",
 	},
+	// With an error
+	{
+		qr: QueryResult{
+			Err: &RPCError{1000, "generic error"},
+		},
+		encoded: "x\x00\x00\x00\x04Fields\x00\x05\x00\x00\x00\x00?RowsAffected\x00\x00\x00\x00\x00\x00\x00\x00\x00?InsertId\x00\x00\x00\x00\x00\x00\x00\x00\x00\x04Rows\x00\x05\x00\x00\x00\x00\x03Err\x00.\x00\x00\x00\x12Code\x00\xe8\x03\x00\x00\x00\x00\x00\x00\x05Message\x00\r\x00\x00\x00\x00generic error\x00\x00",
+	},
 }

 func TestRun(t *testing.T) {
--- a/go/mysql/proto/query_result_bson.go
+++ b/go/mysql/proto/query_result_bson.go
@ -48,7 +48,12 @@ func (queryResult *QueryResult) MarshalBson(buf *bytes2.ChunkedWriter, key strin
 		}
 		lenWriter.Close()
 	}
-	queryResult.Err.MarshalBson(buf, "Err")
+	// *RPCError
+	if queryResult.Err == nil {
+		bson.EncodePrefix(buf, bson.Null, "Err")
+	} else {
+		(*queryResult.Err).MarshalBson(buf, "Err")
+	}

 	lenWriter.Close()
 }
@ -115,7 +120,11 @@ func (queryResult *QueryResult) UnmarshalBson(buf *bytes.Buffer, kind byte) {
 				}
 			}
 		case "Err":
-			queryResult.Err.UnmarshalBson(buf, kind)
+			// *RPCError
+			if kind != bson.Null {
+				queryResult.Err = new(RPCError)
+				(*queryResult.Err).UnmarshalBson(buf, kind)
+			}
 		default:
 			bson.Skip(buf, kind)
 		}
--- a/go/mysql/proto/structs.go
+++ b/go/mysql/proto/structs.go
@ -83,7 +83,7 @@ type QueryResult struct {
 	RowsAffected uint64
 	InsertId     uint64
 	Rows         [][]sqltypes.Value
-	Err          RPCError
+	Err          *RPCError
 }

 //go:generate bsongen -file $GOFILE -type QueryResult -o query_result_bson.go
--- a/go/vt/tabletserver/proto/session_info_bson.go
+++ b/go/vt/tabletserver/proto/session_info_bson.go
@ -0,0 +1,60 @@
+// Copyright 2012, Google Inc. All rights reserved.
+// Use of this source code is governed by a BSD-style
+// license that can be found in the LICENSE file.
+
+package proto
+
+// DO NOT EDIT.
+// FILE GENERATED BY BSONGEN.
+
+import (
+	"bytes"
+
+	"github.com/youtube/vitess/go/bson"
+	"github.com/youtube/vitess/go/bytes2"
+	mproto "github.com/youtube/vitess/go/mysql/proto"
+)
+
+// MarshalBson bson-encodes SessionInfo.
+func (sessionInfo *SessionInfo) MarshalBson(buf *bytes2.ChunkedWriter, key string) {
+	bson.EncodeOptionalPrefix(buf, bson.Object, key)
+	lenWriter := bson.NewLenWriter(buf)
+
+	bson.EncodeInt64(buf, "SessionId", sessionInfo.SessionId)
+	// *mproto.RPCError
+	if sessionInfo.Err == nil {
+		bson.EncodePrefix(buf, bson.Null, "Err")
+	} else {
+		(*sessionInfo.Err).MarshalBson(buf, "Err")
+	}
+
+	lenWriter.Close()
+}
+
+// UnmarshalBson bson-decodes into SessionInfo.
+func (sessionInfo *SessionInfo) UnmarshalBson(buf *bytes.Buffer, kind byte) {
+	switch kind {
+	case bson.EOO, bson.Object:
+		// valid
+	case bson.Null:
+		return
+	default:
+		panic(bson.NewBsonError("unexpected kind %v for SessionInfo", kind))
+	}
+	bson.Next(buf, 4)
+
+	for kind := bson.NextByte(buf); kind != bson.EOO; kind = bson.NextByte(buf) {
+		switch bson.ReadCString(buf) {
+		case "SessionId":
+			sessionInfo.SessionId = bson.DecodeInt64(buf, kind)
+		case "Err":
+			// *mproto.RPCError
+			if kind != bson.Null {
+				sessionInfo.Err = new(mproto.RPCError)
+				(*sessionInfo.Err).UnmarshalBson(buf, kind)
+			}
+		default:
+			bson.Skip(buf, kind)
+		}
+	}
+}
--- a/go/vt/tabletserver/proto/structs.go
+++ b/go/vt/tabletserver/proto/structs.go
@ -22,8 +22,11 @@ type SessionParams struct {
 // session_id in the Session object for any subsequent call.
 type SessionInfo struct {
 	SessionId int64
+	Err       *mproto.RPCError
 }

+//go:generate bsongen -file $GOFILE -type SessionInfo -o session_info_bson.go
+
 // Query is the payload to Execute.
 type Query struct {
 	Sql           string
@ -98,8 +101,11 @@ type Session struct {
 // the transaction.
 type TransactionInfo struct {
 	TransactionId int64
+	Err           *mproto.RPCError
 }

+//go:generate bsongen -file $GOFILE -type TransactionInfo -o transaction_info_bson.go
+
 // SplitQueryRequest represents a request to split a Query into queries that
 // each return a subset of the original query.
 // SplitColumn: preferred column to split. Server will pick a random PK column
--- a/go/vt/tabletserver/proto/transaction_info_bson.go
+++ b/go/vt/tabletserver/proto/transaction_info_bson.go
@ -0,0 +1,60 @@
+// Copyright 2012, Google Inc. All rights reserved.
+// Use of this source code is governed by a BSD-style
+// license that can be found in the LICENSE file.
+
+package proto
+
+// DO NOT EDIT.
+// FILE GENERATED BY BSONGEN.
+
+import (
+	"bytes"
+
+	"github.com/youtube/vitess/go/bson"
+	"github.com/youtube/vitess/go/bytes2"
+	mproto "github.com/youtube/vitess/go/mysql/proto"
+)
+
+// MarshalBson bson-encodes TransactionInfo.
+func (transactionInfo *TransactionInfo) MarshalBson(buf *bytes2.ChunkedWriter, key string) {
+	bson.EncodeOptionalPrefix(buf, bson.Object, key)
+	lenWriter := bson.NewLenWriter(buf)
+
+	bson.EncodeInt64(buf, "TransactionId", transactionInfo.TransactionId)
+	// *mproto.RPCError
+	if transactionInfo.Err == nil {
+		bson.EncodePrefix(buf, bson.Null, "Err")
+	} else {
+		(*transactionInfo.Err).MarshalBson(buf, "Err")
+	}
+
+	lenWriter.Close()
+}
+
+// UnmarshalBson bson-decodes into TransactionInfo.
+func (transactionInfo *TransactionInfo) UnmarshalBson(buf *bytes.Buffer, kind byte) {
+	switch kind {
+	case bson.EOO, bson.Object:
+		// valid
+	case bson.Null:
+		return
+	default:
+		panic(bson.NewBsonError("unexpected kind %v for TransactionInfo", kind))
+	}
+	bson.Next(buf, 4)
+
+	for kind := bson.NextByte(buf); kind != bson.EOO; kind = bson.NextByte(buf) {
+		switch bson.ReadCString(buf) {
+		case "TransactionId":
+			transactionInfo.TransactionId = bson.DecodeInt64(buf, kind)
+		case "Err":
+			// *mproto.RPCError
+			if kind != bson.Null {
+				transactionInfo.Err = new(mproto.RPCError)
+				(*transactionInfo.Err).UnmarshalBson(buf, kind)
+			}
+		default:
+			bson.Skip(buf, kind)
+		}
+	}
+}
--- a/go/vt/tabletserver/tablet_error.go
+++ b/go/vt/tabletserver/tablet_error.go
@ -218,10 +218,10 @@ func AddTabletErrorToQueryResult(err error, reply *mproto.QueryResult) {
 	if err == nil {
 		return
 	}
-	var rpcErr mproto.RPCError
+	var rpcErr *mproto.RPCError
 	terr, ok := err.(*TabletError)
 	if ok {
-		rpcErr = mproto.RPCError{
+		rpcErr = &mproto.RPCError{
 			// Transform TabletError code to VitessError code
 			Code: int64(terr.ErrorType) + vterrors.TabletError,
 			// Make sure the the VitessError message is identical to the TabletError
@ -230,7 +230,7 @@ func AddTabletErrorToQueryResult(err error, reply *mproto.QueryResult) {
 			Message: terr.Error(),
 		}
 	} else {
-		rpcErr = mproto.RPCError{
+		rpcErr = &mproto.RPCError{
 			Code:    vterrors.UnknownTabletError,
 			Message: err.Error(),
 		}
--- a/go/vt/tabletserver/tabletconntest/tabletconntest.go
+++ b/go/vt/tabletserver/tabletconntest/tabletconntest.go
@ -213,7 +213,7 @@ var executeQueryResult = mproto.QueryResult{
 }

 var executeQueryResultError = mproto.QueryResult{
-	Err: mproto.RPCError{
+	Err: &mproto.RPCError{
 		Code:    1000,
 		Message: "succeeded despite err",
 	},
--- a/go/vt/vterrors/vterrors.go
+++ b/go/vt/vterrors/vterrors.go
@ -50,10 +50,10 @@ func (e *VitessError) AsString() string {
 	return fmt.Sprintf("Code: %v, err: %v", e.Code, e.err)
 }

-// FromRPCError recovers a VitessError from an RPCError (which is how VitessErrors
+// FromRPCError recovers a VitessError from a *RPCError (which is how VitessErrors
 // are transmitted across RPC boundaries).
-func FromRPCError(rpcErr mproto.RPCError) error {
-	if rpcErr.Code == 0 && rpcErr.Message == "" {
+func FromRPCError(rpcErr *mproto.RPCError) error {
+	if rpcErr == nil {
 		return nil
 	}
 	return &VitessError{
--- a/go/vt/vtgate/proto/vtgate_proto_test.go
+++ b/go/vt/vtgate/proto/vtgate_proto_test.go
@ -165,7 +165,7 @@ func TestQueryShard(t *testing.T) {
 func TestQueryResult(t *testing.T) {
 	// We can't do the reflection test because bson
 	// doesn't do it correctly for embedded fields.
-	want := "\xb1\x01\x00\x00\x03Result\x00\xba\x00\x00\x00\x04Fields\x009\x00\x00\x00\x030\x001\x00\x00\x00\x05Name\x00\x04\x00\x00\x00\x00name\x12Type\x00\x01\x00\x00\x00\x00\x00\x00\x00\x12Flags\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00?RowsAffected\x00\x02\x00\x00\x00\x00\x00\x00\x00?InsertId\x00\x03\x00\x00\x00\x00\x00\x00\x00\x04Rows\x00 \x00\x00\x00\x040\x00\x18\x00\x00\x00\x050\x00\x01\x00\x00\x00\x001\x051\x00\x02\x00\x00\x00\x00aa\x00\x00\x03Err\x00!\x00\x00\x00\x12Code\x00\x00\x00\x00\x00\x00\x00\x00\x00\x05Message\x00\x00\x00\x00\x00\x00\x00\x00\x03Session\x00\xd0\x00\x00\x00\bInTransaction\x00\x01\x04ShardSessions\x00\xac\x00\x00\x00\x030\x00Q\x00\x00\x00\x05Keyspace\x00\x01\x00\x00\x00\x00a\x05Shard\x00\x01\x00\x00\x00\x000\x05TabletType\x00\a\x00\x00\x00\x00replica\x12TransactionId\x00\x01\x00\x00\x00\x00\x00\x00\x00\x00\x031\x00P\x00\x00\x00\x05Keyspace\x00\x01\x00\x00\x00\x00b\x05Shard\x00\x01\x00\x00\x00\x001\x05TabletType\x00\x06\x00\x00\x00\x00master\x12TransactionId\x00\x02\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x05Error\x00\x05\x00\x00\x00\x00error\x00"
+	want := "\x90\x01\x00\x00\x03Result\x00\x99\x00\x00\x00\x04Fields\x009\x00\x00\x00\x030\x001\x00\x00\x00\x05Name\x00\x04\x00\x00\x00\x00name\x12Type\x00\x01\x00\x00\x00\x00\x00\x00\x00\x12Flags\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00?RowsAffected\x00\x02\x00\x00\x00\x00\x00\x00\x00?InsertId\x00\x03\x00\x00\x00\x00\x00\x00\x00\x04Rows\x00 \x00\x00\x00\x040\x00\x18\x00\x00\x00\x050\x00\x01\x00\x00\x00\x001\x051\x00\x02\x00\x00\x00\x00aa\x00\x00\nErr\x00\x00\x03Session\x00\xd0\x00\x00\x00\bInTransaction\x00\x01\x04ShardSessions\x00\xac\x00\x00\x00\x030\x00Q\x00\x00\x00\x05Keyspace\x00\x01\x00\x00\x00\x00a\x05Shard\x00\x01\x00\x00\x00\x000\x05TabletType\x00\a\x00\x00\x00\x00replica\x12TransactionId\x00\x01\x00\x00\x00\x00\x00\x00\x00\x00\x031\x00P\x00\x00\x00\x05Keyspace\x00\x01\x00\x00\x00\x00b\x05Shard\x00\x01\x00\x00\x00\x001\x05TabletType\x00\x06\x00\x00\x00\x00master\x12TransactionId\x00\x02\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x05Error\x00\x05\x00\x00\x00\x00error\x00"

 	custom := QueryResult{
 		Result: &mproto.QueryResult{