optimizer: tiny clean up.

optimizer/evaluator/evaluator.go

@@ -55,12 +55,10 @@ func Eval(ctx context.Context, expr ast.ExprNode) (interface{}, error) {
 // EvalBool evalueates an expression to a boolean value.
 func EvalBool(ctx context.Context, expr ast.ExprNode) (bool, error) {
 	val, err := Eval(ctx, expr)
-	if err != nil {
+	if err != nil || val == nil {
 		return false, errors.Trace(err)
 	}
-	if val == nil {
-		return false, nil
-	}
+
 	i, err := types.ToBool(val)
 	if err != nil {
 		return false, errors.Trace(err)
@@ -75,7 +73,7 @@ func boolToInt64(v bool) int64 {
 	return int64(0)
 }
 
-// Evaluator is a ast Visitor that evaluates an expression.
+// Evaluator is an ast Visitor that evaluates an expression.
 type Evaluator struct {
 	ctx context.Context
 	err error
@@ -239,7 +237,7 @@ func (e *Evaluator) checkInList(not bool, in interface{}, list []interface{}) (i
 
 		r, err := types.Compare(in, v)
 		if err != nil {
-			return nil, err
+			return nil, errors.Trace(err)
 		}
 
 		if r == 0 {

optimizer/evaluator/evaluator_binop.go

@@ -96,7 +96,7 @@ func (e *Evaluator) handleOrOr(o *ast.BinaryOperationExpr) bool {
 	if !types.IsNil(leftVal) {
 		x, err := types.ToBool(leftVal)
 		if err != nil {
-			e.err = err
+			e.err = errors.Trace(err)
 			return false
 		} else if x == 1 {
 			// true || any other types is true.
@@ -137,7 +137,7 @@ func (e *Evaluator) handleXor(o *ast.BinaryOperationExpr) bool {
 
 	y, err := types.ToBool(righVal)
 	if err != nil {
-		e.err = err
+		e.err = errors.Trace(err)
 		return false
 	}
 	if x == y {
@@ -167,13 +167,13 @@ func (e *Evaluator) handleComparisonOp(o *ast.BinaryOperationExpr) bool {
 
 	n, err := types.Compare(a, b)
 	if err != nil {
-		e.err = err
+		e.err = errors.Trace(err)
 		return false
 	}
 
 	r, err := getCompResult(o.Op, n)
 	if err != nil {
-		e.err = err
+		e.err = errors.Trace(err)
 		return false
 	}
 	if r {
@@ -215,13 +215,13 @@ func (e *Evaluator) handleBitOp(o *ast.BinaryOperationExpr) bool {
 
 	x, err := types.ToInt64(a)
 	if err != nil {
-		e.err = err
+		e.err = errors.Trace(err)
 		return false
 	}
 
 	y, err := types.ToInt64(b)
 	if err != nil {
-		e.err = err
+		e.err = errors.Trace(err)
 		return false
 	}
 
@@ -256,8 +256,8 @@ func (e *Evaluator) handleArithmeticOp(o *ast.BinaryOperationExpr) bool {
 		e.err = errors.Trace(err)
 		return false
 	}
-	a, b = types.Coerce(a, b)
 
+	a, b = types.Coerce(a, b)
 	if a == nil || b == nil {
 		o.SetValue(nil)
 		return true
@@ -385,7 +385,7 @@ func computeDiv(a, b interface{}) (interface{}, error) {
 	case float64:
 		y, err := types.ToFloat64(b)
 		if err != nil {
-			return nil, err
+			return nil, errors.Trace(err)
 		}
 
 		if y == 0 {
@@ -399,12 +399,12 @@ func computeDiv(a, b interface{}) (interface{}, error) {
 		// we will use 4 here
 		xa, err := types.ToDecimal(a)
 		if err != nil {
-			return nil, err
+			return nil, errors.Trace(err)
 		}
 
 		xb, err := types.ToDecimal(b)
 		if err != nil {
-			return nil, err
+			return nil, errors.Trace(err)
 		}
 		if f, _ := xb.Float64(); f == 0 {
 			// division by zero return null
@@ -505,12 +505,12 @@ func computeIntDiv(a, b interface{}) (interface{}, error) {
 	// if any is none integer, use decimal to calculate
 	x, err := types.ToDecimal(a)
 	if err != nil {
-		return nil, err
+		return nil, errors.Trace(err)
 	}
 
 	y, err := types.ToDecimal(b)
 	if err != nil {
-		return nil, err
+		return nil, errors.Trace(err)
 	}
 
 	if f, _ := y.Float64(); f == 0 {
@@ -526,9 +526,9 @@ func coerceArithmetic(a interface{}) (interface{}, error) {
 		// MySQL will convert string to float for arithmetic operation
 		f, err := types.StrToFloat(x)
 		if err != nil {
-			return nil, err
+			return nil, errors.Trace(err)
 		}
-		return f, err
+		return f, errors.Trace(err)
 	case mysql.Time:
 		// if time has no precision, return int64
 		v := x.ToNumber()
@@ -547,9 +547,9 @@ func coerceArithmetic(a interface{}) (interface{}, error) {
 		// []byte is the same as string, converted to float for arithmetic operator.
 		f, err := types.StrToFloat(string(x))
 		if err != nil {
-			return nil, err
+			return nil, errors.Trace(err)
 		}
-		return f, err
+		return f, errors.Trace(err)
 	case mysql.Hex:
 		return x.ToNumber(), nil
 	case mysql.Bit:

optimizer/evaluator/evaluator_like.go

@@ -14,10 +14,11 @@
 package evaluator
 
 import (
+	"regexp"
+
 	"github.com/juju/errors"
 	"github.com/pingcap/tidb/ast"
 	"github.com/pingcap/tidb/util/types"
-	"regexp"
 )
 
 const (
@@ -199,7 +200,7 @@ func (e *Evaluator) patternRegexp(p *ast.PatternRegexpExpr) bool {
 		}
 
 		if re, err = regexp.Compile(spattern); err != nil {
-			e.err = err
+			e.err = errors.Trace(err)
 			return false
 		}
 

optimizer/optimizer.go

@@ -25,7 +25,7 @@ import (
 	"github.com/pingcap/tidb/terror"
 )
 
-// Optimize do optimization and create a Plan.
+// Optimize does optimization and creates a Plan.
 // The node must be prepared first.
 func Optimize(ctx context.Context, node ast.Node) (plan.Plan, error) {
 	// We have to inter type again because after parameter is set, the expression type may change.
@@ -39,8 +39,9 @@ func Optimize(ctx context.Context, node ast.Node) (plan.Plan, error) {
 	if err != nil {
 		return nil, errors.Trace(err)
 	}
-	bestCost := plan.EstimateCost(p)
+
 	bestPlan := p
+	bestCost := plan.EstimateCost(p)
 
 	alts, err := plan.Alternatives(p)
 	if err != nil {
@@ -57,8 +58,8 @@ func Optimize(ctx context.Context, node ast.Node) (plan.Plan, error) {
 }
 
 // Prepare prepares a raw statement parsed from parser.
-// The statement must be prepared before it can be passed to optimize function
-// We pass InfoSchema instead of get from Context in case it is changed after resolving name.
+// The statement must be prepared before it can be passed to optimize function.
+// We pass InfoSchema instead of getting from Context in case it is changed after resolving name.
 func Prepare(is infoschema.InfoSchema, ctx context.Context, node ast.Node) error {
 	if err := Validate(node, true); err != nil {
 		return errors.Trace(err)

optimizer/plan/alternatives.go

@@ -16,7 +16,7 @@ package plan
 import "github.com/juju/errors"
 
 // Alternatives returns multiple alternative plans that
-// can be picked base on their cost.
+// can be picked based on their cost.
 func Alternatives(p Plan) ([]Plan, error) {
 	var plans []Plan
 	switch x := p.(type) {
@@ -52,19 +52,19 @@ func tableScanAlternatives(p *TableScan) []Plan {
 			LowVal:  []interface{}{nil},
 			HighVal: []interface{}{MaxVal},
 		}
-		ip := &IndexScan{
+		is := &IndexScan{
 			Index:  v,
 			Table:  p.Table,
 			Ranges: []*IndexRange{fullRange},
 		}
-		ip.SetFields(p.Fields())
-		alts = append(alts, ip)
+		is.SetFields(p.Fields())
+		alts = append(alts, is)
 	}
 	return alts
 }
 
 // planWithSrcAlternatives shallow copies the WithSrcPlan,
-// and set its src to src alternatives.
+// and sets its src to src alternatives.
 func planWithSrcAlternatives(p WithSrcPlan) ([]Plan, error) {
 	srcs, err := Alternatives(p.Src())
 	if err != nil {

optimizer/plan/cost.go

@@ -71,7 +71,7 @@ func (c *costEstimator) Leave(p Plan) (Plan, bool) {
 			v.rowCount = v.Src().RowCount()
 			v.totalCost = v.Src().TotalCost()
 		} else {
-			// Sort plan must retrieves all the rows before returns the first row.
+			// Sort plan must retrieve all the rows before returns the first row.
 			v.startupCost = v.Src().TotalCost() + v.Src().RowCount()*SortCost
 			if v.limit == 0 {
 				v.rowCount = v.Src().RowCount()
@@ -87,6 +87,7 @@ func (c *costEstimator) Leave(p Plan) (Plan, bool) {
 	}
 	return p, true
 }
+
 func (c *costEstimator) indexScan(v *IndexScan) {
 	var rowCount float64
 	if len(v.Ranges) == 1 && v.Ranges[0].LowVal[0] == nil && v.Ranges[0].HighVal[0] == MaxVal {

optimizer/resolver.go

@@ -23,12 +23,12 @@ import (
 )
 
 // ResolveName resolves table name and column name.
-// It generates ResultFields for ResetSetNode and resolve ColumnNameExpr to a ResultField.
+// It generates ResultFields for ResultSetNode and resolves ColumnNameExpr to a ResultField.
 func ResolveName(node ast.Node, info infoschema.InfoSchema, ctx context.Context) error {
 	defaultSchema := db.GetCurrentSchema(ctx)
 	resolver := nameResolver{Info: info, Ctx: ctx, DefaultSchema: model.NewCIStr(defaultSchema)}
 	node.Accept(&resolver)
-	return resolver.Err
+	return errors.Trace(resolver.Err)
 }
 
 // nameResolver is the visitor to resolve table name and column name.
@@ -37,7 +37,7 @@ func ResolveName(node ast.Node, info infoschema.InfoSchema, ctx context.Context)
 // available for following elements.
 //
 // During visiting, information are collected and stored in resolverContext.
-// When we enter a subquery, a new resolverContext is pushed to the contextStack, so sub query
+// When we enter a subquery, a new resolverContext is pushed to the contextStack, so subquery
 // information can overwrite outer query information. When we look up for a column reference,
 // we look up from top to bottom in the contextStack.
 type nameResolver struct {
@@ -139,7 +139,7 @@ func (nr *nameResolver) Enter(inNode ast.Node) (outNode ast.Node, skipChildren b
 	case *ast.ByItem:
 		if _, ok := v.Expr.(*ast.ColumnNameExpr); !ok {
 			// If ByItem is not a single column name expression,
-			// the resolving rule is different for order by clause.
+			// the resolving rule is different from order by clause.
 			nr.currentContext().inByItemExpression = true
 		}
 	case *ast.InsertStmt:
@@ -194,8 +194,7 @@ func (nr *nameResolver) Leave(inNode ast.Node) (node ast.Node, ok bool) {
 	return inNode, nr.Err == nil
 }
 
-// handleTableName looks up and set the schema information for table name
-// and set result fields for table name.
+// handleTableName looks up and sets the schema information and result fields for table name.
 func (nr *nameResolver) handleTableName(tn *ast.TableName) {
 	if tn.Schema.L == "" {
 		tn.Schema = nr.DefaultSchema
@@ -282,7 +281,7 @@ func (nr *nameResolver) handleColumnName(cn *ast.ColumnNameExpr) {
 }
 
 // resolveColumnNameInContext looks up and sets ResultField for a column with the ctx.
-func (nr *nameResolver) resolveColumnNameInContext(ctx *resolverContext, cn *ast.ColumnNameExpr) (done bool) {
+func (nr *nameResolver) resolveColumnNameInContext(ctx *resolverContext, cn *ast.ColumnNameExpr) bool {
 	if ctx.inTableRefs {
 		// In TableRefsClause, column reference only in join on condition which is handled before.
 		return false
@@ -439,7 +438,7 @@ func (nr *nameResolver) resolveColumnInResultFields(cn *ast.ColumnNameExpr, rfs
 	return false
 }
 
-// handleFieldList expands wild card field and set fieldList in current context.
+// handleFieldList expands wild card field and sets fieldList in current context.
 func (nr *nameResolver) handleFieldList(fieldList *ast.FieldList) {
 	var resultFields []*ast.ResultField
 	for _, v := range fieldList.Fields {
@@ -460,7 +459,7 @@ func (nr *nameResolver) createResultFields(field *ast.SelectField) (rfs []*ast.R
 	ctx := nr.currentContext()
 	if field.WildCard != nil {
 		if len(ctx.tables) == 0 {
-			nr.Err = errors.Errorf("No table used.")
+			nr.Err = errors.New("No table used.")
 			return
 		}
 		if field.WildCard.Table.L == "" {

optimizer/validator.go

@@ -63,7 +63,7 @@ func (v *validator) Leave(in ast.Node) (out ast.Node, ok bool) {
 }
 
 // checkAllOneColumn checks that all expressions have one column.
-// Expression may has more than one column when it is a rowExpr or
+// Expression may have more than one column when it is a rowExpr or
 // a Subquery with more than one result fields.
 func (v *validator) checkAllOneColumn(exprs ...ast.ExprNode) {
 	for _, expr := range exprs {
@@ -113,7 +113,7 @@ func (v *validator) checkSameColumns(exprs ...ast.ExprNode) {
 	}
 }
 
-// checkFieldList checks there is only one '*" and each field has only one column,
+// checkFieldList checks there is only one '*" and each field has only one column.
 func (v *validator) checkFieldList(x *ast.FieldList) {
 	var hasWildCard bool
 	for _, val := range x.Fields {