diff --git a/.gitignore b/.gitignore index a0d63c5e2390c..fda691c5c8ccb 100644 --- a/.gitignore +++ b/.gitignore @@ -12,5 +12,5 @@ explain_test cmd/explaintest/explain-test.out cmd/explaintest/explaintest_tidb-server *.fail.go -_tools/bin/ +tools/bin/ vendor diff --git a/Makefile b/Makefile index dd7a09038ef7a..ff64fceb30611 100644 --- a/Makefile +++ b/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 diff --git a/ddl/ddl_api.go b/ddl/ddl_api.go index b9fbbf80e9c51..01f883a7fe34a 100644 --- a/ddl/ddl_api.go +++ b/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 } diff --git a/domain/domain.go b/domain/domain.go index 0d145d0b4226a..7e40a50232711 100644 --- a/domain/domain.go +++ b/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 diff --git a/executor/aggregate.go b/executor/aggregate.go index 927c8d2440256..9072d0d92c954 100644 --- a/executor/aggregate.go +++ b/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++ diff --git a/executor/builder.go b/executor/builder.go index d2f221b30d4aa..7551b8eaaea77 100644 --- a/executor/builder.go +++ b/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 diff --git a/executor/explain.go b/executor/explain.go index f31634412724f..5e14443df664e 100644 --- a/executor/explain.go +++ b/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 } diff --git a/executor/index_lookup_join.go b/executor/index_lookup_join.go index 81e10c73eaf8e..d0e83c4f25f88 100644 --- a/executor/index_lookup_join.go +++ b/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) } diff --git a/executor/insert_common.go b/executor/insert_common.go index 984da80cfd19c..e3b79c857bf30 100644 --- a/executor/insert_common.go +++ b/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) diff --git a/executor/show_stats.go b/executor/show_stats.go index f6b604c749db7..893f068cdb832 100644 --- a/executor/show_stats.go +++ b/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)) + } } } } diff --git a/executor/simple.go b/executor/simple.go index 13a14680b9ff6..387c4c0b385b6 100644 --- a/executor/simple.go +++ b/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 } diff --git a/planner/cascades/optimize.go b/planner/cascades/optimize.go index 6b43953b35765..e208c88da8561 100644 --- a/planner/cascades/optimize.go +++ b/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 } diff --git a/planner/core/rule_aggregation_elimination.go b/planner/core/rule_aggregation_elimination.go index d75752cd635a6..b48fdd6c479d4 100644 --- a/planner/core/rule_aggregation_elimination.go +++ b/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...) diff --git a/planner/core/rule_join_elimination.go b/planner/core/rule_join_elimination.go index f6dd30834a701..a928cf6724fee 100644 --- a/planner/core/rule_join_elimination.go +++ b/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 } diff --git a/session/session.go b/session/session.go index 814da0c20de46..23d10483b65ad 100644 --- a/session/session.go +++ b/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 + } } } diff --git a/statistics/feedback.go b/statistics/feedback.go index a4e5869d375a8..24fadd8fa762f 100644 --- a/statistics/feedback.go +++ b/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) } } diff --git a/tools/check/errcheck_excludes.txt b/tools/check/errcheck_excludes.txt new file mode 100644 index 0000000000000..7b25b70fd373e --- /dev/null +++ b/tools/check/errcheck_excludes.txt @@ -0,0 +1,3 @@ +fmt.Fprintf +fmt.Fprint +fmt.Sscanf