*: make errcheck work again

.gitignore

@@ -12,5 +12,5 @@ explain_test
 cmd/explaintest/explain-test.out
 cmd/explaintest/explaintest_tidb-server
 *.fail.go
-_tools/bin/
+tools/bin/
 vendor

Makefile

@@ -62,7 +62,7 @@ build:
 # Install the check tools.
 check-setup:tools/bin/megacheck tools/bin/revive tools/bin/goword tools/bin/gometalinter tools/bin/gosec
 
-check: fmt lint tidy
+check: fmt errcheck lint tidy
 
 # These need to be fixed before they can be ran regularly
 check-fail: goword check-static check-slow
@@ -90,6 +90,11 @@ check-slow:tools/bin/gometalinter tools/bin/gosec
 	  --enable errcheck \
 	  $$($(PACKAGE_DIRECTORIES))
 
+errcheck:tools/bin/errcheck
+	@echo "errcheck"
+	@$(GO) mod vendor
+	@tools/bin/errcheck -exclude ./tools/check/errcheck_excludes.txt -blank $(PACKAGES) | grep -v "_test\.go" | awk '{print} END{if(NR>0) {exit 1}}'
+
 lint:tools/bin/revive
 	@echo "linting"
 	@tools/bin/revive -formatter friendly -config tools/check/revive.toml $(FILES)
@@ -221,3 +226,7 @@ tools/bin/gometalinter:
 tools/bin/gosec:
 	cd tools/check; \
 	$(GO) build -o ../bin/gosec github.com/securego/gosec/cmd/gosec
+
+tools/bin/errcheck:
+	cd tools/check; \
+	$(GO) build -o ../bin/errcheck github.com/kisielk/errcheck

ddl/ddl_api.go

@@ -39,6 +39,7 @@ import (
 	"github.com/pingcap/tidb/types"
 	"github.com/pingcap/tidb/util/schemautil"
 	"github.com/pingcap/tidb/util/set"
+	log "github.com/sirupsen/logrus"
 )
 
 func (d *ddl) CreateSchema(ctx sessionctx.Context, schema model.CIStr, charsetInfo *ast.CharsetOpt) (err error) {
@@ -481,9 +482,14 @@ func setTimestampDefaultValue(c *table.Column, hasDefaultValue bool, setOnUpdate
 	// For timestamp Col, if is not set default value or not set null, use current timestamp.
 	if mysql.HasTimestampFlag(c.Flag) && mysql.HasNotNullFlag(c.Flag) {
 		if setOnUpdateNow {
-			c.SetDefaultValue(types.ZeroDatetimeStr)
+			if err := c.SetDefaultValue(types.ZeroDatetimeStr); err != nil {
+				log.Error(errors.ErrorStack(err))
+			}
 		} else {
-			c.SetDefaultValue(strings.ToUpper(ast.CurrentTimestamp))
+			if err := c.SetDefaultValue(strings.ToUpper(ast.CurrentTimestamp)); err != nil {
+				log.Error(errors.ErrorStack(err))
+			}
+
 		}
 	}
 }
@@ -1183,7 +1189,9 @@ func handleTableOptions(options []*ast.TableOption, tbInfo *model.TableInfo) err
 		}
 	}
 
-	setDefaultTableCharsetAndCollation(tbInfo)
+	if err := setDefaultTableCharsetAndCollation(tbInfo); err != nil {
+		log.Error(errors.ErrorStack(err))
+	}
 	return nil
 }
 

domain/domain.go

@@ -393,7 +393,9 @@ func (do *Domain) infoSyncerKeeper() {
 		select {
 		case <-do.info.Done():
 			log.Info("[ddl] server info syncer need to restart")
-			do.info.Restart(context.Background())
+			if err := do.info.Restart(context.Background()); err != nil {
+				log.Error(err)
+			}
 			log.Info("[ddl] server info syncer restarted.")
 		case <-do.exit:
 			return

executor/aggregate.go

@@ -474,7 +474,9 @@ func (w *HashAggFinalWorker) getFinalResult(sctx sessionctx.Context) {
 	for groupKey := range w.groupSet {
 		partialResults := w.getPartialResult(sctx.GetSessionVars().StmtCtx, []byte(groupKey), w.partialResultMap)
 		for i, af := range w.aggFuncs {
-			af.AppendFinalResult2Chunk(sctx, partialResults[i], result)
+			if err := af.AppendFinalResult2Chunk(sctx, partialResults[i], result); err != nil {
+				log.Error(errors.ErrorStack(err))
+			}
 		}
 		if len(w.aggFuncs) == 0 {
 			result.SetNumVirtualRows(result.NumRows() + 1)
@@ -660,7 +662,9 @@ func (e *HashAggExec) unparallelExec(ctx context.Context, chk *chunk.Chunk) erro
 			chk.SetNumVirtualRows(chk.NumRows() + 1)
 		}
 		for i, af := range e.PartialAggFuncs {
-			af.AppendFinalResult2Chunk(e.ctx, partialResults[i], chk)
+			if err := (af.AppendFinalResult2Chunk(e.ctx, partialResults[i], chk)); err != nil {
+				return err
+			}
 		}
 		if chk.NumRows() == e.maxChunkSize {
 			e.cursor4GroupKey++

executor/builder.go

@@ -513,7 +513,9 @@ func (b *executorBuilder) buildShow(v *plannercore.Show) Executor {
 	}
 	if e.Tp == ast.ShowMasterStatus {
 		// show master status need start ts.
-		e.ctx.Txn(true)
+		if _, err := e.ctx.Txn(true); err != nil {
+			b.err = errors.Trace(err)
+		}
 	}
 	if len(v.Conditions) == 0 {
 		return e

executor/explain.go

@@ -82,9 +82,13 @@ func (e *ExplainExec) generateExplainInfo(ctx context.Context) ([][]string, erro
 				break
 			}
 		}
-		e.analyzeExec.Close()
+		if err := e.analyzeExec.Close(); err != nil {
+			return nil, err
+		}
+	}
+	if err := e.explain.RenderResult(); err != nil {
+		return nil, err
 	}
-	e.explain.RenderResult()
 	if e.analyzeExec != nil {
 		e.ctx.GetSessionVars().StmtCtx.RuntimeStatsColl = nil
 	}

executor/index_lookup_join.go

@@ -146,9 +146,12 @@ func (e *IndexLookUpJoin) Open(ctx context.Context) error {
 	// The trick here is `getStartTS` will cache start ts in the dataReaderBuilder,
 	// so even txn is destroyed later, the dataReaderBuilder could still use the
 	// cached start ts to construct DAG.
-	e.innerCtx.readerBuilder.getStartTS()
+	_, err := e.innerCtx.readerBuilder.getStartTS()
+	if err != nil {
+		return err
+	}
 
-	err := e.children[0].Open(ctx)
+	err = e.children[0].Open(ctx)
 	if err != nil {
 		return errors.Trace(err)
 	}

executor/insert_common.go

@@ -219,7 +219,10 @@ func (e *InsertValues) handleErr(col *table.Column, val *types.Datum, rowIdx int
 		return types.ErrWarnDataOutOfRange.GenWithStackByArgs(col.Name.O, rowIdx+1)
 	}
 	if types.ErrTruncated.Equal(err) {
-		valStr, _ := val.ToString()
+		valStr, err1 := val.ToString()
+		if err1 != nil {
+			log.Warn(err1)
+		}
 		return table.ErrTruncatedWrongValueForField.GenWithStackByArgs(types.TypeStr(col.Tp), valStr, col.Name.O, rowIdx+1)
 	}
 	return e.filterErr(err)

executor/show_stats.go

@@ -22,6 +22,7 @@ import (
 	"github.com/pingcap/tidb/statistics"
 	"github.com/pingcap/tidb/store/tikv/oracle"
 	"github.com/pingcap/tidb/types"
+	log "github.com/sirupsen/logrus"
 )
 
 func (e *ShowExec) fetchShowStatsMeta() error {
@@ -115,10 +116,14 @@ func (e *ShowExec) fetchShowStatsBuckets() error {
 		for _, tbl := range db.Tables {
 			pi := tbl.GetPartitionInfo()
 			if pi == nil {
-				e.appendTableForStatsBuckets(db.Name.O, tbl.Name.O, "", h.GetTableStats(tbl))
+				if err := e.appendTableForStatsBuckets(db.Name.O, tbl.Name.O, "", h.GetTableStats(tbl)); err != nil {
+					log.Error(errors.ErrorStack(err))
+				}
 			} else {
 				for _, def := range pi.Definitions {
-					e.appendTableForStatsBuckets(db.Name.O, tbl.Name.O, def.Name.O, h.GetPartitionStats(tbl, def.ID))
+					if err := e.appendTableForStatsBuckets(db.Name.O, tbl.Name.O, def.Name.O, h.GetPartitionStats(tbl, def.ID)); err != nil {
+						log.Error(errors.ErrorStack(err))
+					}
 				}
 			}
 		}

executor/simple.go

@@ -132,7 +132,9 @@ func (e *SimpleExec) executeBegin(ctx context.Context, s *ast.BeginStmt) error {
 	// reverts to its previous state.
 	e.ctx.GetSessionVars().SetStatusFlag(mysql.ServerStatusInTrans, true)
 	// Call ctx.Txn(true) to active pending txn.
-	e.ctx.Txn(true)
+	if _, err := e.ctx.Txn(true); err != nil {
+		return err
+	}
 	return nil
 }
 

planner/cascades/optimize.go

@@ -73,7 +73,9 @@ func exploreGroup(g *memo.Group) error {
 		// Explore child groups firstly.
 		curExpr.Explored = true
 		for _, childGroup := range curExpr.Children {
-			exploreGroup(childGroup)
+			if err := exploreGroup(childGroup); err != nil {
+				return err
+			}
 			curExpr.Explored = curExpr.Explored && childGroup.Explored
 		}
 

planner/core/rule_aggregation_elimination.go

@@ -119,7 +119,10 @@ func (a *aggregationEliminateChecker) wrapCastFunction(ctx sessionctx.Context, a
 func (a *aggregationEliminator) optimize(p LogicalPlan) (LogicalPlan, error) {
 	newChildren := make([]LogicalPlan, 0, len(p.Children()))
 	for _, child := range p.Children() {
-		newChild, _ := a.optimize(child)
+		newChild, err := a.optimize(child)
+		if err != nil {
+			return nil, err
+		}
 		newChildren = append(newChildren, newChild)
 	}
 	p.SetChildren(newChildren...)

planner/core/rule_join_elimination.go

@@ -16,6 +16,7 @@ package core
 import (
 	"github.com/pingcap/parser/ast"
 	"github.com/pingcap/tidb/expression"
+	log "github.com/sirupsen/logrus"
 )
 
 type outerJoinEliminator struct {
@@ -74,7 +75,8 @@ func (o *outerJoinEliminator) isAggColsAllFromOuterTable(outerPlan LogicalPlan,
 	}
 	for _, col := range aggCols {
 		columnName := &ast.ColumnName{Schema: col.DBName, Table: col.TblName, Name: col.ColName}
-		if c, _ := outerPlan.Schema().FindColumn(columnName); c == nil {
+		if c, err := outerPlan.Schema().FindColumn(columnName); c == nil {
+			log.Warn(err)
 			return false
 		}
 	}
@@ -88,7 +90,8 @@ func (o *outerJoinEliminator) isParentColsAllFromOuterTable(outerPlan LogicalPla
 	}
 	for _, col := range parentSchema.Columns {
 		columnName := &ast.ColumnName{Schema: col.DBName, Table: col.TblName, Name: col.ColName}
-		if c, _ := outerPlan.Schema().FindColumn(columnName); c == nil {
+		if c, err := outerPlan.Schema().FindColumn(columnName); c == nil {
+			log.Warn(err)
 			return false
 		}
 	}
@@ -101,7 +104,8 @@ func (o *outerJoinEliminator) isInnerJoinKeysContainUniqueKey(innerPlan LogicalP
 		joinKeysContainKeyInfo := true
 		for _, col := range keyInfo {
 			columnName := &ast.ColumnName{Schema: col.DBName, Table: col.TblName, Name: col.ColName}
-			if c, _ := joinKeys.FindColumn(columnName); c == nil {
+			if c, err := joinKeys.FindColumn(columnName); c == nil {
+				log.Warn(err)
 				joinKeysContainKeyInfo = false
 				break
 			}
@@ -130,7 +134,8 @@ func (o *outerJoinEliminator) isInnerJoinKeysContainIndex(innerPlan LogicalPlan,
 		joinKeysContainIndex := true
 		for _, idxCol := range idx.Columns {
 			columnName := &ast.ColumnName{Schema: ds.DBName, Table: ds.tableInfo.Name, Name: idxCol.Name}
-			if c, _ := joinKeys.FindColumn(columnName); c == nil {
+			if c, err := joinKeys.FindColumn(columnName); c == nil {
+				log.Warn(err)
 				joinKeysContainIndex = false
 				break
 			}

session/session.go

@@ -646,9 +646,13 @@ func (s *session) ExecRestrictedSQLWithSnapshot(sctx sessionctx.Context, sql str
 	}
 	// Set snapshot.
 	if snapshot != 0 {
-		se.sessionVars.SetSystemVar(variable.TiDBSnapshot, strconv.FormatUint(snapshot, 10))
+		if err := se.sessionVars.SetSystemVar(variable.TiDBSnapshot, strconv.FormatUint(snapshot, 10)); err != nil {
+			return nil, nil, errors.Trace(err)
+		}
 		defer func() {
-			se.sessionVars.SetSystemVar(variable.TiDBSnapshot, "")
+			if err := se.sessionVars.SetSystemVar(variable.TiDBSnapshot, ""); err != nil {
+				log.Error(errors.ErrorStack(err))
+			}
 		}()
 	}
 	return execRestrictedSQL(ctx, se, sql)
@@ -745,7 +749,7 @@ func (s *session) getExecRet(ctx sessionctx.Context, sql string) (string, error)
 	d := rows[0].GetDatum(0, &fields[0].Column.FieldType)
 	value, err := d.ToString()
 	if err != nil {
-		return "", errors.Trace(err)
+		return "", err
 	}
 	return value, nil
 }
@@ -878,7 +882,7 @@ func (s *session) execute(ctx context.Context, sql string) (recordSets []sqlexec
 	connID := s.sessionVars.ConnectionID
 	err = s.loadCommonGlobalVariablesIfNeeded()
 	if err != nil {
-		return nil, errors.Trace(err)
+		return nil, err
 	}
 
 	charsetInfo, collation := s.sessionVars.GetCharsetInfo()
@@ -1220,9 +1224,15 @@ func loadSystemTZ(se *session) (string, error) {
 		return "", errLoad
 	}
 	// the record of mysql.tidb under where condition: variable_name = "system_tz" should shall only be one.
-	defer rss[0].Close()
+	defer func() {
+		if err := rss[0].Close(); err != nil {
+			log.Error(errors.ErrorStack(err))
+		}
+	}()
 	chk := rss[0].NewChunk()
-	rss[0].Next(context.Background(), chk)
+	if err := rss[0].Next(context.Background(), chk); err != nil {
+		return "", errors.Trace(err)
+	}
 	return chk.GetRow(0).GetString(0), nil
 }
 
@@ -1472,7 +1482,9 @@ func (s *session) loadCommonGlobalVariablesIfNeeded() error {
 	// when client set Capability Flags CLIENT_INTERACTIVE, init wait_timeout with interactive_timeout
 	if vars.ClientCapability&mysql.ClientInteractive > 0 {
 		if varVal, ok := vars.GetSystemVar(variable.InteractiveTimeout); ok {
-			vars.SetSystemVar(variable.WaitTimeout, varVal)
+			if err := vars.SetSystemVar(variable.WaitTimeout, varVal); err != nil {
+				return err
+			}
 		}
 	}
 

statistics/feedback.go

@@ -1120,7 +1120,9 @@ func setNextValue(d *types.Datum) {
 	case types.KindMysqlTime:
 		t := d.GetMysqlTime()
 		sc := &stmtctx.StatementContext{TimeZone: types.BoundTimezone}
-		t.Add(sc, types.Duration{Duration: 1, Fsp: 0})
+		if _, err := t.Add(sc, types.Duration{Duration: 1, Fsp: 0}); err != nil {
+			log.Error(errors.ErrorStack(err))
+		}
 		d.SetMysqlTime(t)
 	}
 }

tools/check/errcheck_excludes.txt

@@ -0,0 +1,3 @@
+fmt.Fprintf
+fmt.Fprint
+fmt.Sscanf