From c4bf07399679fe82829d5d8ce788602fd2cef825 Mon Sep 17 00:00:00 2001
From: qiuyesuifeng <cuiqiu.bupt@gmail.com>
Date: Fri, 2 Oct 2015 20:25:23 +0800
Subject: [PATCH 1/2] util: do some cleanup.

---
 util/arena/arena.go          | 24 ++++++++++++------------
 util/charset/charset.go      |  2 +-
 util/codec/bytes.go          |  2 ++
 util/mock/context.go         |  4 ++--
 util/mock/mocks/plan.go      |  6 +++---
 util/mock/mocks/recordset.go | 12 ++++++------
 util/mock/mocks/statement.go |  2 +-
 util/prefix_helper.go        | 15 ++++++++-------
 util/types/compare.go        | 31 +++++++++++++++----------------
 util/types/convert.go        | 24 ++++++++++++------------
 util/types/etc.go            |  8 +++-----
 util/types/field_type.go     | 16 +++++++++-------
 util/types/overflow.go       |  4 ++--
 13 files changed, 76 insertions(+), 74 deletions(-)

diff --git a/util/arena/arena.go b/util/arena/arena.go
index 61e6bc298568c..94729b19a429a 100644
--- a/util/arena/arena.go
+++ b/util/arena/arena.go
@@ -13,21 +13,21 @@
 
 package arena
 
-// Allocator pre-allocates memory to
-// reduce memory allocation cost.
+// Allocator pre-allocates memory to reduce memory allocation cost.
 // It is not thread-safe.
 type Allocator interface {
-	// Alloc allocates memory with 0 len and n cap.
+	// Alloc allocates memory with 0 len and capacity cap.
 	Alloc(capacity int) []byte
 
 	// AllocWithLen allocates memory with length and capacity.
 	AllocWithLen(length int, capacity int) []byte
 
-	// Reset resets arena offset, make sure all the allocated memory are not used any more.
+	// Reset resets arena offset.
+	// Make sure all the allocated memory are not used any more.
 	Reset()
 }
 
-// SimpleAllocator is a simple implementation of ArenaAllocator
+// SimpleAllocator is a simple implementation of ArenaAllocator.
 type SimpleAllocator struct {
 	arena []byte
 	off   int
@@ -36,8 +36,8 @@ type SimpleAllocator struct {
 type stdAllocator struct {
 }
 
-func (a *stdAllocator) Alloc(n int) []byte {
-	return make([]byte, 0, n)
+func (a *stdAllocator) Alloc(capacity int) []byte {
+	return make([]byte, 0, capacity)
 }
 
 func (a *stdAllocator) AllocWithLen(length int, capacity int) []byte {
@@ -58,14 +58,14 @@ func NewAllocator(capacity int) *SimpleAllocator {
 }
 
 // Alloc implements Allocator.AllocBytes interface.
-func (s *SimpleAllocator) Alloc(capcity int) []byte {
-	if s.off+capcity < cap(s.arena) {
-		slice := s.arena[s.off:s.off : s.off+capcity]
-		s.off += capcity
+func (s *SimpleAllocator) Alloc(capacity int) []byte {
+	if s.off+capacity < cap(s.arena) {
+		slice := s.arena[s.off:s.off : s.off+capacity]
+		s.off += capacity
 		return slice
 	}
 
-	return make([]byte, 0, capcity)
+	return make([]byte, 0, capacity)
 }
 
 // AllocWithLen implements Allocator.AllocWithLen interface.
diff --git a/util/charset/charset.go b/util/charset/charset.go
index b7ceeb2da4279..cb199c849d975 100644
--- a/util/charset/charset.go
+++ b/util/charset/charset.go
@@ -34,7 +34,7 @@ type Collation struct {
 
 var charsets = make(map[string]*Charset)
 
-// All the supported charsets should in the following table.
+// All the supported charsets should be in the following table.
 var charsetInfos = []*Charset{
 	{"utf8", nil, make(map[string]*Collation), "UTF-8 Unicode", 3},
 	{"latin1", nil, make(map[string]*Collation), "cp1252 West European", 1},
diff --git a/util/codec/bytes.go b/util/codec/bytes.go
index 11c8bdf12e3a7..30dc5a2f5d05c 100644
--- a/util/codec/bytes.go
+++ b/util/codec/bytes.go
@@ -99,6 +99,8 @@ func decodeBytes(b []byte, escapeFirst byte, escape byte, term byte) ([]byte, []
 		}
 
 		if b[i+1] == term {
+			// For `var r []byte`, `b := []byte{}`, `r = append(r, b...)`,
+			// here r is equals to nil, so it is better to distinguish it.
 			if r == nil {
 				r = b[:i]
 			} else {
diff --git a/util/mock/context.go b/util/mock/context.go
index a564f8570223f..3f297b240482d 100644
--- a/util/mock/context.go
+++ b/util/mock/context.go
@@ -23,7 +23,7 @@ import (
 
 var _ context.Context = (*Context)(nil)
 
-// Context is a mock context.Context for test.
+// Context represents mocked context.Context.
 type Context struct {
 	values map[fmt.Stringer]interface{}
 }
@@ -54,7 +54,7 @@ func (c *Context) FinishTxn(rollback bool) error {
 	return nil
 }
 
-// NewContext creates a mock context.Context.
+// NewContext creates a new mocked context.Context.
 func NewContext() context.Context {
 	return &Context{
 		values: make(map[fmt.Stringer]interface{}),
diff --git a/util/mock/mocks/plan.go b/util/mock/mocks/plan.go
index 6e4b1d2709922..d326deee53ec3 100644
--- a/util/mock/mocks/plan.go
+++ b/util/mock/mocks/plan.go
@@ -21,14 +21,14 @@ import (
 	"github.com/pingcap/tidb/util/format"
 )
 
-// Plan mocks a plan.Plan instance
+// Plan represents mocked plan.Plan.
 type Plan struct {
 	rset   *Recordset
 	rows   [][]interface{}
 	cursor int
 }
 
-// NewPlan creates a new MockPlan
+// NewPlan creates a new mocked plan.Plan.
 func NewPlan(rset *Recordset) *Plan {
 	return &Plan{rset: rset}
 }
@@ -60,7 +60,7 @@ func (p *Plan) Next(ctx context.Context) (row *plan.Row, err error) {
 	return
 }
 
-// Close implements plan.Plan Close interface{}
+// Close implements plan.Plan Close interface.
 func (p *Plan) Close() error {
 	p.cursor = 0
 	return nil
diff --git a/util/mock/mocks/recordset.go b/util/mock/mocks/recordset.go
index acfa9b4bb987c..b668c8d8c9eea 100644
--- a/util/mock/mocks/recordset.go
+++ b/util/mock/mocks/recordset.go
@@ -19,7 +19,7 @@ import (
 	"github.com/pingcap/tidb/plan"
 )
 
-// Recordset represents mock Recordsets
+// Recordset represents mocked rset.Recordset.
 type Recordset struct {
 	rows   [][]interface{}
 	fields []string
@@ -27,7 +27,7 @@ type Recordset struct {
 	cursor int
 }
 
-// NewRecordset creates a new mocked Recordset.
+// NewRecordset creates a new mocked rset.Recordset.
 func NewRecordset(rows [][]interface{}, fields []string, offset int) *Recordset {
 	return &Recordset{
 		rows:   rows,
@@ -58,12 +58,12 @@ func (r *Recordset) Fields() ([]*field.ResultField, error) {
 	return ret[:r.offset], nil
 }
 
-// FirstRow implements rset.Recordset.
+// FirstRow implements rset.Recordset FirstRow interface.
 func (r *Recordset) FirstRow() ([]interface{}, error) {
 	return r.rows[0], nil
 }
 
-// Rows implements rset.Recordset.
+// Rows implements rset.Recordset Rows interface.
 func (r *Recordset) Rows(limit, offset int) ([][]interface{}, error) {
 	var ret [][]interface{}
 	for _, row := range r.rows {
@@ -78,7 +78,7 @@ func (r *Recordset) SetFieldOffset(offset int) {
 	r.offset = offset
 }
 
-// Next implements rset.Recordset.
+// Next implements rset.Recordset Next interface.
 func (r *Recordset) Next() (row *plan.Row, err error) {
 	if r.cursor == len(r.rows) {
 		return
@@ -88,7 +88,7 @@ func (r *Recordset) Next() (row *plan.Row, err error) {
 	return
 }
 
-// Close implements rset.Recordset.
+// Close implements rset.Recordset Close interface.
 func (r *Recordset) Close() error {
 	r.cursor = 0
 	return nil
diff --git a/util/mock/mocks/statement.go b/util/mock/mocks/statement.go
index ed051c7e05260..9c97d950550b7 100644
--- a/util/mock/mocks/statement.go
+++ b/util/mock/mocks/statement.go
@@ -21,7 +21,7 @@ import (
 	"github.com/pingcap/tidb/util/format"
 )
 
-// Statement represents a mocked Statement.
+// Statement represents mocked stmt.Statement.
 type Statement struct {
 	text string
 	Rset *Recordset
diff --git a/util/prefix_helper.go b/util/prefix_helper.go
index 97437f24f008c..12aa114962dfc 100644
--- a/util/prefix_helper.go
+++ b/util/prefix_helper.go
@@ -38,12 +38,13 @@ func hasPrefix(prefix []byte) kv.FnKeyCmp {
 func ScanMetaWithPrefix(txn kv.Transaction, prefix string, filter func([]byte, []byte) bool) error {
 	iter, err := txn.Seek([]byte(prefix), hasPrefix([]byte(prefix)))
 	if err != nil {
-		return err
+		return errors.Trace(err)
 	}
 	defer iter.Close()
+
 	for {
 		if err != nil {
-			return err
+			return errors.Trace(err)
 		}
 
 		if iter.Valid() && strings.HasPrefix(iter.Key(), prefix) {
@@ -61,21 +62,21 @@ func ScanMetaWithPrefix(txn kv.Transaction, prefix string, filter func([]byte, [
 
 // DelKeyWithPrefix deletes keys with prefix.
 func DelKeyWithPrefix(ctx context.Context, prefix string) error {
-	log.Debug("delKeyWithPrefix", prefix)
 	txn, err := ctx.GetTxn(false)
 	if err != nil {
-		return err
+		return errors.Trace(err)
 	}
 
 	var keys []string
 	iter, err := txn.Seek([]byte(prefix), hasPrefix([]byte(prefix)))
 	if err != nil {
-		return err
+		return errors.Trace(err)
 	}
+
 	defer iter.Close()
 	for {
 		if err != nil {
-			return err
+			return errors.Trace(err)
 		}
 
 		if iter.Valid() && strings.HasPrefix(iter.Key(), prefix) {
@@ -89,7 +90,7 @@ func DelKeyWithPrefix(ctx context.Context, prefix string) error {
 	for _, key := range keys {
 		err := txn.Delete([]byte(key))
 		if err != nil {
-			return err
+			return errors.Trace(err)
 		}
 	}
 
diff --git a/util/types/compare.go b/util/types/compare.go
index 2c918b8c40270..3c894bd228853 100644
--- a/util/types/compare.go
+++ b/util/types/compare.go
@@ -55,7 +55,7 @@ func CompareFloat64(x, y float64) int {
 	return 1
 }
 
-// CompareInteger  returns an integer comparing the int64 x to the uint64 y.
+// CompareInteger returns an integer comparing the int64 x to the uint64 y.
 func CompareInteger(x int64, y uint64) int {
 	if x < 0 {
 		return -1
@@ -74,24 +74,24 @@ func CompareString(x, y string) int {
 	return 1
 }
 
-// compareFloatString compares float a to float-formated string s.
+// compareFloatString compares float a with string s.
 // compareFloatString first parses s to a float value, if failed, returns error.
 func compareFloatString(a float64, s string) (int, error) {
-	// MySQL will convert string to a float point value
-	// MySQL use a very loose conversation, e.g, 123.abc -> 123
-	// we should do a trade off whether supporting this feature or using a strict mode
-	// now we use a strict mode
+	// MySQL will convert string to a float point value.
+	// MySQL uses a very loose conversation, e.g, 123.abc -> 123
+	// We should do a trade off whether supporting this feature or using a strict mode.
+	// Now we use a strict mode.
 	b, err := StrToFloat(s)
 	if err != nil {
-		return 0, err
+		return 0, errors.Trace(err)
 	}
 	return CompareFloat64(a, b), nil
 }
 
-// compareStringFloat compares float-formated string s to float a.
+// compareStringFloat compares string s with float a.
 func compareStringFloat(s string, a float64) (int, error) {
 	n, err := compareFloatString(a, s)
-	return -n, err
+	return -n, errors.Trace(err)
 }
 
 func coerceCompare(a, b interface{}) (x interface{}, y interface{}, err error) {
@@ -117,7 +117,7 @@ func coerceCompare(a, b interface{}) (x interface{}, y interface{}, err error) {
 		err = errors.Errorf("invalid comapre type %T cmp %T", a, b)
 	}
 
-	return x, y, err
+	return x, y, errors.Trace(err)
 }
 
 func compareRow(a, b []interface{}) (int, error) {
@@ -136,7 +136,7 @@ func compareRow(a, b []interface{}) (int, error) {
 	return 0, nil
 }
 
-// Compare returns an integer comparing the interface a to b.
+// Compare returns an integer comparing the interface a with b.
 // a > b -> 1
 // a = b -> 0
 // a < b -> -1
@@ -166,7 +166,6 @@ func Compare(a, b interface{}) (int, error) {
 	}
 
 	// TODO: support compare time type with other int, float, decimal types.
-	// TODO: support hexadecimal type
 	switch x := a.(type) {
 	case float64:
 		switch y := b.(type) {
@@ -216,7 +215,7 @@ func Compare(a, b interface{}) (int, error) {
 		case string:
 			f, err := mysql.ConvertToDecimal(y)
 			if err != nil {
-				return 0, err
+				return 0, errors.Trace(err)
 			}
 			return x.Cmp(f), nil
 		}
@@ -233,15 +232,15 @@ func Compare(a, b interface{}) (int, error) {
 		case mysql.Decimal:
 			f, err := mysql.ConvertToDecimal(x)
 			if err != nil {
-				return 0, err
+				return 0, errors.Trace(err)
 			}
 			return f.Cmp(y), nil
 		case mysql.Time:
 			n, err := y.CompareString(x)
-			return -n, err
+			return -n, errors.Trace(err)
 		case mysql.Duration:
 			n, err := y.CompareString(x)
-			return -n, err
+			return -n, errors.Trace(err)
 		case mysql.Hex:
 			return CompareString(x, y.ToString()), nil
 		case mysql.Bit:
diff --git a/util/types/convert.go b/util/types/convert.go
index 52c007742fdbb..640b37a8f2c90 100644
--- a/util/types/convert.go
+++ b/util/types/convert.go
@@ -49,6 +49,7 @@ var unsignedUpperBound = map[byte]uint64{
 	mysql.TypeLonglong: math.MaxUint64,
 	mysql.TypeBit:      math.MaxUint64,
 }
+
 var signedUpperBound = map[byte]int64{
 	mysql.TypeTiny:     math.MaxInt8,
 	mysql.TypeShort:    math.MaxInt16,
@@ -217,7 +218,7 @@ func typeError(v interface{}, target *FieldType) error {
 }
 
 // Cast casts val to certain types and does not return error.
-func Cast(val interface{}, target *FieldType) (v interface{}) { //NTYPE
+func Cast(val interface{}, target *FieldType) (v interface{}) {
 	tp := target.Tp
 	switch tp {
 	case mysql.TypeString:
@@ -284,7 +285,7 @@ func Cast(val interface{}, target *FieldType) (v interface{}) { //NTYPE
 }
 
 // Convert converts the val with type tp.
-func Convert(val interface{}, target *FieldType) (v interface{}, err error) { //NTYPE
+func Convert(val interface{}, target *FieldType) (v interface{}, err error) {
 	tp := target.Tp
 	if val == nil {
 		return nil, nil
@@ -483,7 +484,7 @@ func Convert(val interface{}, target *FieldType) (v interface{}, err error) { //
 // StrToInt converts a string to an integer in best effort.
 // TODO: handle overflow and add unittest.
 func StrToInt(str string) (int64, error) {
-	str = strings.Trim(str, " \t\r\n")
+	str = strings.TrimSpace(str)
 	if len(str) == 0 {
 		return 0, nil
 	}
@@ -512,7 +513,7 @@ func StrToInt(str string) (int64, error) {
 // StrToUint converts a string to an unsigned integer in best effort.
 // TODO: handle overflow and add unittest.
 func StrToUint(str string) (uint64, error) {
-	str = strings.Trim(str, " \t\r\n")
+	str = strings.TrimSpace(str)
 	if len(str) == 0 {
 		return uint64(0), nil
 	}
@@ -540,7 +541,7 @@ func StrToUint(str string) (uint64, error) {
 
 // StrToFloat converts a string to a float64 in best effort.
 func StrToFloat(str string) (float64, error) {
-	str = strings.Trim(str, " \t\r\n")
+	str = strings.TrimSpace(str)
 	if len(str) == 0 {
 		return 0, nil
 	}
@@ -551,7 +552,7 @@ func StrToFloat(str string) (float64, error) {
 	return strconv.ParseFloat(str, 64)
 }
 
-// ToUint64 converts a interface to an uint64.
+// ToUint64 converts an interface to an uint64.
 func ToUint64(value interface{}) (uint64, error) {
 	switch v := value.(type) {
 	case int:
@@ -592,7 +593,7 @@ func ToUint64(value interface{}) (uint64, error) {
 	}
 }
 
-// ToInt64 converts a interface to an int64.
+// ToInt64 converts an interface to an int64.
 func ToInt64(value interface{}) (int64, error) {
 	switch v := value.(type) {
 	case bool:
@@ -638,7 +639,7 @@ func ToInt64(value interface{}) (int64, error) {
 	}
 }
 
-// ToFloat64 converts a interface to a float64.
+// ToFloat64 converts an interface to a float64.
 func ToFloat64(value interface{}) (float64, error) {
 	switch v := value.(type) {
 	case bool:
@@ -682,14 +683,13 @@ func ToFloat64(value interface{}) (float64, error) {
 	}
 }
 
-// ToDecimal converts a interface to the Decimal.
+// ToDecimal converts an interface to a Decimal.
 func ToDecimal(value interface{}) (mysql.Decimal, error) {
 	switch v := value.(type) {
 	case bool:
 		if v {
 			return mysql.ConvertToDecimal(1)
 		}
-
 		return mysql.ConvertToDecimal(0)
 	case []byte:
 		return mysql.ConvertToDecimal(string(v))
@@ -702,7 +702,7 @@ func ToDecimal(value interface{}) (mysql.Decimal, error) {
 	}
 }
 
-// ToString converts a interface to a string.
+// ToString converts an interface to a string.
 func ToString(value interface{}) (string, error) {
 	switch v := value.(type) {
 	case bool:
@@ -743,7 +743,7 @@ func ToString(value interface{}) (string, error) {
 	}
 }
 
-// ToBool converts a interface to a bool.
+// ToBool converts an interface to a bool.
 // We will use 1 for true, and 0 for false.
 func ToBool(value interface{}) (int64, error) {
 	isZero := false
diff --git a/util/types/etc.go b/util/types/etc.go
index 24db6d9176470..7387e25515245 100644
--- a/util/types/etc.go
+++ b/util/types/etc.go
@@ -20,7 +20,6 @@ package types
 import (
 	"fmt"
 	"io"
-	"reflect"
 	"strings"
 
 	"github.com/juju/errors"
@@ -182,7 +181,7 @@ func EOFAsNil(err error) error {
 	if errors2.ErrorEqual(err, io.EOF) {
 		return nil
 	}
-	return err
+	return errors.Trace(err)
 }
 
 // InvOp2 returns an invalid operation error.
@@ -258,7 +257,7 @@ func IsOrderedType(v interface{}) (r bool) {
 	return false
 }
 
-// Clone copies a interface to another interface.
+// Clone copies an interface to another interface.
 // It does a deep copy.
 func Clone(from interface{}) (interface{}, error) {
 	if from == nil {
@@ -285,7 +284,6 @@ func Clone(from interface{}) (interface{}, error) {
 		}
 		return r, nil
 	default:
-		log.Error(reflect.TypeOf(from))
 		return nil, errors.Errorf("Clone invalid type %T", from)
 	}
 }
@@ -335,7 +333,7 @@ func convergeType(a interface{}, hasDecimal, hasFloat *bool) (x interface{}) {
 
 // Coerce changes type.
 // If a or b is Decimal, changes the both to Decimal.
-// If a or b is Float, changes the both to Float.
+// Else if a or b is Float, changes the both to Float.
 func Coerce(a, b interface{}) (x, y interface{}) {
 	var hasDecimal bool
 	var hasFloat bool
diff --git a/util/types/field_type.go b/util/types/field_type.go
index fe989800af3ea..2ac6042ef6511 100644
--- a/util/types/field_type.go
+++ b/util/types/field_type.go
@@ -82,13 +82,15 @@ func (ft *FieldType) String() string {
 	if mysql.HasBinaryFlag(ft.Flag) {
 		ans = append(ans, "BINARY")
 	}
-	if ft.Charset != "" && ft.Charset != charset.CharsetBin &&
-		(IsTypeChar(ft.Tp) || IsTypeBlob(ft.Tp)) {
-		ans = append(ans, fmt.Sprintf("CHARACTER SET %s", ft.Charset))
-	}
-	if ft.Collate != "" && ft.Collate != charset.CharsetBin &&
-		(IsTypeChar(ft.Tp) || IsTypeBlob(ft.Tp)) {
-		ans = append(ans, fmt.Sprintf("COLLATE %s", ft.Collate))
+
+	if IsTypeChar(ft.Tp) || IsTypeBlob(ft.Tp) {
+		if ft.Charset != "" && ft.Charset != charset.CharsetBin {
+			ans = append(ans, fmt.Sprintf("CHARACTER SET %s", ft.Charset))
+		}
+		if ft.Collate != "" && ft.Collate != charset.CharsetBin {
+			ans = append(ans, fmt.Sprintf("COLLATE %s", ft.Collate))
+		}
 	}
+
 	return strings.Join(ans, " ")
 }
diff --git a/util/types/overflow.go b/util/types/overflow.go
index f1c682c083bbe..9a56e71664c08 100644
--- a/util/types/overflow.go
+++ b/util/types/overflow.go
@@ -22,7 +22,7 @@ import (
 // ErrArithOverflow is the error for arthimetic operation overflow.
 var ErrArithOverflow = errors.New("operation overflow")
 
-// AddUint64 adds uint64 a and b if no overflow, else, returns error.
+// AddUint64 adds uint64 a and b if no overflow, else returns error.
 func AddUint64(a uint64, b uint64) (uint64, error) {
 	if math.MaxUint64-a < b {
 		return 0, errors.Trace(ErrArithOverflow)
@@ -30,7 +30,7 @@ func AddUint64(a uint64, b uint64) (uint64, error) {
 	return a + b, nil
 }
 
-// AddInt64 adds int64 a and b if no overflow, otherwise, returns error.
+// AddInt64 adds int64 a and b if no overflow, otherwise returns error.
 func AddInt64(a int64, b int64) (int64, error) {
 	if (a > 0 && b > 0 && math.MaxInt64-a < b) ||
 		(a < 0 && b < 0 && math.MinInt64-a > b) {

From a5f93cb2e8a94e31a24d5362e50a6987d531cf85 Mon Sep 17 00:00:00 2001
From: qiuyesuifeng <cuiqiu.bupt@gmail.com>
Date: Wed, 7 Oct 2015 10:56:32 +0800
Subject: [PATCH 2/2] util: address comment.

---
 util/codec/bytes.go | 2 --
 1 file changed, 2 deletions(-)

diff --git a/util/codec/bytes.go b/util/codec/bytes.go
index 30dc5a2f5d05c..11c8bdf12e3a7 100644
--- a/util/codec/bytes.go
+++ b/util/codec/bytes.go
@@ -99,8 +99,6 @@ func decodeBytes(b []byte, escapeFirst byte, escape byte, term byte) ([]byte, []
 		}
 
 		if b[i+1] == term {
-			// For `var r []byte`, `b := []byte{}`, `r = append(r, b...)`,
-			// here r is equals to nil, so it is better to distinguish it.
 			if r == nil {
 				r = b[:i]
 			} else {