From 57ff1448ae630bc030b69adbad7e6495f603aa04 Mon Sep 17 00:00:00 2001 From: Weizhen Wang Date: Sun, 3 Jul 2022 17:22:59 +0800 Subject: [PATCH 1/5] *: support predeclared and asciicheck (#35895) ref pingcap/tidb#35345 --- DEPS.bzl | 18 +++++++++++++++++- build/BUILD.bazel | 2 ++ build/linter/asciicheck/BUILD.bazel | 9 +++++++++ build/linter/asciicheck/analysis.go | 20 ++++++++++++++++++++ build/linter/predeclared/BUILD.bazel | 12 ++++++++++++ build/linter/predeclared/analysis.go | 27 +++++++++++++++++++++++++++ build/nogo_config.json | 16 ++++++++++++++++ go.mod | 2 ++ go.sum | 5 +++++ store/driver/error/error.go | 2 +- store/driver/error/error_test.go | 2 +- 11 files changed, 112 insertions(+), 3 deletions(-) create mode 100644 build/linter/asciicheck/BUILD.bazel create mode 100644 build/linter/asciicheck/analysis.go create mode 100644 build/linter/predeclared/BUILD.bazel create mode 100644 build/linter/predeclared/analysis.go diff --git a/DEPS.bzl b/DEPS.bzl index 79f86585b8572..db09c1f163cbf 100644 --- a/DEPS.bzl +++ b/DEPS.bzl @@ -1513,11 +1513,11 @@ def go_deps(): name = "com_github_kisielk_errcheck", build_file_proto_mode = "disable_global", importpath = "github.com/kisielk/errcheck", - sum = "h1:cErYo+J4SmEjdXZrVXGwLJCE2sB06s23LpkcyWNrT+s=", patch_args = ["-p1"], patches = [ "//build/patches:com_github_kisielk_errcheck.patch", ], + sum = "h1:cErYo+J4SmEjdXZrVXGwLJCE2sB06s23LpkcyWNrT+s=", version = "v1.6.1", ) go_repository( @@ -1801,6 +1801,14 @@ def go_deps(): sum = "h1:fD57ERR4JtEqsWbfPhv4DMiApHyliiK5xCTNVSPiaAs=", version = "v0.0.0-20200227124842-a10e7caefd8e", ) + go_repository( + name = "com_github_nishanths_predeclared", + build_file_proto_mode = "disable", + importpath = "github.com/nishanths/predeclared", + sum = "h1:V2EPdZPliZymNAn79T8RkNApBjMmVKh5XRpLm/w98Vk=", + version = "v0.2.2", + ) + go_repository( name = "com_github_nxadm_tail", build_file_proto_mode = "disable_global", @@ -2285,6 +2293,14 @@ def go_deps(): sum = "h1:Slr1R9HxAlEKefgq5jn9U+DnETlIUa6HfgEzj0g5d7s=", version = "v1.2.0", ) + go_repository( + name = "com_github_tdakkota_asciicheck", + build_file_proto_mode = "disable", + importpath = "github.com/tdakkota/asciicheck", + sum = "h1:PKzG7JUTUmVspQTDqtkX9eSiLGossXTybutHwTXuO0A=", + version = "v0.1.1", + ) + go_repository( name = "com_github_tiancaiamao_appdash", build_file_proto_mode = "disable_global", diff --git a/build/BUILD.bazel b/build/BUILD.bazel index 40d98545befd6..3b3ceede0cc22 100644 --- a/build/BUILD.bazel +++ b/build/BUILD.bazel @@ -120,11 +120,13 @@ nogo( "@org_golang_x_tools//go/analysis/passes/unreachable:go_default_library", "@org_golang_x_tools//go/analysis/passes/unsafeptr:go_default_library", "@org_golang_x_tools//go/analysis/passes/unusedresult:go_default_library", + "//build/linter/asciicheck:asciicheck", "//build/linter/durationcheck:durationcheck", "//build/linter/exportloopref:exportloopref", "//build/linter/gofmt:gofmt", "//build/linter/ineffassign:ineffassign", "//build/linter/prealloc:prealloc", + "//build/linter/predeclared:predeclared", "//build/linter/unconvert:unconvert", ] + staticcheck_analyzers(STATICHECK_ANALYZERS) + select({ diff --git a/build/linter/asciicheck/BUILD.bazel b/build/linter/asciicheck/BUILD.bazel new file mode 100644 index 0000000000000..8b1346d863744 --- /dev/null +++ b/build/linter/asciicheck/BUILD.bazel @@ -0,0 +1,9 @@ +load("@io_bazel_rules_go//go:def.bzl", "go_library") + +go_library( + name = "asciicheck", + srcs = ["analysis.go"], + importpath = "github.com/pingcap/tidb/build/linter/asciicheck", + visibility = ["//visibility:public"], + deps = ["@com_github_tdakkota_asciicheck//:go_default_library"], +) diff --git a/build/linter/asciicheck/analysis.go b/build/linter/asciicheck/analysis.go new file mode 100644 index 0000000000000..c78c5db3fd066 --- /dev/null +++ b/build/linter/asciicheck/analysis.go @@ -0,0 +1,20 @@ +// Copyright 2022 PingCAP, Inc. +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package asciicheck + +import "github.com/tdakkota/asciicheck" + +// Analyzer is the analyzer struct of asciicheck. +var Analyzer = asciicheck.NewAnalyzer() diff --git a/build/linter/predeclared/BUILD.bazel b/build/linter/predeclared/BUILD.bazel new file mode 100644 index 0000000000000..5efbe386f0f13 --- /dev/null +++ b/build/linter/predeclared/BUILD.bazel @@ -0,0 +1,12 @@ +load("@io_bazel_rules_go//go:def.bzl", "go_library") + +go_library( + name = "predeclared", + srcs = ["analysis.go"], + importpath = "github.com/pingcap/tidb/build/linter/predeclared", + visibility = ["//visibility:public"], + deps = [ + "//build/linter/util", + "@com_github_nishanths_predeclared//passes/predeclared", + ], +) diff --git a/build/linter/predeclared/analysis.go b/build/linter/predeclared/analysis.go new file mode 100644 index 0000000000000..605b631553f24 --- /dev/null +++ b/build/linter/predeclared/analysis.go @@ -0,0 +1,27 @@ +// Copyright 2022 PingCAP, Inc. +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package predeclared + +import ( + "github.com/nishanths/predeclared/passes/predeclared" + "github.com/pingcap/tidb/build/linter/util" +) + +// Analyzer is the analyzer struct of predeclared. +var Analyzer = predeclared.Analyzer + +func init() { + util.SkipAnalyzer(Analyzer) +} diff --git a/build/nogo_config.json b/build/nogo_config.json index 794090c0f6ccc..3b9938e39c8d1 100644 --- a/build/nogo_config.json +++ b/build/nogo_config.json @@ -1,4 +1,11 @@ { + "asciicheck": { + "exclude_files": { + "/external/": "no need to vet third party code", + ".*_generated\\.go$": "ignore generated code", + "br/pkg/lightning/web/res_vfsdata.go": "ignore code" + } + }, "asmdecl": { "exclude_files": { "/external/": "no need to vet third party code", @@ -643,5 +650,14 @@ "parser/yy_parser.go": "ignore generated code", "/cgo/": "no need to vet third party code for cgo" } + }, + "predeclared": { + "exclude_files": { + "/external/": "no need to vet third party code", + ".*_generated\\.go$": "ignore generated code", + "parser/yy_parser.go": "ignore generated code", + "parser/parser.go": "ignore generated code", + "/cgo/": "no need to vet third party code for cgo" + } } } diff --git a/go.mod b/go.mod index 9e8222fb8275b..17d8a6f3938d4 100644 --- a/go.mod +++ b/go.mod @@ -103,6 +103,8 @@ require ( github.com/gordonklaus/ineffassign v0.0.0-20210914165742-4cc7213b9bc8 github.com/kisielk/errcheck v1.6.1 github.com/kyoh86/exportloopref v0.1.8 + github.com/nishanths/predeclared v0.2.2 + github.com/tdakkota/asciicheck v0.1.1 honnef.co/go/tools v0.0.1-2020.1.4 ) diff --git a/go.sum b/go.sum index 5a10ed780c146..a002b3c67bdc7 100644 --- a/go.sum +++ b/go.sum @@ -602,6 +602,8 @@ github.com/ngaut/pools v0.0.0-20180318154953-b7bc8c42aac7/go.mod h1:iWMfgwqYW+e8 github.com/ngaut/sync2 v0.0.0-20141008032647-7a24ed77b2ef h1:K0Fn+DoFqNqktdZtdV3bPQ/0cuYh2H4rkg0tytX/07k= github.com/ngaut/sync2 v0.0.0-20141008032647-7a24ed77b2ef/go.mod h1:7WjlapSfwQyo6LNmIvEWzsW1hbBQfpUO4JWnuQRmva8= github.com/niemeyer/pretty v0.0.0-20200227124842-a10e7caefd8e/go.mod h1:zD1mROLANZcx1PVRCS0qkT7pwLkGfwJo4zjcN/Tysno= +github.com/nishanths/predeclared v0.2.2 h1:V2EPdZPliZymNAn79T8RkNApBjMmVKh5XRpLm/w98Vk= +github.com/nishanths/predeclared v0.2.2/go.mod h1:RROzoN6TnGQupbC+lqggsOlcgysk3LMK/HI84Mp280c= github.com/nxadm/tail v1.4.4/go.mod h1:kenIhsEOeOJmVchQTgglprH7qJGnHDVpk1VPCcaMI8A= github.com/nxadm/tail v1.4.8 h1:nPr65rt6Y5JFSKQO7qToXr7pePgD6Gwiw05lkbyAQTE= github.com/nxadm/tail v1.4.8/go.mod h1:+ncqLTQzXmGhMZNUePPaPqPvBxHAIsmXswZKocGu+AU= @@ -772,6 +774,8 @@ github.com/stretchr/testify v1.7.0/go.mod h1:6Fq8oRcR53rry900zMqJjRRixrwX3KX962/ github.com/stretchr/testify v1.7.2-0.20220504104629-106ec21d14df h1:rh3VYpfvzXRbJ90ymx1yfhGl/wq8ac2m/cUbao61kwY= github.com/stretchr/testify v1.7.2-0.20220504104629-106ec21d14df/go.mod h1:6Fq8oRcR53rry900zMqJjRRixrwX3KX962/h/Wwjteg= github.com/subosito/gotenv v1.2.0/go.mod h1:N0PQaV/YGNqwC0u51sEeR/aUtSLEXKX9iv69rRypqCw= +github.com/tdakkota/asciicheck v0.1.1 h1:PKzG7JUTUmVspQTDqtkX9eSiLGossXTybutHwTXuO0A= +github.com/tdakkota/asciicheck v0.1.1/go.mod h1:yHp0ai0Z9gUljN3o0xMhYJnH/IcvkdTBOX2fmJ93JEM= github.com/tiancaiamao/appdash v0.0.0-20181126055449-889f96f722a2 h1:mbAskLJ0oJfDRtkanvQPiooDH8HvJ2FBh+iKT/OmiQQ= github.com/tiancaiamao/appdash v0.0.0-20181126055449-889f96f722a2/go.mod h1:2PfKggNGDuadAa0LElHrByyrz4JPZ9fFx6Gs7nx7ZZU= github.com/tikv/client-go/v2 v2.0.1-0.20220627063500-947d923945fd h1:VAyYcN1Nw7RupQszUYqOkueEVapWSxKFU7uBaYY5Dv8= @@ -1205,6 +1209,7 @@ golang.org/x/tools v0.0.0-20200227222343-706bc42d1f0d/go.mod h1:TB2adYChydJhpapK golang.org/x/tools v0.0.0-20200304193943-95d2e580d8eb/go.mod h1:o4KQGtdN14AW+yjsvvwRTJJuXz8XRtIHtEnmAXLyFUw= golang.org/x/tools v0.0.0-20200312045724-11d5b4c81c7d/go.mod h1:o4KQGtdN14AW+yjsvvwRTJJuXz8XRtIHtEnmAXLyFUw= golang.org/x/tools v0.0.0-20200331025713-a30bf2db82d4/go.mod h1:Sl4aGygMT6LrqrWclx+PTx3U+LnKx/seiNR+3G19Ar8= +golang.org/x/tools v0.0.0-20200414032229-332987a829c3/go.mod h1:EkVYQZoAsY45+roYkvgYkIh4xh/qjgUK9TdY2XT94GE= golang.org/x/tools v0.0.0-20200501065659-ab2804fb9c9d/go.mod h1:EkVYQZoAsY45+roYkvgYkIh4xh/qjgUK9TdY2XT94GE= golang.org/x/tools v0.0.0-20200512131952-2bc93b1c0c88/go.mod h1:EkVYQZoAsY45+roYkvgYkIh4xh/qjgUK9TdY2XT94GE= golang.org/x/tools v0.0.0-20200515010526-7d3b6ebf133d/go.mod h1:EkVYQZoAsY45+roYkvgYkIh4xh/qjgUK9TdY2XT94GE= diff --git a/store/driver/error/error.go b/store/driver/error/error.go index 6b7b444239d9c..1d9543cc1437d 100644 --- a/store/driver/error/error.go +++ b/store/driver/error/error.go @@ -12,7 +12,7 @@ // See the License for the specific language governing permissions and // limitations under the License. -package error //nolint:predeclared +package error //nolint: predeclared import ( stderrs "errors" diff --git a/store/driver/error/error_test.go b/store/driver/error/error_test.go index a8c2c6ddc9152..dde341e8da4f1 100644 --- a/store/driver/error/error_test.go +++ b/store/driver/error/error_test.go @@ -12,7 +12,7 @@ // See the License for the specific language governing permissions and // limitations under the License. -package error //nolint:predeclared +package error //nolint: predeclared import ( "testing" From aa7a0fbdc30880ad6bd03f9ae39489a176340917 Mon Sep 17 00:00:00 2001 From: Weizhen Wang Date: Mon, 4 Jul 2022 10:35:00 +0800 Subject: [PATCH 2/5] *: enable misspell (#35719) ref pingcap/tidb#35345 --- DEPS.bzl | 7 +++ build/BUILD.bazel | 1 + build/linter/asciicheck/BUILD.bazel | 2 +- build/linter/misspell/BUILD.bazel | 13 ++++ build/linter/misspell/analyzer.go | 92 +++++++++++++++++++++++++++++ build/linter/util/util.go | 37 ++++++++++++ build/nogo_config.json | 7 +++ go.mod | 1 + go.sum | 2 + parser/parser_test.go | 2 +- tools/check/ut.go | 8 +-- 11 files changed, 166 insertions(+), 6 deletions(-) create mode 100644 build/linter/misspell/BUILD.bazel create mode 100644 build/linter/misspell/analyzer.go diff --git a/DEPS.bzl b/DEPS.bzl index db09c1f163cbf..528f216b3a59d 100644 --- a/DEPS.bzl +++ b/DEPS.bzl @@ -934,6 +934,13 @@ def go_deps(): sum = "h1:iR3fYXUjHCR97qWS8ch1y9zPNsgXThGwjKPrYfqMPks=", version = "v0.0.0-20190930125516-244bba706f1a", ) + go_repository( + name = "com_github_golangci_misspell", + build_file_proto_mode = "disable", + importpath = "github.com/golangci/misspell", + sum = "h1:pLzmVdl3VxTOncgzHcvLOKirdvcx/TydsClUQXTehjo=", + version = "v0.3.5", + ) go_repository( name = "com_github_golangci_prealloc", diff --git a/build/BUILD.bazel b/build/BUILD.bazel index 3b3ceede0cc22..ea8958f219b75 100644 --- a/build/BUILD.bazel +++ b/build/BUILD.bazel @@ -125,6 +125,7 @@ nogo( "//build/linter/exportloopref:exportloopref", "//build/linter/gofmt:gofmt", "//build/linter/ineffassign:ineffassign", + "//build/linter/misspell:misspell", "//build/linter/prealloc:prealloc", "//build/linter/predeclared:predeclared", "//build/linter/unconvert:unconvert", diff --git a/build/linter/asciicheck/BUILD.bazel b/build/linter/asciicheck/BUILD.bazel index 8b1346d863744..f686bc348ca2c 100644 --- a/build/linter/asciicheck/BUILD.bazel +++ b/build/linter/asciicheck/BUILD.bazel @@ -5,5 +5,5 @@ go_library( srcs = ["analysis.go"], importpath = "github.com/pingcap/tidb/build/linter/asciicheck", visibility = ["//visibility:public"], - deps = ["@com_github_tdakkota_asciicheck//:go_default_library"], + deps = ["@com_github_tdakkota_asciicheck//:asciicheck"], ) diff --git a/build/linter/misspell/BUILD.bazel b/build/linter/misspell/BUILD.bazel new file mode 100644 index 0000000000000..8cdc74330a98a --- /dev/null +++ b/build/linter/misspell/BUILD.bazel @@ -0,0 +1,13 @@ +load("@io_bazel_rules_go//go:def.bzl", "go_library") + +go_library( + name = "misspell", + srcs = ["analyzer.go"], + importpath = "github.com/pingcap/tidb/build/linter/misspell", + visibility = ["//visibility:public"], + deps = [ + "//build/linter/util", + "@com_github_golangci_misspell//:go_default_library", + "@org_golang_x_tools//go/analysis", + ], +) diff --git a/build/linter/misspell/analyzer.go b/build/linter/misspell/analyzer.go new file mode 100644 index 0000000000000..3bf4dd16ae77c --- /dev/null +++ b/build/linter/misspell/analyzer.go @@ -0,0 +1,92 @@ +// Copyright 2022 PingCAP, Inc. +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package misspell + +import ( + "fmt" + "go/token" + + "github.com/golangci/misspell" + "github.com/pingcap/tidb/build/linter/util" + "golang.org/x/tools/go/analysis" +) + +// Name is the name of the analyzer. +const Name = "misspell" + +// Analyzer is the analyzer struct of misspell. +var Analyzer = &analysis.Analyzer{ + Name: Name, + Doc: "Checks the spelling error in code", + Run: run, +} + +func init() { + util.SkipAnalyzer(Analyzer) +} + +// Misspell is the config of misspell. +type Misspell struct { + Locale string + IgnoreWords []string `mapstructure:"ignore-words"` +} + +func run(pass *analysis.Pass) (interface{}, error) { + r := misspell.Replacer{ + Replacements: misspell.DictMain, + } + + // Figure out regional variations + settings := &Misspell{ + Locale: "", + } + + if len(settings.IgnoreWords) != 0 { + r.RemoveRule(settings.IgnoreWords) + } + + r.Compile() + files := make([]string, 0, len(pass.Files)) + for _, file := range pass.Files { + pos := pass.Fset.PositionFor(file.Pos(), false) + files = append(files, pos.Filename) + } + for _, f := range files { + err := runOnFile(f, &r, pass) + if err != nil { + return nil, err + } + } + + return nil, nil +} + +func runOnFile(fileName string, r *misspell.Replacer, pass *analysis.Pass) error { + fileContent, tf, err := util.ReadFile(pass.Fset, fileName) + if err != nil { + return fmt.Errorf("can't get file %s contents: %s", fileName, err) + } + + // use r.Replace, not r.ReplaceGo because r.ReplaceGo doesn't find + // issues inside strings: it searches only inside comments. r.Replace + // searches all words: it treats input as a plain text. A standalone misspell + // tool uses r.Replace by default. + _, diffs := r.Replace(string(fileContent)) + for _, diff := range diffs { + text := fmt.Sprintf("[%s] `%s` is a misspelling of `%s`", Name, diff.Original, diff.Corrected) + pass.Reportf(token.Pos(tf.Base()+util.FindOffset(string(fileContent), diff.Line, diff.Column)), text) + } + return nil +} diff --git a/build/linter/util/util.go b/build/linter/util/util.go index 5b58883576d60..0aaeb2bbb7419 100644 --- a/build/linter/util/util.go +++ b/build/linter/util/util.go @@ -18,6 +18,7 @@ import ( "fmt" "go/ast" "go/token" + "io/ioutil" "reflect" "strings" @@ -167,3 +168,39 @@ func MakeFakeLoaderPackageInfo(pass *analysis.Pass) *loader.PackageInfo { Info: *typeInfo, } } + +// ReadFile reads a file and adds it to the FileSet +// so that we can report errors against it using lineStart. +func ReadFile(fset *token.FileSet, filename string) ([]byte, *token.File, error) { + content, err := ioutil.ReadFile(filename) + if err != nil { + return nil, nil, err + } + tf := fset.AddFile(filename, -1, len(content)) + tf.SetLinesForContent(content) + return content, tf, nil +} + +// FindOffset returns the offset of a given position in a file. +func FindOffset(fileText string, line, column int) int { + // we count our current line and column position + currentCol := 1 + currentLine := 1 + + for offset, ch := range fileText { + // see if we found where we wanted to go to + if currentLine == line && currentCol == column { + return offset + } + + // line break - increment the line counter and reset the column + if ch == '\n' { + currentLine++ + currentCol = 1 + } else { + currentCol++ + } + + } + return -1 //not found +} diff --git a/build/nogo_config.json b/build/nogo_config.json index 3b9938e39c8d1..b5edb72788f4f 100644 --- a/build/nogo_config.json +++ b/build/nogo_config.json @@ -178,6 +178,13 @@ ".*_generated\\.go$": "ignore generated code" } }, + "misspell": { + "exclude_files": { + "/cgo/": "ignore cgo code", + "/external/": "no need to vet third party code", + ".*_generated\\.go$": "ignore generated code" + } + }, "nilfunc": { "exclude_files": { "/external/": "no need to vet third party code", diff --git a/go.mod b/go.mod index 17d8a6f3938d4..64211d111e21b 100644 --- a/go.mod +++ b/go.mod @@ -99,6 +99,7 @@ require ( github.com/aliyun/alibaba-cloud-sdk-go v1.61.1581 github.com/charithe/durationcheck v0.0.9 github.com/golangci/gofmt v0.0.0-20190930125516-244bba706f1a + github.com/golangci/misspell v0.3.5 github.com/golangci/unconvert v0.0.0-20180507085042-28b1c447d1f4 github.com/gordonklaus/ineffassign v0.0.0-20210914165742-4cc7213b9bc8 github.com/kisielk/errcheck v1.6.1 diff --git a/go.sum b/go.sum index a002b3c67bdc7..efdf0b627e904 100644 --- a/go.sum +++ b/go.sum @@ -345,6 +345,8 @@ github.com/golang/snappy v0.0.4 h1:yAGX7huGHXlcLOEtBnF4w7FQwA26wojNCwOYAEhLjQM= github.com/golang/snappy v0.0.4/go.mod h1:/XxbfmMg8lxefKM7IXC3fBNl/7bRcc72aCRzEWrmP2Q= github.com/golangci/gofmt v0.0.0-20190930125516-244bba706f1a h1:iR3fYXUjHCR97qWS8ch1y9zPNsgXThGwjKPrYfqMPks= github.com/golangci/gofmt v0.0.0-20190930125516-244bba706f1a/go.mod h1:9qCChq59u/eW8im404Q2WWTrnBUQKjpNYKMbU4M7EFU= +github.com/golangci/misspell v0.3.5 h1:pLzmVdl3VxTOncgzHcvLOKirdvcx/TydsClUQXTehjo= +github.com/golangci/misspell v0.3.5/go.mod h1:dEbvlSfYbMQDtrpRMQU675gSDLDNa8sCPPChZ7PhiVA= github.com/golangci/prealloc v0.0.0-20180630174525-215b22d4de21 h1:leSNB7iYzLYSSx3J/s5sVf4Drkc68W2wm4Ixh/mr0us= github.com/golangci/prealloc v0.0.0-20180630174525-215b22d4de21/go.mod h1:tf5+bzsHdTM0bsB7+8mt0GUMvjCgwLpTapNZHU8AajI= github.com/golangci/unconvert v0.0.0-20180507085042-28b1c447d1f4 h1:zwtduBRr5SSWhqsYNgcuWO2kFlpdOZbP0+yRjmvPGys= diff --git a/parser/parser_test.go b/parser/parser_test.go index f97ca11feada5..ee5c9fc7e384c 100644 --- a/parser/parser_test.go +++ b/parser/parser_test.go @@ -2177,7 +2177,7 @@ func TestIdentifier(t *testing.T) { {`select COUNT from DESC`, false, ""}, {`select COUNT from SELECT.DESC`, true, "SELECT `COUNT` FROM `SELECT`.`DESC`"}, {"use `select`", true, "USE `select`"}, - {"use `sel``ect`", true, "USE `sel``ect`"}, + {"use `sel``ect`", true, "USE `sel``ect`"}, //nolint: misspell {"use select", false, "USE `select`"}, {`select * from t as a`, true, "SELECT * FROM `t` AS `a`"}, {"select 1 full, 1 row, 1 abs", false, ""}, diff --git a/tools/check/ut.go b/tools/check/ut.go index bf1d87aafc7a0..45b969360d245 100644 --- a/tools/check/ut.go +++ b/tools/check/ut.go @@ -125,7 +125,7 @@ func cmdList(args ...string) bool { } exist, err := testBinaryExist(pkg) if err != nil { - fmt.Println("check test binary existance error", err) + fmt.Println("check test binary existence error", err) return false } if !exist { @@ -203,7 +203,7 @@ func cmdRun(args ...string) bool { for _, pkg := range pkgs { exist, err := testBinaryExist(pkg) if err != nil { - fmt.Println("check test binary existance error", err) + fmt.Println("check test binary existence error", err) return false } if !exist { @@ -229,7 +229,7 @@ func cmdRun(args ...string) bool { } exist, err := testBinaryExist(pkg) if err != nil { - fmt.Println("check test binary existance error", err) + fmt.Println("check test binary existence error", err) return false } @@ -254,7 +254,7 @@ func cmdRun(args ...string) bool { } exist, err := testBinaryExist(pkg) if err != nil { - fmt.Println("check test binary existance error", err) + fmt.Println("check test binary existence error", err) return false } if !exist { From c836501f89001491f1db7c0865fcf73e8a28ab81 Mon Sep 17 00:00:00 2001 From: djshow832 Date: Mon, 4 Jul 2022 12:31:00 +0800 Subject: [PATCH 3/5] server, session: report an error when the session states cannot be fetched (#35892) close pingcap/tidb#35866 --- errno/errcode.go | 1 + errno/errname.go | 1 + errors.toml | 5 + server/driver.go | 2 + server/driver_tidb.go | 14 + session/session.go | 24 ++ session/tidb.go | 3 +- sessionctx/sessionstates/BUILD.bazel | 1 + .../sessionstates/session_states_test.go | 406 +++++++++++++++--- 9 files changed, 406 insertions(+), 51 deletions(-) diff --git a/errno/errcode.go b/errno/errcode.go index a26a1a1eaea6e..4054229f8dce5 100644 --- a/errno/errcode.go +++ b/errno/errcode.go @@ -1025,6 +1025,7 @@ const ( ErrNonTransactionalJobFailure = 8143 ErrSettingNoopVariable = 8144 ErrGettingNoopVariable = 8145 + ErrCannotMigrateSession = 8146 // Error codes used by TiDB ddl package ErrUnsupportedDDLOperation = 8200 diff --git a/errno/errname.go b/errno/errname.go index 58866b7564cd0..a8be48e6eed06 100644 --- a/errno/errname.go +++ b/errno/errname.go @@ -1020,6 +1020,7 @@ var MySQLErrName = map[uint16]*mysql.ErrMessage{ ErrNonTransactionalJobFailure: mysql.Message("non-transactional job failed, job id: %d, total jobs: %d. job range: [%s, %s], job sql: %s, err: %v", []int{2, 3, 4}), ErrSettingNoopVariable: mysql.Message("setting %s has no effect in TiDB", nil), ErrGettingNoopVariable: mysql.Message("variable %s has no effect in TiDB", nil), + ErrCannotMigrateSession: mysql.Message("cannot migrate the current session: %s", nil), ErrWarnOptimizerHintInvalidInteger: mysql.Message("integer value is out of range in '%s'", nil), ErrWarnOptimizerHintUnsupportedHint: mysql.Message("Optimizer hint %s is not supported by TiDB and is ignored", nil), diff --git a/errors.toml b/errors.toml index 11518f664cda6..c1c62e3d89c88 100755 --- a/errors.toml +++ b/errors.toml @@ -2261,6 +2261,11 @@ error = ''' non-transactional job failed, job id: %d, total jobs: %d. job range: [%s, %s], job sql: %s, err: %v ''' +["session:8146"] +error = ''' +cannot migrate the current session: %s +''' + ["structure:8217"] error = ''' invalid encoded hash key flag diff --git a/server/driver.go b/server/driver.go index 0363758e3e47b..ae996715113ae 100644 --- a/server/driver.go +++ b/server/driver.go @@ -72,6 +72,8 @@ type ResultSet interface { StoreFetchedRows(rows []chunk.Row) GetFetchedRows() []chunk.Row Close() error + // IsClosed checks whether the result set is closed. + IsClosed() bool } // fetchNotifier represents notifier will be called in COM_FETCH. diff --git a/server/driver_tidb.go b/server/driver_tidb.go index c169449d7df2c..bd96059a30ca4 100644 --- a/server/driver_tidb.go +++ b/server/driver_tidb.go @@ -332,6 +332,15 @@ func (tc *TiDBContext) EncodeSessionStates(ctx context.Context, sctx sessionctx. if !ok { return errors.Errorf("prepared statement %d not found", id) } + // Bound params are sent by CMD_STMT_SEND_LONG_DATA, the proxy can wait for COM_STMT_EXECUTE. + for _, boundParam := range stmt.BoundParams() { + if boundParam != nil { + return session.ErrCannotMigrateSession.GenWithStackByArgs("prepared statements have bound params") + } + } + if rs := stmt.GetResultSet(); rs != nil && !rs.IsClosed() { + return session.ErrCannotMigrateSession.GenWithStackByArgs("prepared statements have open result sets") + } preparedStmtInfo.ParamTypes = stmt.GetParamsType() } return nil @@ -416,6 +425,11 @@ func (trs *tidbResultSet) Close() error { return err } +// IsClosed implements ResultSet.IsClosed interface. +func (trs *tidbResultSet) IsClosed() bool { + return atomic.LoadInt32(&trs.closed) == 1 +} + // OnFetchReturned implements fetchNotifier#OnFetchReturned func (trs *tidbResultSet) OnFetchReturned() { if cl, ok := trs.recordSet.(fetchNotifier); ok { diff --git a/session/session.go b/session/session.go index a00b269d2ee06..efaddb884b0f5 100644 --- a/session/session.go +++ b/session/session.go @@ -3455,6 +3455,30 @@ func (s *session) GetStmtStats() *stmtstats.StatementStats { // EncodeSessionStates implements SessionStatesHandler.EncodeSessionStates interface. func (s *session) EncodeSessionStates(ctx context.Context, sctx sessionctx.Context, sessionStates *sessionstates.SessionStates) error { + // Transaction status is hard to encode, so we do not support it. + s.txn.mu.Lock() + valid := s.txn.Valid() + s.txn.mu.Unlock() + if valid { + return ErrCannotMigrateSession.GenWithStackByArgs("session has an active transaction") + } + // Data in local temporary tables is hard to encode, so we do not support it. + // Check temporary tables here to avoid circle dependency. + if s.sessionVars.LocalTemporaryTables != nil { + localTempTables := s.sessionVars.LocalTemporaryTables.(*infoschema.LocalTemporaryTables) + if localTempTables.Count() > 0 { + return ErrCannotMigrateSession.GenWithStackByArgs("session has local temporary tables") + } + } + // The advisory locks will be released when the session is closed. + if len(s.advisoryLocks) > 0 { + return ErrCannotMigrateSession.GenWithStackByArgs("session has advisory locks") + } + // The TableInfo stores session ID and server ID, so the session cannot be migrated. + if len(s.lockedTables) > 0 { + return ErrCannotMigrateSession.GenWithStackByArgs("session has locked tables") + } + if err := s.sessionVars.EncodeSessionStates(ctx, sessionStates); err != nil { return err } diff --git a/session/tidb.go b/session/tidb.go index 12ee40da2d4be..d0530488b5b3f 100644 --- a/session/tidb.go +++ b/session/tidb.go @@ -378,5 +378,6 @@ func ResultSetToStringSlice(ctx context.Context, s Session, rs sqlexec.RecordSet // Session errors. var ( - ErrForUpdateCantRetry = dbterror.ClassSession.NewStd(errno.ErrForUpdateCantRetry) + ErrForUpdateCantRetry = dbterror.ClassSession.NewStd(errno.ErrForUpdateCantRetry) + ErrCannotMigrateSession = dbterror.ClassSession.NewStd(errno.ErrCannotMigrateSession) ) diff --git a/sessionctx/sessionstates/BUILD.bazel b/sessionctx/sessionstates/BUILD.bazel index 050bd784a7575..1cd0a6c172cc2 100644 --- a/sessionctx/sessionstates/BUILD.bazel +++ b/sessionctx/sessionstates/BUILD.bazel @@ -17,6 +17,7 @@ go_test( timeout = "short", srcs = ["session_states_test.go"], deps = [ + "//config", "//errno", "//parser/mysql", "//parser/terror", diff --git a/sessionctx/sessionstates/session_states_test.go b/sessionctx/sessionstates/session_states_test.go index fc9f42a57884c..b4b886a893af7 100644 --- a/sessionctx/sessionstates/session_states_test.go +++ b/sessionctx/sessionstates/session_states_test.go @@ -16,6 +16,7 @@ package sessionstates_test import ( "context" + "encoding/binary" "fmt" "strconv" "strings" @@ -23,6 +24,7 @@ import ( "time" "github.com/pingcap/errors" + "github.com/pingcap/tidb/config" "github.com/pingcap/tidb/errno" "github.com/pingcap/tidb/parser/mysql" "github.com/pingcap/tidb/parser/terror" @@ -578,37 +580,37 @@ func TestPreparedStatements(t *testing.T) { defer sv.Close() tests := []struct { - setFunc func(tk *testkit.TestKit) any - checkFunc func(tk *testkit.TestKit, param any) + setFunc func(tk *testkit.TestKit, conn server.MockConn) any + checkFunc func(tk *testkit.TestKit, conn server.MockConn, param any) restoreErr int cleanFunc func(tk *testkit.TestKit) }{ { // no such statement - checkFunc: func(tk *testkit.TestKit, param any) { + checkFunc: func(tk *testkit.TestKit, conn server.MockConn, param any) { tk.MustGetErrCode("execute stmt", errno.ErrPreparedStmtNotFound) }, }, { // deallocate it after prepare - setFunc: func(tk *testkit.TestKit) any { + setFunc: func(tk *testkit.TestKit, conn server.MockConn) any { tk.MustExec("prepare stmt from 'select 1'") tk.MustExec("deallocate prepare stmt") return nil }, - checkFunc: func(tk *testkit.TestKit, param any) { + checkFunc: func(tk *testkit.TestKit, conn server.MockConn, param any) { tk.MustGetErrCode("execute stmt", errno.ErrPreparedStmtNotFound) }, }, { // statement with no parameters - setFunc: func(tk *testkit.TestKit) any { + setFunc: func(tk *testkit.TestKit, conn server.MockConn) any { tk.MustExec("create table test.t1(id int)") tk.MustExec("insert into test.t1 value(1), (2), (3)") tk.MustExec("prepare stmt from 'select * from test.t1 order by id'") return nil }, - checkFunc: func(tk *testkit.TestKit, param any) { + checkFunc: func(tk *testkit.TestKit, conn server.MockConn, param any) { tk.MustQuery("execute stmt").Check(testkit.Rows("1", "2", "3")) }, cleanFunc: func(tk *testkit.TestKit) { @@ -617,13 +619,13 @@ func TestPreparedStatements(t *testing.T) { }, { // statement with user-defined parameters - setFunc: func(tk *testkit.TestKit) any { + setFunc: func(tk *testkit.TestKit, conn server.MockConn) any { tk.MustExec("create table test.t1(id int)") tk.MustExec("insert into test.t1 value(1), (2), (3)") tk.MustExec("prepare stmt from 'select * from test.t1 where id>? order by id limit ?'") return nil }, - checkFunc: func(tk *testkit.TestKit, param any) { + checkFunc: func(tk *testkit.TestKit, conn server.MockConn, param any) { tk.MustExec("set @a=1, @b=1") tk.MustQuery("execute stmt using @a, @b").Check(testkit.Rows("2")) }, @@ -633,13 +635,13 @@ func TestPreparedStatements(t *testing.T) { }, { // execute the statement multiple times - setFunc: func(tk *testkit.TestKit) any { + setFunc: func(tk *testkit.TestKit, conn server.MockConn) any { tk.MustExec("create table test.t1(id int)") tk.MustExec("prepare stmt1 from 'insert into test.t1 value(?), (?), (?)'") tk.MustExec("prepare stmt2 from 'select * from test.t1 order by id'") return nil }, - checkFunc: func(tk *testkit.TestKit, param any) { + checkFunc: func(tk *testkit.TestKit, conn server.MockConn, param any) { tk.MustQuery("execute stmt2").Check(testkit.Rows()) tk.MustExec("set @a=1, @b=2, @c=3") tk.MustExec("execute stmt1 using @a, @b, @c") @@ -651,32 +653,32 @@ func TestPreparedStatements(t *testing.T) { }, { // update session variables after prepare - setFunc: func(tk *testkit.TestKit) any { + setFunc: func(tk *testkit.TestKit, conn server.MockConn) any { tk.MustExec("set names utf8mb4 collate utf8mb4_general_ci") tk.MustExec("prepare stmt from 'select @@character_set_client, @@collation_connection'") tk.MustQuery("execute stmt").Check(testkit.Rows("utf8mb4 utf8mb4_general_ci")) tk.MustExec("set names gbk collate gbk_chinese_ci") return nil }, - checkFunc: func(tk *testkit.TestKit, param any) { + checkFunc: func(tk *testkit.TestKit, conn server.MockConn, param any) { tk.MustQuery("execute stmt").Check(testkit.Rows("gbk gbk_chinese_ci")) }, }, { // session-scoped ANSI_QUOTES - setFunc: func(tk *testkit.TestKit) any { + setFunc: func(tk *testkit.TestKit, conn server.MockConn) any { tk.MustExec("set sql_mode='ANSI_QUOTES'") tk.MustExec("prepare stmt from 'select \\'a\\''") tk.MustQuery("execute stmt").Check(testkit.Rows("a")) return nil }, - checkFunc: func(tk *testkit.TestKit, param any) { + checkFunc: func(tk *testkit.TestKit, conn server.MockConn, param any) { tk.MustQuery("execute stmt").Check(testkit.Rows("a")) }, }, { // global-scoped ANSI_QUOTES - setFunc: func(tk *testkit.TestKit) any { + setFunc: func(tk *testkit.TestKit, conn server.MockConn) any { tk.MustExec("set global sql_mode='ANSI_QUOTES'") tk.MustExec("prepare stmt from \"select \\\"a\\\"\"") tk.MustQuery("execute stmt").Check(testkit.Rows("a")) @@ -686,18 +688,18 @@ func TestPreparedStatements(t *testing.T) { }, { // statement name - setFunc: func(tk *testkit.TestKit) any { + setFunc: func(tk *testkit.TestKit, conn server.MockConn) any { tk.MustExec("prepare `stmt 1` from 'select 1'") tk.MustQuery("execute `stmt 1`").Check(testkit.Rows("1")) return nil }, - checkFunc: func(tk *testkit.TestKit, param any) { + checkFunc: func(tk *testkit.TestKit, conn server.MockConn, param any) { tk.MustQuery("execute `stmt 1`").Check(testkit.Rows("1")) }, }, { // multiple prepared statements - setFunc: func(tk *testkit.TestKit) any { + setFunc: func(tk *testkit.TestKit, conn server.MockConn) any { tk.MustExec("create table test.t1(id int)") tk.MustExec("insert into test.t1 value(1), (2), (3)") tk.MustExec("prepare stmt1 from 'select * from test.t1 order by id'") @@ -705,7 +707,7 @@ func TestPreparedStatements(t *testing.T) { tk.MustExec("set @a=1") return nil }, - checkFunc: func(tk *testkit.TestKit, param any) { + checkFunc: func(tk *testkit.TestKit, conn server.MockConn, param any) { tk.MustQuery("execute stmt1").Check(testkit.Rows("1", "2", "3")) tk.MustQuery("execute stmt2 using @a").Check(testkit.Rows("1")) }, @@ -715,7 +717,7 @@ func TestPreparedStatements(t *testing.T) { }, { // change current db after prepare - setFunc: func(tk *testkit.TestKit) any { + setFunc: func(tk *testkit.TestKit, conn server.MockConn) any { tk.MustExec("use test") tk.MustExec("create table t1(id int)") tk.MustExec("insert into t1 value(1), (2), (3)") @@ -723,7 +725,7 @@ func TestPreparedStatements(t *testing.T) { tk.MustExec("use mysql") return nil }, - checkFunc: func(tk *testkit.TestKit, param any) { + checkFunc: func(tk *testkit.TestKit, conn server.MockConn, param any) { tk.MustQuery("select database()").Check(testkit.Rows("mysql")) tk.MustQuery("execute stmt").Check(testkit.Rows("1", "2", "3")) }, @@ -733,7 +735,7 @@ func TestPreparedStatements(t *testing.T) { }, { // update user variable after prepare - setFunc: func(tk *testkit.TestKit) any { + setFunc: func(tk *testkit.TestKit, conn server.MockConn) any { tk.MustExec("create table test.t1(id int)") tk.MustExec("insert into test.t1 value(1), (2), (3)") tk.MustExec("set @a=1") @@ -742,7 +744,7 @@ func TestPreparedStatements(t *testing.T) { tk.MustExec("set @a=2") return nil }, - checkFunc: func(tk *testkit.TestKit, param any) { + checkFunc: func(tk *testkit.TestKit, conn server.MockConn, param any) { tk.MustQuery("execute stmt using @a").Check(testkit.Rows("2")) }, cleanFunc: func(tk *testkit.TestKit) { @@ -751,14 +753,14 @@ func TestPreparedStatements(t *testing.T) { }, { // alter table after prepare - setFunc: func(tk *testkit.TestKit) any { + setFunc: func(tk *testkit.TestKit, conn server.MockConn) any { tk.MustExec("create table test.t1(id int)") tk.MustExec("insert into test.t1 value(1)") tk.MustExec("prepare stmt from 'select * from test.t1'") tk.MustExec("alter table test.t1 add column c char(1) default 'a'") return nil }, - checkFunc: func(tk *testkit.TestKit, param any) { + checkFunc: func(tk *testkit.TestKit, conn server.MockConn, param any) { tk.MustQuery("execute stmt").Check(testkit.Rows("1 a")) }, cleanFunc: func(tk *testkit.TestKit) { @@ -767,7 +769,7 @@ func TestPreparedStatements(t *testing.T) { }, { // drop and create table after prepare - setFunc: func(tk *testkit.TestKit) any { + setFunc: func(tk *testkit.TestKit, conn server.MockConn) any { tk.MustExec("create table test.t1(id int)") tk.MustExec("prepare stmt from 'select * from test.t1'") tk.MustExec("drop table test.t1") @@ -775,7 +777,7 @@ func TestPreparedStatements(t *testing.T) { tk.MustExec("insert into test.t1 value(1, 'a')") return nil }, - checkFunc: func(tk *testkit.TestKit, param any) { + checkFunc: func(tk *testkit.TestKit, conn server.MockConn, param any) { tk.MustQuery("execute stmt").Check(testkit.Rows("1 a")) }, cleanFunc: func(tk *testkit.TestKit) { @@ -784,7 +786,7 @@ func TestPreparedStatements(t *testing.T) { }, { // drop table after prepare - setFunc: func(tk *testkit.TestKit) any { + setFunc: func(tk *testkit.TestKit, conn server.MockConn) any { tk.MustExec("create table test.t1(id int)") tk.MustExec("prepare stmt from 'select * from test.t1'") tk.MustExec("drop table test.t1") @@ -794,7 +796,7 @@ func TestPreparedStatements(t *testing.T) { }, { // drop db after prepare - setFunc: func(tk *testkit.TestKit) any { + setFunc: func(tk *testkit.TestKit, conn server.MockConn) any { tk.MustExec("create database test1") tk.MustExec("use test1") tk.MustExec("create table t1(id int)") @@ -806,7 +808,7 @@ func TestPreparedStatements(t *testing.T) { }, { // update sql_mode after prepare - setFunc: func(tk *testkit.TestKit) any { + setFunc: func(tk *testkit.TestKit, conn server.MockConn) any { tk.MustExec("set sql_mode=''") tk.MustExec("create table test.t1(id int, name char(10))") tk.MustExec("insert into test.t1 value(1, 'a')") @@ -814,7 +816,7 @@ func TestPreparedStatements(t *testing.T) { tk.MustExec("set sql_mode='ONLY_FULL_GROUP_BY'") return nil }, - checkFunc: func(tk *testkit.TestKit, param any) { + checkFunc: func(tk *testkit.TestKit, conn server.MockConn, param any) { // The prepare statement is decoded after decoding session variables, // so `SET SESSION_STATES` won't report errors. tk.MustGetErrCode("execute stmt", errno.ErrFieldNotInGroupBy) @@ -825,7 +827,7 @@ func TestPreparedStatements(t *testing.T) { }, { // update global sql_mode after prepare - setFunc: func(tk *testkit.TestKit) any { + setFunc: func(tk *testkit.TestKit, conn server.MockConn) any { tk.MustExec("set sql_mode=''") tk.MustExec("create table test.t1(id int, name char(10))") tk.MustExec("prepare stmt from 'select id, name from test.t1 group by id'") @@ -840,25 +842,25 @@ func TestPreparedStatements(t *testing.T) { }, { // warnings won't be affected - setFunc: func(tk *testkit.TestKit) any { + setFunc: func(tk *testkit.TestKit, conn server.MockConn) any { // Decoding this prepared statement should report a warning. tk.MustExec("prepare stmt from 'select 0/0'") // Override the warning. tk.MustQuery("select 1") return nil }, - checkFunc: func(tk *testkit.TestKit, param any) { + checkFunc: func(tk *testkit.TestKit, conn server.MockConn, param any) { tk.MustQuery("show warnings").Check(testkit.Rows()) }, }, { // test binary-protocol prepared statement - setFunc: func(tk *testkit.TestKit) any { + setFunc: func(tk *testkit.TestKit, conn server.MockConn) any { stmtID, _, _, err := tk.Session().PrepareStmt("select ?") require.NoError(t, err) return stmtID }, - checkFunc: func(tk *testkit.TestKit, param any) { + checkFunc: func(tk *testkit.TestKit, conn server.MockConn, param any) { datum := []types.Datum{types.NewDatum(1)} rs, err := tk.Session().ExecutePreparedStmt(context.Background(), param.(uint32), datum) require.NoError(t, err) @@ -867,20 +869,20 @@ func TestPreparedStatements(t *testing.T) { }, { // no such prepared statement - checkFunc: func(tk *testkit.TestKit, param any) { + checkFunc: func(tk *testkit.TestKit, conn server.MockConn, param any) { _, err := tk.Session().ExecutePreparedStmt(context.Background(), 1, nil) errEqualsCode(t, err, errno.ErrPreparedStmtNotFound) }, }, { // both text and binary protocols - setFunc: func(tk *testkit.TestKit) any { + setFunc: func(tk *testkit.TestKit, conn server.MockConn) any { tk.MustExec("prepare stmt from 'select 10'") stmtID, _, _, err := tk.Session().PrepareStmt("select ?") require.NoError(t, err) return stmtID }, - checkFunc: func(tk *testkit.TestKit, param any) { + checkFunc: func(tk *testkit.TestKit, conn server.MockConn, param any) { tk.MustQuery("execute stmt").Check(testkit.Rows("10")) datum := []types.Datum{types.NewDatum(1)} rs, err := tk.Session().ExecutePreparedStmt(context.Background(), param.(uint32), datum) @@ -890,19 +892,19 @@ func TestPreparedStatements(t *testing.T) { }, { // drop binary protocol statements - setFunc: func(tk *testkit.TestKit) any { + setFunc: func(tk *testkit.TestKit, conn server.MockConn) any { stmtID, _, _, err := tk.Session().PrepareStmt("select ?") require.NoError(t, err) return stmtID }, - checkFunc: func(tk *testkit.TestKit, param any) { + checkFunc: func(tk *testkit.TestKit, conn server.MockConn, param any) { err := tk.Session().DropPreparedStmt(param.(uint32)) require.NoError(t, err) }, }, { // execute the statement multiple times - setFunc: func(tk *testkit.TestKit) any { + setFunc: func(tk *testkit.TestKit, conn server.MockConn) any { tk.MustExec("create table test.t1(id int)") stmtID1, _, _, err := tk.Session().PrepareStmt("insert into test.t1 value(?), (?), (?)") require.NoError(t, err) @@ -910,7 +912,7 @@ func TestPreparedStatements(t *testing.T) { require.NoError(t, err) return []uint32{stmtID1, stmtID2} }, - checkFunc: func(tk *testkit.TestKit, param any) { + checkFunc: func(tk *testkit.TestKit, conn server.MockConn, param any) { stmtIDs := param.([]uint32) rs, err := tk.Session().ExecutePreparedStmt(context.Background(), stmtIDs[1], nil) require.NoError(t, err) @@ -928,21 +930,40 @@ func TestPreparedStatements(t *testing.T) { }, { // the latter stmt ID should be bigger - setFunc: func(tk *testkit.TestKit) any { + setFunc: func(tk *testkit.TestKit, conn server.MockConn) any { stmtID, _, _, err := tk.Session().PrepareStmt("select ?") require.NoError(t, err) return stmtID }, - checkFunc: func(tk *testkit.TestKit, param any) { + checkFunc: func(tk *testkit.TestKit, conn server.MockConn, param any) { stmtID, _, _, err := tk.Session().PrepareStmt("select ?") require.NoError(t, err) require.True(t, stmtID > param.(uint32)) }, }, + { + // execute the statement with cursor + setFunc: func(tk *testkit.TestKit, conn server.MockConn) any { + cmd := append([]byte{mysql.ComStmtPrepare}, []byte("select ?")...) + require.NoError(t, conn.Dispatch(context.Background(), cmd)) + cmd = getExecuteBytes(1, true, true, paramInfo{value: 1, isNull: false}) + require.NoError(t, conn.Dispatch(context.Background(), cmd)) + cmd = getFetchBytes(1, 10) + require.NoError(t, conn.Dispatch(context.Background(), cmd)) + // This COM_STMT_FETCH returns EOF. + cmd = getFetchBytes(1, 10) + require.NoError(t, conn.Dispatch(context.Background(), cmd)) + return uint32(1) + }, + checkFunc: func(tk *testkit.TestKit, conn server.MockConn, param any) { + cmd := getExecuteBytes(param.(uint32), false, false, paramInfo{value: 1, isNull: false}) + require.NoError(t, conn.Dispatch(context.Background(), cmd)) + }, + }, // Skip this case. Refer to https://github.com/pingcap/tidb/issues/35784. //{ // // update privilege after prepare - // setFunc: func(tk *testkit.TestKit) any { + // setFunc: func(tk *testkit.TestKit, conn server.MockConn) any { // rootTk := testkit.NewTestKit(t, store) // rootTk.MustExec(`CREATE USER 'u1'@'localhost'`) // rootTk.MustExec("create table test.t1(id int)") @@ -952,7 +973,7 @@ func TestPreparedStatements(t *testing.T) { // rootTk.MustExec(`REVOKE SELECT ON test.t1 FROM 'u1'@'localhost'`) // return nil // }, - // prepareFunc: func(tk *testkit.TestKit) { + // prepareFunc: func(tk *testkit.TestKit, conn server.MockConn) { // require.True(t, tk.Session().Auth(&auth.UserIdentity{Username: "u1", Hostname: "localhost"}, nil, nil)) // }, // restoreErr: errno.ErrNoSuchTable, @@ -969,7 +990,7 @@ func TestPreparedStatements(t *testing.T) { tk1 := testkit.NewTestKitWithSession(t, store, conn1.Context().Session) var param any if tt.setFunc != nil { - param = tt.setFunc(tk1) + param = tt.setFunc(tk1, conn1) } conn2 := server.CreateMockConn(t, sv) tk2 := testkit.NewTestKitWithSession(t, store, conn2.Context().Session) @@ -983,7 +1004,7 @@ func TestPreparedStatements(t *testing.T) { tk2.MustGetErrCode(setSQL, tt.restoreErr) } else { tk2.MustExec(setSQL) - tt.checkFunc(tk2, param) + tt.checkFunc(tk2, conn2, param) } if tt.cleanFunc != nil { tt.cleanFunc(tk1) @@ -1167,6 +1188,207 @@ func TestSQLBinding(t *testing.T) { } } +func TestShowStateFail(t *testing.T) { + store, clean := testkit.CreateMockStore(t) + defer clean() + sv := server.CreateMockServer(t, store) + defer sv.Close() + + tests := []struct { + setFunc func(tk *testkit.TestKit, conn server.MockConn) + showErr int + cleanFunc func(tk *testkit.TestKit) + }{ + { + // in an active transaction + setFunc: func(tk *testkit.TestKit, conn server.MockConn) { + tk.MustExec("begin") + }, + showErr: errno.ErrCannotMigrateSession, + }, + { + // out of transaction + setFunc: func(tk *testkit.TestKit, conn server.MockConn) { + tk.MustExec("begin") + tk.MustExec("commit") + }, + }, + { + // created a global temporary table + setFunc: func(tk *testkit.TestKit, conn server.MockConn) { + tk.MustExec("create global temporary table test.t1(id int) on commit delete rows") + tk.MustExec("insert into test.t1 value(1)") + }, + cleanFunc: func(tk *testkit.TestKit) { + tk.MustExec("drop table test.t1") + }, + }, + { + // created a local temporary table + setFunc: func(tk *testkit.TestKit, conn server.MockConn) { + tk.MustExec("create temporary table test.t1(id int)") + }, + showErr: errno.ErrCannotMigrateSession, + }, + { + // drop the local temporary table + setFunc: func(tk *testkit.TestKit, conn server.MockConn) { + tk.MustExec("create temporary table test.t1(id int)") + tk.MustExec("drop table test.t1") + }, + }, + { + // hold and advisory lock + setFunc: func(tk *testkit.TestKit, conn server.MockConn) { + tk.MustQuery("SELECT get_lock('testlock1', 0)").Check(testkit.Rows("1")) + }, + showErr: errno.ErrCannotMigrateSession, + cleanFunc: func(tk *testkit.TestKit) { + tk.MustQuery("SELECT release_lock('testlock1')").Check(testkit.Rows("1")) + }, + }, + { + // release the advisory lock + setFunc: func(tk *testkit.TestKit, conn server.MockConn) { + tk.MustQuery("SELECT get_lock('testlock1', 0)").Check(testkit.Rows("1")) + tk.MustQuery("SELECT release_lock('testlock1')").Check(testkit.Rows("1")) + }, + }, + { + // hold table locks + setFunc: func(tk *testkit.TestKit, conn server.MockConn) { + tk.MustExec("create table test.t1(id int)") + tk.MustExec("lock tables test.t1 write") + tk.MustQuery("show warnings").Check(testkit.Rows()) + }, + showErr: errno.ErrCannotMigrateSession, + cleanFunc: func(tk *testkit.TestKit) { + tk.MustExec("drop table test.t1") + }, + }, + { + // unlock the tables + setFunc: func(tk *testkit.TestKit, conn server.MockConn) { + tk.MustExec("create table test.t1(id int)") + tk.MustExec("lock tables test.t1 write") + tk.MustExec("unlock tables") + }, + cleanFunc: func(tk *testkit.TestKit) { + tk.MustExec("drop table test.t1") + }, + }, + { + // after COM_STMT_SEND_LONG_DATA + setFunc: func(tk *testkit.TestKit, conn server.MockConn) { + cmd := append([]byte{mysql.ComStmtPrepare}, []byte("select ?")...) + require.NoError(t, conn.Dispatch(context.Background(), cmd)) + cmd = getLongDataBytes(1, 0, []byte("abc")) + require.NoError(t, conn.Dispatch(context.Background(), cmd)) + }, + showErr: errno.ErrCannotMigrateSession, + }, + { + // after COM_STMT_SEND_LONG_DATA and COM_STMT_EXECUTE + setFunc: func(tk *testkit.TestKit, conn server.MockConn) { + cmd := append([]byte{mysql.ComStmtPrepare}, []byte("select ?")...) + require.NoError(t, conn.Dispatch(context.Background(), cmd)) + cmd = getLongDataBytes(1, 0, []byte("abc")) + require.NoError(t, conn.Dispatch(context.Background(), cmd)) + cmd = getExecuteBytes(1, false, true, paramInfo{value: 1, isNull: false}) + require.NoError(t, conn.Dispatch(context.Background(), cmd)) + }, + }, + { + // query with cursor, and data is not fetched + setFunc: func(tk *testkit.TestKit, conn server.MockConn) { + tk.MustExec("create table test.t1(id int)") + tk.MustExec("insert test.t1 value(1), (2), (3)") + cmd := append([]byte{mysql.ComStmtPrepare}, []byte("select * from test.t1")...) + require.NoError(t, conn.Dispatch(context.Background(), cmd)) + cmd = getExecuteBytes(1, true, false) + require.NoError(t, conn.Dispatch(context.Background(), cmd)) + }, + showErr: errno.ErrCannotMigrateSession, + cleanFunc: func(tk *testkit.TestKit) { + tk.MustExec("drop table test.t1") + }, + }, + { + // fetched all the data but the EOF packet is not sent + setFunc: func(tk *testkit.TestKit, conn server.MockConn) { + tk.MustExec("create table test.t1(id int)") + tk.MustExec("insert test.t1 value(1), (2), (3)") + cmd := append([]byte{mysql.ComStmtPrepare}, []byte("select * from test.t1")...) + require.NoError(t, conn.Dispatch(context.Background(), cmd)) + cmd = getExecuteBytes(1, true, false) + require.NoError(t, conn.Dispatch(context.Background(), cmd)) + cmd = getFetchBytes(1, 10) + require.NoError(t, conn.Dispatch(context.Background(), cmd)) + }, + showErr: errno.ErrCannotMigrateSession, + cleanFunc: func(tk *testkit.TestKit) { + tk.MustExec("drop table test.t1") + }, + }, + { + // EOF is sent + setFunc: func(tk *testkit.TestKit, conn server.MockConn) { + tk.MustExec("create table test.t1(id int)") + tk.MustExec("insert test.t1 value(1), (2), (3)") + cmd := append([]byte{mysql.ComStmtPrepare}, []byte("select * from test.t1")...) + require.NoError(t, conn.Dispatch(context.Background(), cmd)) + cmd = getExecuteBytes(1, true, false) + require.NoError(t, conn.Dispatch(context.Background(), cmd)) + cmd = getFetchBytes(1, 10) + require.NoError(t, conn.Dispatch(context.Background(), cmd)) + // This COM_STMT_FETCH returns EOF. + cmd = getFetchBytes(1, 10) + require.NoError(t, conn.Dispatch(context.Background(), cmd)) + }, + cleanFunc: func(tk *testkit.TestKit) { + tk.MustExec("drop table test.t1") + }, + }, + { + // statement is reset + setFunc: func(tk *testkit.TestKit, conn server.MockConn) { + tk.MustExec("create table test.t1(id int)") + tk.MustExec("insert test.t1 value(1), (2), (3)") + cmd := append([]byte{mysql.ComStmtPrepare}, []byte("select * from test.t1")...) + require.NoError(t, conn.Dispatch(context.Background(), cmd)) + cmd = getExecuteBytes(1, true, false) + require.NoError(t, conn.Dispatch(context.Background(), cmd)) + cmd = getResetBytes(1) + require.NoError(t, conn.Dispatch(context.Background(), cmd)) + }, + cleanFunc: func(tk *testkit.TestKit) { + tk.MustExec("drop table test.t1") + }, + }, + } + + defer config.RestoreFunc()() + config.UpdateGlobal(func(conf *config.Config) { + conf.EnableTableLock = true + }) + for _, tt := range tests { + conn1 := server.CreateMockConn(t, sv) + tk1 := testkit.NewTestKitWithSession(t, store, conn1.Context().Session) + tt.setFunc(tk1, conn1) + if tt.showErr == 0 { + tk2 := testkit.NewTestKit(t, store) + showSessionStatesAndSet(t, tk1, tk2) + } else { + err := tk1.QueryToErr("show session_states") + errEqualsCode(t, err, tt.showErr) + } + if tt.cleanFunc != nil { + tt.cleanFunc(tk1) + } + conn1.Close() + } +} + func showSessionStatesAndSet(t *testing.T, tk1, tk2 *testkit.TestKit) { rows := tk1.MustQuery("show session_states").Rows() require.Len(t, rows, 1) @@ -1184,3 +1406,87 @@ func errEqualsCode(t *testing.T, err error, code int) { sqlErr := terror.ToSQLError(tErr) require.Equal(t, code, int(sqlErr.Code)) } + +// create bytes for COM_STMT_SEND_LONG_DATA +func getLongDataBytes(stmtID uint32, paramID uint16, param []byte) []byte { + buf := make([]byte, 7+len(param)) + pos := 0 + buf[pos] = mysql.ComStmtSendLongData + pos++ + binary.LittleEndian.PutUint32(buf[pos:], stmtID) + pos += 4 + binary.LittleEndian.PutUint16(buf[pos:], paramID) + pos += 2 + buf = append(buf[:pos], param...) + return buf +} + +type paramInfo struct { + value uint32 + isNull bool +} + +// create bytes for COM_STMT_EXECUTE. It only supports int type for convenience. +func getExecuteBytes(stmtID uint32, useCursor bool, newParam bool, params ...paramInfo) []byte { + nullBitmapLen := (len(params) + 7) >> 3 + buf := make([]byte, 11+nullBitmapLen+len(params)*6) + pos := 0 + buf[pos] = mysql.ComStmtExecute + pos++ + binary.LittleEndian.PutUint32(buf[pos:], stmtID) + pos += 4 + if useCursor { + buf[pos] = 1 + } + pos++ + binary.LittleEndian.PutUint32(buf[pos:], 1) + pos += 4 + for i, param := range params { + if param.isNull { + buf[pos+(i>>3)] |= 1 << (i % 8) + } + } + pos += nullBitmapLen + if newParam { + buf[pos] = 1 + pos++ + for i := 0; i < len(params); i++ { + buf[pos] = mysql.TypeLong + pos++ + buf[pos] = 0 + pos++ + } + } else { + buf[pos] = 0 + pos++ + } + for _, param := range params { + if !param.isNull { + binary.LittleEndian.PutUint32(buf[pos:], param.value) + pos += 4 + } + } + return buf[:pos] +} + +// create bytes for COM_STMT_FETCH. +func getFetchBytes(stmtID, fetchSize uint32) []byte { + buf := make([]byte, 9) + pos := 0 + buf[pos] = mysql.ComStmtFetch + pos++ + binary.LittleEndian.PutUint32(buf[pos:], stmtID) + pos += 4 + binary.LittleEndian.PutUint32(buf[pos:], fetchSize) + return buf +} + +// create bytes for COM_STMT_FETCH. +func getResetBytes(stmtID uint32) []byte { + buf := make([]byte, 5) + pos := 0 + buf[pos] = mysql.ComStmtReset + pos++ + binary.LittleEndian.PutUint32(buf[pos:], stmtID) + return buf +} From aec4349cac9f092466bb7d5067841abe8aae2adf Mon Sep 17 00:00:00 2001 From: Song Gao Date: Mon, 4 Jul 2022 14:03:00 +0800 Subject: [PATCH 4/5] statistics: let index support LoadNeededHistograms (#35775) ref pingcap/tidb#35764 --- executor/show_stats.go | 2 +- parser/model/model.go | 7 + statistics/cmsketch.go | 3 + statistics/handle/bootstrap.go | 14 +- statistics/handle/dump.go | 30 +++-- statistics/handle/handle.go | 195 +++++++++++++++++++--------- statistics/handle/handle_hist.go | 18 +-- statistics/handle/handle_test.go | 19 +++ statistics/handle/lru_cache_test.go | 6 +- statistics/histogram.go | 67 +++++++--- statistics/histogram_test.go | 4 +- statistics/selectivity_test.go | 6 +- statistics/statistics_test.go | 10 +- statistics/table.go | 34 +++-- 14 files changed, 271 insertions(+), 144 deletions(-) diff --git a/executor/show_stats.go b/executor/show_stats.go index 417e285573d75..939ac8eb22299 100644 --- a/executor/show_stats.go +++ b/executor/show_stats.go @@ -439,7 +439,7 @@ func (e *ShowExec) appendTableForStatsHealthy(dbName, tblName, partitionName str } func (e *ShowExec) fetchShowHistogramsInFlight() { - e.appendRow([]interface{}{statistics.HistogramNeededColumns.Length()}) + e.appendRow([]interface{}{statistics.HistogramNeededItems.Length()}) } func (e *ShowExec) fetchShowAnalyzeStatus() error { diff --git a/parser/model/model.go b/parser/model/model.go index 43e3e4bc5bcfb..5bc4a69d3f7e8 100644 --- a/parser/model/model.go +++ b/parser/model/model.go @@ -1396,6 +1396,13 @@ type TableColumnID struct { ColumnID int64 } +// TableItemID is composed by table ID and column/index ID +type TableItemID struct { + TableID int64 + ID int64 + IsIndex bool +} + // PolicyRefInfo is the struct to refer the placement policy. type PolicyRefInfo struct { ID int64 `json:"id"` diff --git a/statistics/cmsketch.go b/statistics/cmsketch.go index 15308e3a84c7a..f32d859e7ad07 100644 --- a/statistics/cmsketch.go +++ b/statistics/cmsketch.go @@ -441,6 +441,9 @@ func DecodeCMSketchAndTopN(data []byte, topNRows []chunk.Row) (*CMSketch, *TopN, // TotalCount returns the total count in the sketch, it is only used for test. func (c *CMSketch) TotalCount() uint64 { + if c == nil { + return 0 + } return c.count } diff --git a/statistics/handle/bootstrap.go b/statistics/handle/bootstrap.go index d865bb2343703..094a02bd1da79 100644 --- a/statistics/handle/bootstrap.go +++ b/statistics/handle/bootstrap.go @@ -112,12 +112,14 @@ func (h *Handle) initStatsHistograms4Chunk(is infoschema.InfoSchema, cache *stat } hist := statistics.NewHistogram(id, ndv, nullCount, version, types.NewFieldType(mysql.TypeBlob), chunk.InitialCapacity, 0) index := &statistics.Index{ - Histogram: *hist, - CMSketch: cms, - TopN: topN, - Info: idxInfo, - StatsVer: statsVer, - Flag: row.GetInt64(10), + Histogram: *hist, + CMSketch: cms, + TopN: topN, + Info: idxInfo, + StatsVer: statsVer, + Flag: row.GetInt64(10), + PhysicalID: tblID, + StatsLoadedStatus: statistics.NewStatsFullLoadStatus(), } lastAnalyzePos.Copy(&index.LastAnalyzePos) table.Indices[hist.ID] = index diff --git a/statistics/handle/dump.go b/statistics/handle/dump.go index 088c1e494e7d1..4fd89b6c11b84 100644 --- a/statistics/handle/dump.go +++ b/statistics/handle/dump.go @@ -283,11 +283,13 @@ func TableStatsFromJSON(tableInfo *model.TableInfo, physicalID int64, jsonTbl *J statsVer = *jsonIdx.StatsVer } idx := &statistics.Index{ - Histogram: *hist, - CMSketch: cm, - TopN: topN, - Info: idxInfo, - StatsVer: statsVer, + Histogram: *hist, + CMSketch: cm, + TopN: topN, + Info: idxInfo, + StatsVer: statsVer, + PhysicalID: physicalID, + StatsLoadedStatus: statistics.NewStatsFullLoadStatus(), } tbl.Indices[idx.ID] = idx } @@ -322,15 +324,15 @@ func TableStatsFromJSON(tableInfo *model.TableInfo, physicalID int64, jsonTbl *J statsVer = *jsonCol.StatsVer } col := &statistics.Column{ - PhysicalID: physicalID, - Histogram: *hist, - CMSketch: cm, - TopN: topN, - FMSketch: fms, - Info: colInfo, - IsHandle: tableInfo.PKIsHandle && mysql.HasPriKeyFlag(colInfo.GetFlag()), - StatsVer: statsVer, - ColLoadedStatus: statistics.NewColFullLoadStatus(), + PhysicalID: physicalID, + Histogram: *hist, + CMSketch: cm, + TopN: topN, + FMSketch: fms, + Info: colInfo, + IsHandle: tableInfo.PKIsHandle && mysql.HasPriKeyFlag(colInfo.GetFlag()), + StatsVer: statsVer, + StatsLoadedStatus: statistics.NewStatsFullLoadStatus(), } col.Count = int64(col.TotalRowCount()) tbl.Columns[col.ID] = col diff --git a/statistics/handle/handle.go b/statistics/handle/handle.go index aa4e970f1c180..8452babe364c9 100644 --- a/statistics/handle/handle.go +++ b/statistics/handle/handle.go @@ -635,9 +635,9 @@ func (h *Handle) updateStatsCache(newCache statsCache) (updated bool) { return } -// LoadNeededHistograms will load histograms for those needed columns. +// LoadNeededHistograms will load histograms for those needed columns/indices. func (h *Handle) LoadNeededHistograms() (err error) { - cols := statistics.HistogramNeededColumns.AllCols() + items := statistics.HistogramNeededItems.AllItems() reader, err := h.getGlobalStatsReader(0) if err != nil { return err @@ -651,64 +651,130 @@ func (h *Handle) LoadNeededHistograms() (err error) { }() loadFMSketch := config.GetGlobalConfig().Performance.EnableLoadFMSketch - for _, col := range cols { - oldCache := h.statsCache.Load().(statsCache) - tbl, ok := oldCache.Get(col.TableID) - if !ok { - continue - } - c, ok := tbl.Columns[col.ColumnID] - if !ok || !c.IsLoadNeeded() { - statistics.HistogramNeededColumns.Delete(col) - continue + for _, item := range items { + if !item.IsIndex { + err = h.loadNeededColumnHistograms(reader, item, loadFMSketch) + } else { + err = h.loadNeededIndexHistograms(reader, item, loadFMSketch) } - hg, err := h.histogramFromStorage(reader, col.TableID, c.ID, &c.Info.FieldType, c.Histogram.NDV, 0, c.LastUpdateVersion, c.NullCount, c.TotColSize, c.Correlation) if err != nil { - return errors.Trace(err) + return err } - cms, topN, err := h.cmSketchAndTopNFromStorage(reader, col.TableID, 0, col.ColumnID) + } + return nil +} + +func (h *Handle) loadNeededColumnHistograms(reader *statsReader, col model.TableItemID, loadFMSketch bool) (err error) { + oldCache := h.statsCache.Load().(statsCache) + tbl, ok := oldCache.Get(col.TableID) + if !ok { + return nil + } + c, ok := tbl.Columns[col.ID] + if !ok || !c.IsLoadNeeded() { + statistics.HistogramNeededItems.Delete(col) + return nil + } + hg, err := h.histogramFromStorage(reader, col.TableID, c.ID, &c.Info.FieldType, c.Histogram.NDV, 0, c.LastUpdateVersion, c.NullCount, c.TotColSize, c.Correlation) + if err != nil { + return errors.Trace(err) + } + cms, topN, err := h.cmSketchAndTopNFromStorage(reader, col.TableID, 0, col.ID) + if err != nil { + return errors.Trace(err) + } + var fms *statistics.FMSketch + if loadFMSketch { + fms, err = h.fmSketchFromStorage(reader, col.TableID, 0, col.ID) if err != nil { return errors.Trace(err) } - var fms *statistics.FMSketch - if loadFMSketch { - fms, err = h.fmSketchFromStorage(reader, col.TableID, 0, col.ColumnID) - if err != nil { - return errors.Trace(err) - } - } - rows, _, err := reader.read("select stats_ver from mysql.stats_histograms where is_index = 0 and table_id = %? and hist_id = %?", col.TableID, col.ColumnID) + } + rows, _, err := reader.read("select stats_ver from mysql.stats_histograms where is_index = 0 and table_id = %? and hist_id = %?", col.TableID, col.ID) + if err != nil { + return errors.Trace(err) + } + if len(rows) == 0 { + logutil.BgLogger().Error("fail to get stats version for this histogram", zap.Int64("table_id", col.TableID), zap.Int64("hist_id", col.ID)) + return errors.Trace(errors.New(fmt.Sprintf("fail to get stats version for this histogram, table_id:%v, hist_id:%v", col.TableID, col.ID))) + } + colHist := &statistics.Column{ + PhysicalID: col.TableID, + Histogram: *hg, + Info: c.Info, + CMSketch: cms, + TopN: topN, + FMSketch: fms, + IsHandle: c.IsHandle, + StatsVer: rows[0].GetInt64(0), + StatsLoadedStatus: statistics.NewStatsFullLoadStatus(), + } + // Column.Count is calculated by Column.TotalRowCount(). Hence we don't set Column.Count when initializing colHist. + colHist.Count = int64(colHist.TotalRowCount()) + // Reload the latest stats cache, otherwise the `updateStatsCache` may fail with high probability, because functions + // like `GetPartitionStats` called in `fmSketchFromStorage` would have modified the stats cache already. + oldCache = h.statsCache.Load().(statsCache) + tbl, ok = oldCache.Get(col.TableID) + if !ok { + return nil + } + tbl = tbl.Copy() + tbl.Columns[c.ID] = colHist + if h.updateStatsCache(oldCache.update([]*statistics.Table{tbl}, nil, oldCache.version)) { + statistics.HistogramNeededItems.Delete(col) + } + return nil +} + +func (h *Handle) loadNeededIndexHistograms(reader *statsReader, idx model.TableItemID, loadFMSketch bool) (err error) { + oldCache := h.statsCache.Load().(statsCache) + tbl, ok := oldCache.Get(idx.TableID) + if !ok { + return nil + } + index, ok := tbl.Indices[idx.ID] + if !ok { + statistics.HistogramNeededItems.Delete(idx) + return nil + } + hg, err := h.histogramFromStorage(reader, idx.TableID, index.ID, types.NewFieldType(mysql.TypeBlob), index.Histogram.NDV, 1, index.LastUpdateVersion, index.NullCount, index.TotColSize, index.Correlation) + if err != nil { + return errors.Trace(err) + } + cms, topN, err := h.cmSketchAndTopNFromStorage(reader, idx.TableID, 1, idx.ID) + if err != nil { + return errors.Trace(err) + } + var fms *statistics.FMSketch + if loadFMSketch { + fms, err = h.fmSketchFromStorage(reader, idx.TableID, 1, idx.ID) if err != nil { return errors.Trace(err) } - if len(rows) == 0 { - logutil.BgLogger().Error("fail to get stats version for this histogram", zap.Int64("table_id", col.TableID), zap.Int64("hist_id", col.ColumnID)) - } - colHist := &statistics.Column{ - PhysicalID: col.TableID, - Histogram: *hg, - Info: c.Info, - CMSketch: cms, - TopN: topN, - FMSketch: fms, - IsHandle: c.IsHandle, - StatsVer: rows[0].GetInt64(0), - ColLoadedStatus: statistics.NewColFullLoadStatus(), - } - // Column.Count is calculated by Column.TotalRowCount(). Hence we don't set Column.Count when initializing colHist. - colHist.Count = int64(colHist.TotalRowCount()) - // Reload the latest stats cache, otherwise the `updateStatsCache` may fail with high probability, because functions - // like `GetPartitionStats` called in `fmSketchFromStorage` would have modified the stats cache already. - oldCache = h.statsCache.Load().(statsCache) - tbl, ok = oldCache.Get(col.TableID) - if !ok { - continue - } - tbl = tbl.Copy() - tbl.Columns[c.ID] = colHist - if h.updateStatsCache(oldCache.update([]*statistics.Table{tbl}, nil, oldCache.version)) { - statistics.HistogramNeededColumns.Delete(col) - } + } + rows, _, err := reader.read("select stats_ver from mysql.stats_histograms where is_index = 1 and table_id = %? and hist_id = %?", idx.TableID, idx.ID) + if err != nil { + return errors.Trace(err) + } + if len(rows) == 0 { + logutil.BgLogger().Error("fail to get stats version for this histogram", zap.Int64("table_id", idx.TableID), zap.Int64("hist_id", idx.ID)) + return errors.Trace(errors.New(fmt.Sprintf("fail to get stats version for this histogram, table_id:%v, hist_id:%v", idx.TableID, idx.ID))) + } + idxHist := &statistics.Index{Histogram: *hg, CMSketch: cms, TopN: topN, FMSketch: fms, + Info: index.Info, ErrorRate: index.ErrorRate, StatsVer: rows[0].GetInt64(0), + Flag: index.Flag, PhysicalID: idx.TableID, + StatsLoadedStatus: statistics.NewStatsFullLoadStatus()} + index.LastAnalyzePos.Copy(&idxHist.LastAnalyzePos) + + oldCache = h.statsCache.Load().(statsCache) + tbl, ok = oldCache.Get(idx.TableID) + if !ok { + return nil + } + tbl = tbl.Copy() + tbl.Indices[idx.ID] = idxHist + if h.updateStatsCache(oldCache.update([]*statistics.Table{tbl}, nil, oldCache.version)) { + statistics.HistogramNeededItems.Delete(idx) } return nil } @@ -794,7 +860,10 @@ func (h *Handle) indexStatsFromStorage(reader *statsReader, row chunk.Row, table if err != nil { return errors.Trace(err) } - idx = &statistics.Index{Histogram: *hg, CMSketch: cms, TopN: topN, FMSketch: fmSketch, Info: idxInfo, ErrorRate: errorRate, StatsVer: row.GetInt64(7), Flag: flag} + idx = &statistics.Index{Histogram: *hg, CMSketch: cms, TopN: topN, FMSketch: fmSketch, + Info: idxInfo, ErrorRate: errorRate, StatsVer: row.GetInt64(7), Flag: flag, + PhysicalID: table.PhysicalID, + StatsLoadedStatus: statistics.NewStatsFullLoadStatus()} lastAnalyzePos.Copy(&idx.LastAnalyzePos) } break @@ -873,17 +942,17 @@ func (h *Handle) columnStatsFromStorage(reader *statsReader, row chunk.Row, tabl return errors.Trace(err) } col = &statistics.Column{ - PhysicalID: table.PhysicalID, - Histogram: *hg, - Info: colInfo, - CMSketch: cms, - TopN: topN, - FMSketch: fmSketch, - ErrorRate: errorRate, - IsHandle: tableInfo.PKIsHandle && mysql.HasPriKeyFlag(colInfo.GetFlag()), - Flag: flag, - StatsVer: statsVer, - ColLoadedStatus: statistics.NewColFullLoadStatus(), + PhysicalID: table.PhysicalID, + Histogram: *hg, + Info: colInfo, + CMSketch: cms, + TopN: topN, + FMSketch: fmSketch, + ErrorRate: errorRate, + IsHandle: tableInfo.PKIsHandle && mysql.HasPriKeyFlag(colInfo.GetFlag()), + Flag: flag, + StatsVer: statsVer, + StatsLoadedStatus: statistics.NewStatsFullLoadStatus(), } // Column.Count is calculated by Column.TotalRowCount(). Hence we don't set Column.Count when initializing col. col.Count = int64(col.TotalRowCount()) diff --git a/statistics/handle/handle_hist.go b/statistics/handle/handle_hist.go index 2dcccadce1f76..36af151ce19db 100644 --- a/statistics/handle/handle_hist.go +++ b/statistics/handle/handle_hist.go @@ -274,15 +274,15 @@ func (h *Handle) readStatsForOne(col model.TableColumnID, c *statistics.Column, logutil.BgLogger().Error("fail to get stats version for this histogram", zap.Int64("table_id", col.TableID), zap.Int64("hist_id", col.ColumnID)) } colHist := &statistics.Column{ - PhysicalID: col.TableID, - Histogram: *hg, - Info: c.Info, - CMSketch: cms, - TopN: topN, - FMSketch: fms, - IsHandle: c.IsHandle, - StatsVer: rows[0].GetInt64(0), - ColLoadedStatus: statistics.NewColFullLoadStatus(), + PhysicalID: col.TableID, + Histogram: *hg, + Info: c.Info, + CMSketch: cms, + TopN: topN, + FMSketch: fms, + IsHandle: c.IsHandle, + StatsVer: rows[0].GetInt64(0), + StatsLoadedStatus: statistics.NewStatsFullLoadStatus(), } // Column.Count is calculated by Column.TotalRowCount(). Hence, we don't set Column.Count when initializing colHist. colHist.Count = int64(colHist.TotalRowCount()) diff --git a/statistics/handle/handle_test.go b/statistics/handle/handle_test.go index 5849625eee6c3..fa5e26d589bcc 100644 --- a/statistics/handle/handle_test.go +++ b/statistics/handle/handle_test.go @@ -613,6 +613,25 @@ func TestLoadStats(t *testing.T) { stat = h.GetTableStats(tableInfo) hg = stat.Columns[tableInfo.Columns[2].ID].Histogram require.Greater(t, hg.Len(), 0) + + // assert index LoadNeededHistograms + idx := stat.Indices[tableInfo.Indices[0].ID] + idx.EvictAllStats() + hg = idx.Histogram + cms = idx.CMSketch + topN = idx.TopN + require.Equal(t, float64(cms.TotalCount()+topN.TotalCount())+hg.TotalRowCount(), float64(0)) + require.False(t, idx.IsEssentialStatsLoaded()) + idx.IsInvalid(false) + require.NoError(t, h.LoadNeededHistograms()) + stat = h.GetTableStats(tableInfo) + idx = stat.Indices[tableInfo.Indices[0].ID] + hg = idx.Histogram + cms = idx.CMSketch + topN = idx.TopN + require.Greater(t, float64(cms.TotalCount()+topN.TotalCount())+hg.TotalRowCount(), float64(0)) + require.True(t, idx.IsFullLoad()) + // Following test tests whether the LoadNeededHistograms would panic. require.NoError(t, failpoint.Enable("github.com/pingcap/tidb/statistics/handle/mockGetStatsReaderFail", `return(true)`)) err = h.LoadNeededHistograms() diff --git a/statistics/handle/lru_cache_test.go b/statistics/handle/lru_cache_test.go index ffb39c593cd2a..d717b95643c23 100644 --- a/statistics/handle/lru_cache_test.go +++ b/statistics/handle/lru_cache_test.go @@ -36,9 +36,9 @@ func newMockStatisticsTable(columns int, indices int) *statistics.Table { t.Indices = make(map[int64]*statistics.Index) for i := 1; i <= columns; i++ { t.Columns[int64(i)] = &statistics.Column{ - Info: &model.ColumnInfo{ID: int64(i)}, - CMSketch: statistics.NewCMSketch(1, 1), - ColLoadedStatus: statistics.NewColFullLoadStatus(), + Info: &model.ColumnInfo{ID: int64(i)}, + CMSketch: statistics.NewCMSketch(1, 1), + StatsLoadedStatus: statistics.NewStatsFullLoadStatus(), } } for i := 1; i <= indices; i++ { diff --git a/statistics/histogram.go b/statistics/histogram.go index 94377998c3421..ab31ae92e49f1 100644 --- a/statistics/histogram.go +++ b/statistics/histogram.go @@ -1057,8 +1057,8 @@ type Column struct { LastAnalyzePos types.Datum StatsVer int64 // StatsVer is the version of the current stats, used to maintain compatibility - // ColLoadedStatus indicates the status of column statistics - ColLoadedStatus + // StatsLoadedStatus indicates the status of column statistics + StatsLoadedStatus } func (c *Column) String() string { @@ -1114,9 +1114,9 @@ func (c *Column) MemoryUsage() CacheItemMemoryUsage { return columnMemUsage } -// HistogramNeededColumns stores the columns whose Histograms need to be loaded from physical kv layer. +// HistogramNeededItems stores the columns/indices whose Histograms need to be loaded from physical kv layer. // Currently, we only load index/pk's Histogram from kv automatically. Columns' are loaded by needs. -var HistogramNeededColumns = neededColumnMap{cols: map[tableColumnID]struct{}{}} +var HistogramNeededItems = neededStatsMap{items: map[model.TableItemID]struct{}{}} // IsInvalid checks if this column is invalid. If this column has histogram but not loaded yet, then we mark it // as need histogram. @@ -1136,7 +1136,7 @@ func (c *Column) IsInvalid(sctx sessionctx.Context, collPseudo bool) bool { } // In some tests, the c.Info is not set, so we add this check here. if c.Info != nil { - HistogramNeededColumns.insert(tableColumnID{TableID: c.PhysicalID, ColumnID: c.Info.ID}) + HistogramNeededItems.insert(model.TableItemID{TableID: c.PhysicalID, ID: c.Info.ID, IsIndex: false}) } } } @@ -1332,6 +1332,8 @@ type Index struct { Info *model.IndexInfo Flag int64 LastAnalyzePos types.Datum + PhysicalID int64 + StatsLoadedStatus } // ItemID implements TableCacheItem @@ -1361,6 +1363,7 @@ func (idx *Index) String() string { // TotalRowCount returns the total count of this index. func (idx *Index) TotalRowCount() float64 { + idx.checkStats() if idx.StatsVer >= Version2 { return idx.Histogram.TotalRowCount() + float64(idx.TopN.TotalCount()) } @@ -1369,9 +1372,21 @@ func (idx *Index) TotalRowCount() float64 { // IsInvalid checks if this index is invalid. func (idx *Index) IsInvalid(collPseudo bool) bool { + if !collPseudo { + idx.checkStats() + } return (collPseudo && idx.NotAccurate()) || idx.TotalRowCount() == 0 } +// EvictAllStats evicts all stats +// Note that this function is only used for test +func (idx *Index) EvictAllStats() { + idx.Buckets = nil + idx.CMSketch = nil + idx.TopN = nil + idx.StatsLoadedStatus.evictedStatus = allEvicted +} + // MemoryUsage returns the total memory usage of a Histogram and CMSketch in Index. // We ignore the size of other metadata in Index. func (idx *Index) MemoryUsage() CacheItemMemoryUsage { @@ -1431,6 +1446,7 @@ func (idx *Index) equalRowCount(b []byte, realtimeRowCount int64) float64 { // QueryBytes is used to query the count of specified bytes. func (idx *Index) QueryBytes(d []byte) uint64 { + idx.checkStats() h1, h2 := murmur3.Sum128(d) if count, ok := idx.TopN.QueryTopN(d); ok { return count @@ -1441,6 +1457,7 @@ func (idx *Index) QueryBytes(d []byte) uint64 { // GetRowCount returns the row count of the given ranges. // It uses the modifyCount to adjust the influence of modifications on the table. func (idx *Index) GetRowCount(sctx sessionctx.Context, coll *HistColl, indexRanges []*ranger.Range, realtimeRowCount int64) (float64, error) { + idx.checkStats() sc := sctx.GetSessionVars().StmtCtx totalCount := float64(0) isSingleCol := len(idx.Info.Columns) == 1 @@ -1609,6 +1626,13 @@ func (idx *Index) expBackoffEstimation(sctx sessionctx.Context, coll *HistColl, return singleColumnEstResults[0] * math.Sqrt(singleColumnEstResults[1]) * math.Sqrt(math.Sqrt(singleColumnEstResults[2])) * math.Sqrt(math.Sqrt(math.Sqrt(singleColumnEstResults[3]))), true, nil } +func (idx *Index) checkStats() { + if idx.IsFullLoad() { + return + } + HistogramNeededItems.insert(model.TableItemID{TableID: idx.PhysicalID, ID: idx.Info.ID, IsIndex: true}) +} + type countByRangeFunc = func(sessionctx.Context, int64, []*ranger.Range) (float64, error) // newHistogramBySelectivity fulfills the content of new histogram by the given selectivity result. @@ -1659,7 +1683,7 @@ func (idx *Index) newIndexBySelectivity(sc *stmtctx.StatementContext, statsNode ranLowEncode, ranHighEncode []byte err error ) - newIndexHist := &Index{Info: idx.Info, StatsVer: idx.StatsVer, CMSketch: idx.CMSketch} + newIndexHist := &Index{Info: idx.Info, StatsVer: idx.StatsVer, CMSketch: idx.CMSketch, PhysicalID: idx.PhysicalID} newIndexHist.Histogram = *NewHistogram(idx.ID, int64(float64(idx.NDV)*statsNode.Selectivity), 0, 0, types.NewFieldType(mysql.TypeBlob), chunk.InitialCapacity, 0) lowBucketIdx, highBucketIdx := 0, 0 @@ -1766,7 +1790,7 @@ func (coll *HistColl) NewHistCollBySelectivity(sctx sessionctx.Context, statsNod zap.Error(err)) continue } - newCol.ColLoadedStatus = oldCol.ColLoadedStatus + newCol.StatsLoadedStatus = oldCol.StatsLoadedStatus newColl.Columns[node.ID] = newCol } for id, idx := range coll.Indices { @@ -2282,29 +2306,29 @@ const ( allEvicted ) -// ColLoadedStatus indicates the status of column statistics -type ColLoadedStatus struct { +// StatsLoadedStatus indicates the status of statistics +type StatsLoadedStatus struct { statsInitialized bool evictedStatus int } -// NewColFullLoadStatus returns the status that the column fully loaded -func NewColFullLoadStatus() ColLoadedStatus { - return ColLoadedStatus{ +// NewStatsFullLoadStatus returns the status that the column/index fully loaded +func NewStatsFullLoadStatus() StatsLoadedStatus { + return StatsLoadedStatus{ statsInitialized: true, evictedStatus: allLoaded, } } -// IsStatsInitialized indicates whether the column's statistics was loaded from storage before. +// IsStatsInitialized indicates whether the column/index's statistics was loaded from storage before. // Note that `IsStatsInitialized` only can be set in initializing -func (s ColLoadedStatus) IsStatsInitialized() bool { +func (s StatsLoadedStatus) IsStatsInitialized() bool { return s.statsInitialized } // IsLoadNeeded indicates whether it needs load statistics during LoadNeededHistograms or sync stats -// If the column was loaded and any statistics of it is evicting, it also needs re-load statistics. -func (s ColLoadedStatus) IsLoadNeeded() bool { +// If the column/index was loaded and any statistics of it is evicting, it also needs re-load statistics. +func (s StatsLoadedStatus) IsLoadNeeded() bool { if s.statsInitialized { return s.evictedStatus > allLoaded } @@ -2312,12 +2336,17 @@ func (s ColLoadedStatus) IsLoadNeeded() bool { } // IsEssentialStatsLoaded indicates whether the essential statistics is loaded. -// If the column was loaded, and at least histogram and topN still exists, the necessary statistics is still loaded. -func (s ColLoadedStatus) IsEssentialStatsLoaded() bool { +// If the column/index was loaded, and at least histogram and topN still exists, the necessary statistics is still loaded. +func (s StatsLoadedStatus) IsEssentialStatsLoaded() bool { return s.statsInitialized && (s.evictedStatus < allEvicted) } // IsCMSEvicted indicates whether the cms got evicted now. -func (s ColLoadedStatus) IsCMSEvicted() bool { +func (s StatsLoadedStatus) IsCMSEvicted() bool { return s.statsInitialized && s.evictedStatus >= onlyCmsEvicted } + +// IsFullLoad indicates whether the stats are full loaded +func (s StatsLoadedStatus) IsFullLoad() bool { + return s.statsInitialized && s.evictedStatus == allLoaded +} diff --git a/statistics/histogram_test.go b/statistics/histogram_test.go index 238cb2f1fbbc8..78c68d3a16dab 100644 --- a/statistics/histogram_test.go +++ b/statistics/histogram_test.go @@ -40,7 +40,7 @@ func TestNewHistogramBySelectivity(t *testing.T) { intCol := &Column{} intCol.Histogram = *NewHistogram(1, 30, 30, 0, types.NewFieldType(mysql.TypeLonglong), chunk.InitialCapacity, 0) intCol.IsHandle = true - intCol.ColLoadedStatus = NewColFullLoadStatus() + intCol.StatsLoadedStatus = NewStatsFullLoadStatus() for i := 0; i < 10; i++ { intCol.Bounds.AppendInt64(0, int64(i*3)) intCol.Bounds.AppendInt64(0, int64(i*3+2)) @@ -62,7 +62,7 @@ num: 1 lower_bound: 12 upper_bound: 14 repeats: 0 ndv: 0 num: 30 lower_bound: 27 upper_bound: 29 repeats: 0 ndv: 0` stringCol := &Column{} - stringCol.ColLoadedStatus = NewColFullLoadStatus() + stringCol.StatsLoadedStatus = NewStatsFullLoadStatus() stringCol.Histogram = *NewHistogram(2, 15, 30, 0, types.NewFieldType(mysql.TypeString), chunk.InitialCapacity, 0) stringCol.Bounds.AppendString(0, "a") stringCol.Bounds.AppendString(0, "aaaabbbb") diff --git a/statistics/selectivity_test.go b/statistics/selectivity_test.go index 760eb2f96d5ea..675263bc241be 100644 --- a/statistics/selectivity_test.go +++ b/statistics/selectivity_test.go @@ -850,9 +850,9 @@ func prepareSelectivity(testKit *testkit.TestKit, dom *domain.Domain) (*statisti } for i := 1; i <= 5; i++ { statsTbl.Columns[int64(i)] = &statistics.Column{ - Histogram: *mockStatsHistogram(int64(i), colValues, 10, types.NewFieldType(mysql.TypeLonglong)), - Info: tbl.Columns[i-1], - ColLoadedStatus: statistics.NewColFullLoadStatus(), + Histogram: *mockStatsHistogram(int64(i), colValues, 10, types.NewFieldType(mysql.TypeLonglong)), + Info: tbl.Columns[i-1], + StatsLoadedStatus: statistics.NewStatsFullLoadStatus(), } } diff --git a/statistics/statistics_test.go b/statistics/statistics_test.go index 4ecabdfdd462d..bb35b094f3f25 100644 --- a/statistics/statistics_test.go +++ b/statistics/statistics_test.go @@ -251,10 +251,10 @@ func SubTestColumnRange() func(*testing.T) { hg.PreCalculateScalar() require.NoError(t, err) col := &Column{ - Histogram: *hg, - CMSketch: buildCMSketch(s.rc.(*recordSet).data), - Info: &model.ColumnInfo{}, - ColLoadedStatus: NewColFullLoadStatus(), + Histogram: *hg, + CMSketch: buildCMSketch(s.rc.(*recordSet).data), + Info: &model.ColumnInfo{}, + StatsLoadedStatus: NewStatsFullLoadStatus(), } tbl := &Table{ HistColl: HistColl{ @@ -327,7 +327,7 @@ func SubTestIntColumnRanges() func(*testing.T) { hg.PreCalculateScalar() require.NoError(t, err) require.Equal(t, int64(100000), rowCount) - col := &Column{Histogram: *hg, Info: &model.ColumnInfo{}, ColLoadedStatus: NewColFullLoadStatus()} + col := &Column{Histogram: *hg, Info: &model.ColumnInfo{}, StatsLoadedStatus: NewStatsFullLoadStatus()} tbl := &Table{ HistColl: HistColl{ Count: int64(col.TotalRowCount()), diff --git a/statistics/table.go b/statistics/table.go index 44a35bf7169b4..1f638ee37ce15 100644 --- a/statistics/table.go +++ b/statistics/table.go @@ -351,42 +351,37 @@ func (t *Table) GetStatsHealthy() (int64, bool) { return healthy, true } -type tableColumnID struct { - TableID int64 - ColumnID int64 +type neededStatsMap struct { + m sync.RWMutex + items map[model.TableItemID]struct{} } -type neededColumnMap struct { - m sync.RWMutex - cols map[tableColumnID]struct{} -} - -func (n *neededColumnMap) AllCols() []tableColumnID { +func (n *neededStatsMap) AllItems() []model.TableItemID { n.m.RLock() - keys := make([]tableColumnID, 0, len(n.cols)) - for key := range n.cols { + keys := make([]model.TableItemID, 0, len(n.items)) + for key := range n.items { keys = append(keys, key) } n.m.RUnlock() return keys } -func (n *neededColumnMap) insert(col tableColumnID) { +func (n *neededStatsMap) insert(col model.TableItemID) { n.m.Lock() - n.cols[col] = struct{}{} + n.items[col] = struct{}{} n.m.Unlock() } -func (n *neededColumnMap) Delete(col tableColumnID) { +func (n *neededStatsMap) Delete(col model.TableItemID) { n.m.Lock() - delete(n.cols, col) + delete(n.items, col) n.m.Unlock() } -func (n *neededColumnMap) Length() int { +func (n *neededStatsMap) Length() int { n.m.RLock() defer n.m.RUnlock() - return len(n.cols) + return len(n.items) } // RatioOfPseudoEstimate means if modifyCount / statsTblCount is greater than this ratio, we think the stats is invalid @@ -898,8 +893,9 @@ func PseudoTable(tblInfo *model.TableInfo) *Table { for _, idx := range tblInfo.Indices { if idx.State == model.StatePublic { t.Indices[idx.ID] = &Index{ - Info: idx, - Histogram: *NewHistogram(idx.ID, 0, 0, 0, types.NewFieldType(mysql.TypeBlob), 0, 0)} + PhysicalID: fakePhysicalID, + Info: idx, + Histogram: *NewHistogram(idx.ID, 0, 0, 0, types.NewFieldType(mysql.TypeBlob), 0, 0)} } } return t From e50b9430fc650ce4629ead05186272ffedf014c8 Mon Sep 17 00:00:00 2001 From: Weizhen Wang Date: Mon, 4 Jul 2022 14:53:02 +0800 Subject: [PATCH 5/5] *: enable gci (#35823) ref pingcap/tidb#35345 --- DEPS.bzl | 16 +++++ br/pkg/lightning/backend/kv/sql2kv.go | 7 +- br/pkg/lightning/backend/local/engine_test.go | 3 +- br/pkg/lightning/errormanager/errormanager.go | 7 +- .../errormanager/errormanager_test.go | 5 +- br/pkg/lightning/mydump/charset_convertor.go | 3 +- br/pkg/lightning/mydump/router.go | 5 +- .../lightning/restore/restore_schema_test.go | 7 +- br/pkg/restore/rawkv_client_test.go | 3 +- br/pkg/storage/azblob.go | 5 +- br/pkg/storage/compress.go | 1 - br/pkg/storage/s3.go | 5 +- br/pkg/stream/client.go | 5 +- br/pkg/stream/integration_test.go | 3 +- br/pkg/task/stream_test.go | 1 - build/BUILD.bazel | 1 + build/linter/gci/BUILD.bazel | 13 ++++ build/linter/gci/analysis.go | 65 +++++++++++++++++++ build/nogo_config.json | 12 ++++ cmd/benchraw/main.go | 1 - ddl/serial_test.go | 5 +- ddl/table_split_test.go | 4 +- distsql/select_result_test.go | 1 - dumpling/cli/versions.go | 3 +- dumpling/cmd/dumpling/main.go | 5 +- dumpling/export/block_allow_list.go | 3 +- dumpling/export/block_allow_list_test.go | 5 +- dumpling/export/config.go | 7 +- dumpling/export/conn.go | 3 +- dumpling/export/consistency.go | 1 - dumpling/export/consistency_test.go | 3 +- dumpling/export/dump.go | 7 +- dumpling/export/dump_test.go | 8 +-- dumpling/export/http_handler.go | 5 +- dumpling/export/ir.go | 1 - dumpling/export/ir_impl.go | 3 +- dumpling/export/metadata.go | 3 +- dumpling/export/metadata_test.go | 3 +- dumpling/export/prepare.go | 1 - dumpling/export/prepare_test.go | 3 +- dumpling/export/retry.go | 5 +- dumpling/export/sql.go | 5 +- dumpling/export/sql_test.go | 8 +-- dumpling/export/status.go | 3 +- dumpling/export/util.go | 3 +- dumpling/export/util_for_test.go | 1 - dumpling/export/util_test.go | 3 +- dumpling/export/writer.go | 3 +- dumpling/export/writer_serial_test.go | 5 +- dumpling/export/writer_test.go | 3 +- dumpling/export/writer_util.go | 5 +- executor/benchmark_test.go | 5 +- executor/opt_rule_blacklist.go | 1 - executor/simple.go | 11 ++-- expression/constant_test.go | 3 +- expression/helper_test.go | 3 +- go.mod | 6 +- go.sum | 4 ++ infoschema/tables.go | 15 ++--- owner/manager_test.go | 21 +++--- parser/ast/ddl_test.go | 3 +- parser/charset/encoding_latin1.go | 1 + parser/hintparser_test.go | 3 +- parser/types/field_type_test.go | 5 +- planner/core/fragment_test.go | 4 +- planner/core/planbuilder_test.go | 1 - server/server.go | 3 +- session/session.go | 45 +++++++------ sessiontxn/isolation/optimistic_test.go | 3 +- store/mockstore/unistore/tikv/deadlock.go | 5 +- store/mockstore/unistore/tikv/mvcc/mvcc.go | 1 - tidb-binlog/pump_client/client_test.go | 53 ++++++++------- util/generatedexpr/gen_expr_test.go | 3 +- util/logutil/hex_test.go | 3 +- util/mock/iter_test.go | 1 - util/sem/sem_test.go | 1 - 76 files changed, 268 insertions(+), 222 deletions(-) create mode 100644 build/linter/gci/BUILD.bazel create mode 100644 build/linter/gci/analysis.go diff --git a/DEPS.bzl b/DEPS.bzl index 528f216b3a59d..4b36dbcba743c 100644 --- a/DEPS.bzl +++ b/DEPS.bzl @@ -473,6 +473,14 @@ def go_deps(): sum = "h1:0rkFMAbn5KBKNpJyHQ6Prb95vIKanmAe62KxsrN+sqA=", version = "v0.0.0-20171016134553-529a34b1c186", ) + go_repository( + name = "com_github_daixiang0_gci", + build_file_proto_mode = "disable", + importpath = "github.com/daixiang0/gci", + sum = "h1:+EZ83znNs73C9ZBTM7xhNagMP6gJs5wlptiFiuce5BM=", + version = "v0.3.4", + ) + go_repository( name = "com_github_danjacques_gofslock", build_file_proto_mode = "disable_global", @@ -1250,6 +1258,14 @@ def go_deps(): sum = "h1:5IcZpTvzydCQeHzK4Ef/D5rrSqwxob0t8PQPMybUNFM=", version = "v1.1.2", ) + go_repository( + name = "com_github_hexops_gotextdiff", + build_file_proto_mode = "disable", + importpath = "github.com/hexops/gotextdiff", + sum = "h1:gitA9+qJrrTCsiCl7+kh75nPqQt1cx4ZkudSTLoUqJM=", + version = "v1.0.3", + ) + go_repository( name = "com_github_hpcloud_tail", build_file_proto_mode = "disable_global", diff --git a/br/pkg/lightning/backend/kv/sql2kv.go b/br/pkg/lightning/backend/kv/sql2kv.go index 8a10ce607d9f7..6423e59bcb65a 100644 --- a/br/pkg/lightning/backend/kv/sql2kv.go +++ b/br/pkg/lightning/backend/kv/sql2kv.go @@ -33,7 +33,9 @@ import ( "github.com/pingcap/tidb/expression" "github.com/pingcap/tidb/meta/autoid" "github.com/pingcap/tidb/parser/model" - "github.com/pingcap/tidb/parser/mysql" + "github.com/pingcap/tidb/parser/mysql" //nolint: goimports + // Import tidb/planner/core to initialize expression.RewriteAstExpr + _ "github.com/pingcap/tidb/planner/core" "github.com/pingcap/tidb/sessionctx/variable" "github.com/pingcap/tidb/table" "github.com/pingcap/tidb/table/tables" @@ -42,9 +44,6 @@ import ( "github.com/pingcap/tidb/util/chunk" "go.uber.org/zap" "go.uber.org/zap/zapcore" - - // Import tidb/planner/core to initialize expression.RewriteAstExpr - _ "github.com/pingcap/tidb/planner/core" ) var ExtraHandleColumnInfo = model.NewExtraHandleColInfo() diff --git a/br/pkg/lightning/backend/local/engine_test.go b/br/pkg/lightning/backend/local/engine_test.go index a0d8592d5398f..c7ffe04b95285 100644 --- a/br/pkg/lightning/backend/local/engine_test.go +++ b/br/pkg/lightning/backend/local/engine_test.go @@ -26,10 +26,9 @@ import ( "github.com/cockroachdb/pebble" "github.com/cockroachdb/pebble/sstable" "github.com/google/uuid" + "github.com/pingcap/tidb/br/pkg/lightning/backend" "github.com/pingcap/tidb/br/pkg/lightning/log" "github.com/stretchr/testify/require" - - "github.com/pingcap/tidb/br/pkg/lightning/backend" ) func TestIngestSSTWithClosedEngine(t *testing.T) { diff --git a/br/pkg/lightning/errormanager/errormanager.go b/br/pkg/lightning/errormanager/errormanager.go index b900d19c3a431..43035716d729c 100644 --- a/br/pkg/lightning/errormanager/errormanager.go +++ b/br/pkg/lightning/errormanager/errormanager.go @@ -25,15 +25,14 @@ import ( "github.com/jedib0t/go-pretty/v6/table" "github.com/jedib0t/go-pretty/v6/text" "github.com/pingcap/errors" - "go.uber.org/multierr" - "go.uber.org/zap" - "golang.org/x/sync/errgroup" - "github.com/pingcap/tidb/br/pkg/lightning/common" "github.com/pingcap/tidb/br/pkg/lightning/config" "github.com/pingcap/tidb/br/pkg/lightning/log" "github.com/pingcap/tidb/br/pkg/redact" "github.com/pingcap/tidb/br/pkg/utils" + "go.uber.org/multierr" + "go.uber.org/zap" + "golang.org/x/sync/errgroup" ) const ( diff --git a/br/pkg/lightning/errormanager/errormanager_test.go b/br/pkg/lightning/errormanager/errormanager_test.go index 38f81b51f0299..88808e35628b8 100644 --- a/br/pkg/lightning/errormanager/errormanager_test.go +++ b/br/pkg/lightning/errormanager/errormanager_test.go @@ -25,12 +25,11 @@ import ( "testing" "github.com/DATA-DOG/go-sqlmock" + "github.com/pingcap/tidb/br/pkg/lightning/config" "github.com/pingcap/tidb/br/pkg/lightning/log" + "github.com/pingcap/tidb/br/pkg/utils" "github.com/stretchr/testify/require" "go.uber.org/atomic" - - "github.com/pingcap/tidb/br/pkg/lightning/config" - "github.com/pingcap/tidb/br/pkg/utils" ) func TestInit(t *testing.T) { diff --git a/br/pkg/lightning/mydump/charset_convertor.go b/br/pkg/lightning/mydump/charset_convertor.go index 53a77816bb5e3..fa5f1ee5ef540 100644 --- a/br/pkg/lightning/mydump/charset_convertor.go +++ b/br/pkg/lightning/mydump/charset_convertor.go @@ -19,10 +19,9 @@ import ( "unicode/utf8" "github.com/pingcap/errors" + "github.com/pingcap/tidb/br/pkg/lightning/config" "golang.org/x/text/encoding" "golang.org/x/text/encoding/simplifiedchinese" - - "github.com/pingcap/tidb/br/pkg/lightning/config" ) // CharsetConvertor is used to convert a character set to utf8mb4 encoding. diff --git a/br/pkg/lightning/mydump/router.go b/br/pkg/lightning/mydump/router.go index c3a6ff3aae161..98ecd4c63e08a 100644 --- a/br/pkg/lightning/mydump/router.go +++ b/br/pkg/lightning/mydump/router.go @@ -7,12 +7,11 @@ import ( "strings" "github.com/pingcap/errors" + "github.com/pingcap/tidb/br/pkg/lightning/config" + "github.com/pingcap/tidb/br/pkg/lightning/log" "github.com/pingcap/tidb/util/filter" "github.com/pingcap/tidb/util/slice" "go.uber.org/zap" - - "github.com/pingcap/tidb/br/pkg/lightning/config" - "github.com/pingcap/tidb/br/pkg/lightning/log" ) type SourceType int diff --git a/br/pkg/lightning/restore/restore_schema_test.go b/br/pkg/lightning/restore/restore_schema_test.go index 7bce917eb828c..d7a585f0c10a3 100644 --- a/br/pkg/lightning/restore/restore_schema_test.go +++ b/br/pkg/lightning/restore/restore_schema_test.go @@ -23,10 +23,6 @@ import ( "github.com/DATA-DOG/go-sqlmock" "github.com/golang/mock/gomock" "github.com/pingcap/errors" - filter "github.com/pingcap/tidb/util/table-filter" - "github.com/stretchr/testify/require" - "github.com/stretchr/testify/suite" - "github.com/pingcap/tidb/br/pkg/lightning/backend" "github.com/pingcap/tidb/br/pkg/lightning/checkpoints" "github.com/pingcap/tidb/br/pkg/lightning/config" @@ -39,6 +35,9 @@ import ( "github.com/pingcap/tidb/parser/model" "github.com/pingcap/tidb/parser/mysql" tmock "github.com/pingcap/tidb/util/mock" + filter "github.com/pingcap/tidb/util/table-filter" + "github.com/stretchr/testify/require" + "github.com/stretchr/testify/suite" ) type restoreSchemaSuite struct { diff --git a/br/pkg/restore/rawkv_client_test.go b/br/pkg/restore/rawkv_client_test.go index 1864aca1b0a77..1458ce04ff822 100644 --- a/br/pkg/restore/rawkv_client_test.go +++ b/br/pkg/restore/rawkv_client_test.go @@ -12,10 +12,9 @@ import ( berrors "github.com/pingcap/tidb/br/pkg/errors" "github.com/pingcap/tidb/br/pkg/restore" "github.com/pingcap/tidb/kv" + "github.com/pingcap/tidb/util/codec" "github.com/stretchr/testify/require" "github.com/tikv/client-go/v2/rawkv" - - "github.com/pingcap/tidb/util/codec" ) // fakeRawkvClient is a mock for rawkv.client diff --git a/br/pkg/storage/azblob.go b/br/pkg/storage/azblob.go index 08f25cbe06bf1..24c3b06930025 100644 --- a/br/pkg/storage/azblob.go +++ b/br/pkg/storage/azblob.go @@ -12,15 +12,14 @@ import ( "path" "strings" - "github.com/google/uuid" - "github.com/spf13/pflag" - "github.com/Azure/azure-sdk-for-go/sdk/azidentity" "github.com/Azure/azure-sdk-for-go/sdk/storage/azblob" + "github.com/google/uuid" "github.com/pingcap/errors" backuppb "github.com/pingcap/kvproto/pkg/brpb" "github.com/pingcap/log" berrors "github.com/pingcap/tidb/br/pkg/errors" + "github.com/spf13/pflag" "go.uber.org/zap" ) diff --git a/br/pkg/storage/compress.go b/br/pkg/storage/compress.go index 34e4796b34810..fc0976420996b 100644 --- a/br/pkg/storage/compress.go +++ b/br/pkg/storage/compress.go @@ -8,7 +8,6 @@ import ( "io" "github.com/pingcap/errors" - berrors "github.com/pingcap/tidb/br/pkg/errors" ) diff --git a/br/pkg/storage/s3.go b/br/pkg/storage/s3.go index b75da76505123..3bdcf7cab4d2b 100644 --- a/br/pkg/storage/s3.go +++ b/br/pkg/storage/s3.go @@ -27,11 +27,10 @@ import ( "github.com/pingcap/errors" backuppb "github.com/pingcap/kvproto/pkg/brpb" "github.com/pingcap/log" - "github.com/spf13/pflag" - "go.uber.org/zap" - berrors "github.com/pingcap/tidb/br/pkg/errors" "github.com/pingcap/tidb/br/pkg/logutil" + "github.com/spf13/pflag" + "go.uber.org/zap" ) const ( diff --git a/br/pkg/stream/client.go b/br/pkg/stream/client.go index 5eadee73af36d..3ae88605ca7f1 100644 --- a/br/pkg/stream/client.go +++ b/br/pkg/stream/client.go @@ -10,14 +10,13 @@ import ( "github.com/gogo/protobuf/proto" "github.com/pingcap/errors" - "github.com/pingcap/log" - "go.uber.org/zap" - backuppb "github.com/pingcap/kvproto/pkg/brpb" + "github.com/pingcap/log" berrors "github.com/pingcap/tidb/br/pkg/errors" "github.com/pingcap/tidb/br/pkg/redact" "github.com/pingcap/tidb/kv" clientv3 "go.etcd.io/etcd/client/v3" + "go.uber.org/zap" ) // MetaDataClient is the client for operations over metadata. diff --git a/br/pkg/stream/integration_test.go b/br/pkg/stream/integration_test.go index cbb253b1fcc60..92a465172afec 100644 --- a/br/pkg/stream/integration_test.go +++ b/br/pkg/stream/integration_test.go @@ -10,10 +10,9 @@ import ( "net/url" "testing" - berrors "github.com/pingcap/tidb/br/pkg/errors" - "github.com/pingcap/errors" "github.com/pingcap/log" + berrors "github.com/pingcap/tidb/br/pkg/errors" "github.com/pingcap/tidb/br/pkg/logutil" "github.com/pingcap/tidb/br/pkg/storage" "github.com/pingcap/tidb/br/pkg/stream" diff --git a/br/pkg/task/stream_test.go b/br/pkg/task/stream_test.go index 3b13f8657c0e2..3313351132585 100644 --- a/br/pkg/task/stream_test.go +++ b/br/pkg/task/stream_test.go @@ -25,7 +25,6 @@ import ( "github.com/pingcap/errors" backuppb "github.com/pingcap/kvproto/pkg/brpb" - "github.com/pingcap/kvproto/pkg/metapb" "github.com/pingcap/tidb/br/pkg/conn" "github.com/pingcap/tidb/br/pkg/pdutil" diff --git a/build/BUILD.bazel b/build/BUILD.bazel index ea8958f219b75..215244513658f 100644 --- a/build/BUILD.bazel +++ b/build/BUILD.bazel @@ -124,6 +124,7 @@ nogo( "//build/linter/durationcheck:durationcheck", "//build/linter/exportloopref:exportloopref", "//build/linter/gofmt:gofmt", + "//build/linter/gci:gci", "//build/linter/ineffassign:ineffassign", "//build/linter/misspell:misspell", "//build/linter/prealloc:prealloc", diff --git a/build/linter/gci/BUILD.bazel b/build/linter/gci/BUILD.bazel new file mode 100644 index 0000000000000..9318955d516bd --- /dev/null +++ b/build/linter/gci/BUILD.bazel @@ -0,0 +1,13 @@ +load("@io_bazel_rules_go//go:def.bzl", "go_library") + +go_library( + name = "gci", + srcs = ["analysis.go"], + importpath = "github.com/pingcap/tidb/build/linter/gci", + visibility = ["//visibility:public"], + deps = [ + "@com_github_daixiang0_gci//pkg/configuration", + "@com_github_daixiang0_gci//pkg/gci", + "@org_golang_x_tools//go/analysis", + ], +) diff --git a/build/linter/gci/analysis.go b/build/linter/gci/analysis.go new file mode 100644 index 0000000000000..6ac0854302160 --- /dev/null +++ b/build/linter/gci/analysis.go @@ -0,0 +1,65 @@ +// Copyright 2022 PingCAP, Inc. +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package gci + +import ( + "fmt" + "sync" + + "github.com/daixiang0/gci/pkg/configuration" + "github.com/daixiang0/gci/pkg/gci" + "golang.org/x/tools/go/analysis" +) + +// Analyzer is the analyzer struct of gci. +var Analyzer = &analysis.Analyzer{ + Name: "gci", + Doc: "Gci controls golang package import order and makes it always deterministic.", + Run: run, +} + +func run(pass *analysis.Pass) (any, error) { + fileNames := make([]string, 0, len(pass.Files)) + for _, f := range pass.Files { + pos := pass.Fset.PositionFor(f.Pos(), false) + fileNames = append(fileNames, pos.Filename) + } + var rawCfg gci.GciStringConfiguration + rawCfg.Cfg = configuration.FormatterConfiguration{ + NoInlineComments: false, + NoPrefixComments: false, + Debug: false, + } + cfg, _ := rawCfg.Parse() + var diffs []string + var lock sync.Mutex + err := gci.DiffFormattedFilesToArray(fileNames, *cfg, &diffs, &lock) + if err != nil { + return nil, err + } + + for _, diff := range diffs { + if diff == "" { + continue + } + + pass.Report(analysis.Diagnostic{ + Pos: 1, + Message: fmt.Sprintf("\n%s", diff), + }) + } + + return nil, nil +} diff --git a/build/nogo_config.json b/build/nogo_config.json index b5edb72788f4f..bd890a8f55832 100644 --- a/build/nogo_config.json +++ b/build/nogo_config.json @@ -142,6 +142,18 @@ ".*failpoint_binding__.go$": "ignore generated code" } }, + "gci": { + "exclude_files": { + "/external/": "no need to vet third party code", + ".*_generated\\.go$": "ignore generated code", + "/cgo/": "ignore cgo code", + ".*\\.pb\\.go$": "generated code", + "/rules_go_work-*": "ignore generated code", + ".*test_/testmain\\.go$": "ignore generated code", + ".*failpoint_binding__.go$": "ignore generated code", + "util/printer/printer.go": "ignore util/printer code" + } + }, "httpresponse": { "exclude_files": { "/external/": "no need to vet third party code", diff --git a/cmd/benchraw/main.go b/cmd/benchraw/main.go index 3bcce7dec621c..80f1a1d2289bc 100644 --- a/cmd/benchraw/main.go +++ b/cmd/benchraw/main.go @@ -30,7 +30,6 @@ import ( "github.com/pingcap/tidb/parser/terror" "github.com/tikv/client-go/v2/config" "github.com/tikv/client-go/v2/rawkv" - "go.uber.org/zap" ) diff --git a/ddl/serial_test.go b/ddl/serial_test.go index a588b8a16169a..7d5c2c55a2d89 100644 --- a/ddl/serial_test.go +++ b/ddl/serial_test.go @@ -28,7 +28,6 @@ import ( "github.com/pingcap/tidb/config" "github.com/pingcap/tidb/ddl" "github.com/pingcap/tidb/ddl/util" - ddlutil "github.com/pingcap/tidb/ddl/util" "github.com/pingcap/tidb/domain" "github.com/pingcap/tidb/errno" "github.com/pingcap/tidb/infoschema" @@ -726,7 +725,7 @@ func TestCancelJobByErrorCountLimit(t *testing.T) { limit := variable.GetDDLErrorCountLimit() tk.MustExec("set @@global.tidb_ddl_error_count_limit = 16") - err := ddlutil.LoadDDLVars(tk.Session()) + err := util.LoadDDLVars(tk.Session()) require.NoError(t, err) defer tk.MustExec(fmt.Sprintf("set @@global.tidb_ddl_error_count_limit = %d", limit)) @@ -744,7 +743,7 @@ func TestTruncateTableUpdateSchemaVersionErr(t *testing.T) { limit := variable.GetDDLErrorCountLimit() tk.MustExec("set @@global.tidb_ddl_error_count_limit = 5") - err := ddlutil.LoadDDLVars(tk.Session()) + err := util.LoadDDLVars(tk.Session()) require.NoError(t, err) defer tk.MustExec(fmt.Sprintf("set @@global.tidb_ddl_error_count_limit = %d", limit)) diff --git a/ddl/table_split_test.go b/ddl/table_split_test.go index 299abe09fb686..ad944215a6550 100644 --- a/ddl/table_split_test.go +++ b/ddl/table_split_test.go @@ -26,10 +26,8 @@ import ( "github.com/pingcap/tidb/store/mockstore" "github.com/pingcap/tidb/tablecodec" "github.com/pingcap/tidb/testkit" - - "github.com/tikv/client-go/v2/tikv" - "github.com/stretchr/testify/require" + "github.com/tikv/client-go/v2/tikv" ) func TestTableSplit(t *testing.T) { diff --git a/distsql/select_result_test.go b/distsql/select_result_test.go index 622e672e1ee9d..c12892083d641 100644 --- a/distsql/select_result_test.go +++ b/distsql/select_result_test.go @@ -24,7 +24,6 @@ import ( "github.com/pingcap/tidb/util/execdetails" "github.com/pingcap/tidb/util/mock" "github.com/pingcap/tipb/go-tipb" - "github.com/stretchr/testify/require" ) diff --git a/dumpling/cli/versions.go b/dumpling/cli/versions.go index 2d29dd1296bd6..ad55c6ae9539f 100644 --- a/dumpling/cli/versions.go +++ b/dumpling/cli/versions.go @@ -16,9 +16,8 @@ package cli import ( "fmt" - "go.uber.org/zap" - "github.com/pingcap/tidb/dumpling/log" + "go.uber.org/zap" ) var ( diff --git a/dumpling/cmd/dumpling/main.go b/dumpling/cmd/dumpling/main.go index 3d72df1e694ea..5e4b8d8f0521b 100644 --- a/dumpling/cmd/dumpling/main.go +++ b/dumpling/cmd/dumpling/main.go @@ -18,13 +18,12 @@ import ( "fmt" "os" + "github.com/pingcap/tidb/dumpling/cli" + "github.com/pingcap/tidb/dumpling/export" "github.com/prometheus/client_golang/prometheus" "github.com/prometheus/client_golang/prometheus/collectors" "github.com/spf13/pflag" "go.uber.org/zap" - - "github.com/pingcap/tidb/dumpling/cli" - "github.com/pingcap/tidb/dumpling/export" ) func main() { diff --git a/dumpling/export/block_allow_list.go b/dumpling/export/block_allow_list.go index 2ce925abe02da..a3d4852b50c65 100644 --- a/dumpling/export/block_allow_list.go +++ b/dumpling/export/block_allow_list.go @@ -3,9 +3,8 @@ package export import ( - "go.uber.org/zap" - tcontext "github.com/pingcap/tidb/dumpling/context" + "go.uber.org/zap" ) func filterDatabases(tctx *tcontext.Context, conf *Config, databases []string) []string { diff --git a/dumpling/export/block_allow_list_test.go b/dumpling/export/block_allow_list_test.go index 640e9cef7e4dc..28faa4e95f261 100644 --- a/dumpling/export/block_allow_list_test.go +++ b/dumpling/export/block_allow_list_test.go @@ -6,12 +6,11 @@ import ( "strings" "testing" + "github.com/pingcap/tidb/br/pkg/version" + tcontext "github.com/pingcap/tidb/dumpling/context" "github.com/pingcap/tidb/util/filter" tf "github.com/pingcap/tidb/util/table-filter" "github.com/stretchr/testify/require" - - "github.com/pingcap/tidb/br/pkg/version" - tcontext "github.com/pingcap/tidb/dumpling/context" ) func TestFilterTables(t *testing.T) { diff --git a/dumpling/export/config.go b/dumpling/export/config.go index b74799c39d8d2..d9fa184643415 100644 --- a/dumpling/export/config.go +++ b/dumpling/export/config.go @@ -17,15 +17,14 @@ import ( "github.com/docker/go-units" "github.com/go-sql-driver/mysql" "github.com/pingcap/errors" - "github.com/prometheus/client_golang/prometheus" - "github.com/spf13/pflag" - "go.uber.org/zap" - "github.com/pingcap/tidb/br/pkg/storage" "github.com/pingcap/tidb/br/pkg/version" "github.com/pingcap/tidb/util" "github.com/pingcap/tidb/util/promutil" filter "github.com/pingcap/tidb/util/table-filter" + "github.com/prometheus/client_golang/prometheus" + "github.com/spf13/pflag" + "go.uber.org/zap" ) const ( diff --git a/dumpling/export/conn.go b/dumpling/export/conn.go index c6723865b7003..c981febe19450 100644 --- a/dumpling/export/conn.go +++ b/dumpling/export/conn.go @@ -6,10 +6,9 @@ import ( "database/sql" "github.com/pingcap/errors" - "go.uber.org/zap" - "github.com/pingcap/tidb/br/pkg/utils" tcontext "github.com/pingcap/tidb/dumpling/context" + "go.uber.org/zap" ) // BaseConn wraps connection instance. diff --git a/dumpling/export/consistency.go b/dumpling/export/consistency.go index 2af2abaa7d19c..6e0f65960f275 100644 --- a/dumpling/export/consistency.go +++ b/dumpling/export/consistency.go @@ -7,7 +7,6 @@ import ( "database/sql" "github.com/pingcap/errors" - "github.com/pingcap/tidb/br/pkg/utils" "github.com/pingcap/tidb/br/pkg/version" tcontext "github.com/pingcap/tidb/dumpling/context" diff --git a/dumpling/export/consistency_test.go b/dumpling/export/consistency_test.go index cd81d27db11f5..97f94a16a0334 100644 --- a/dumpling/export/consistency_test.go +++ b/dumpling/export/consistency_test.go @@ -10,11 +10,10 @@ import ( "github.com/DATA-DOG/go-sqlmock" "github.com/go-sql-driver/mysql" - "github.com/stretchr/testify/require" - "github.com/pingcap/tidb/br/pkg/version" dbconfig "github.com/pingcap/tidb/config" tcontext "github.com/pingcap/tidb/dumpling/context" + "github.com/stretchr/testify/require" ) func TestConsistencyController(t *testing.T) { diff --git a/dumpling/export/dump.go b/dumpling/export/dump.go index 43d801430e6e8..50a55646e6967 100755 --- a/dumpling/export/dump.go +++ b/dumpling/export/dump.go @@ -20,10 +20,6 @@ import ( "github.com/pingcap/errors" "github.com/pingcap/failpoint" pclog "github.com/pingcap/log" - pd "github.com/tikv/pd/client" - "go.uber.org/zap" - "golang.org/x/sync/errgroup" - "github.com/pingcap/tidb/br/pkg/storage" "github.com/pingcap/tidb/br/pkg/summary" "github.com/pingcap/tidb/br/pkg/version" @@ -36,6 +32,9 @@ import ( "github.com/pingcap/tidb/store/helper" "github.com/pingcap/tidb/tablecodec" "github.com/pingcap/tidb/util/codec" + pd "github.com/tikv/pd/client" + "go.uber.org/zap" + "golang.org/x/sync/errgroup" ) var openDBFunc = sql.Open diff --git a/dumpling/export/dump_test.go b/dumpling/export/dump_test.go index b059a1ae28ac2..51e522bf4824b 100644 --- a/dumpling/export/dump_test.go +++ b/dumpling/export/dump_test.go @@ -9,15 +9,13 @@ import ( "time" "github.com/DATA-DOG/go-sqlmock" - "github.com/pingcap/tidb/util/promutil" - "github.com/stretchr/testify/require" - "golang.org/x/sync/errgroup" - "github.com/pingcap/errors" - "github.com/pingcap/tidb/br/pkg/version" tcontext "github.com/pingcap/tidb/dumpling/context" "github.com/pingcap/tidb/parser" + "github.com/pingcap/tidb/util/promutil" + "github.com/stretchr/testify/require" + "golang.org/x/sync/errgroup" ) func TestDumpBlock(t *testing.T) { diff --git a/dumpling/export/http_handler.go b/dumpling/export/http_handler.go index 91332736f17f2..ca861dd11e8bc 100644 --- a/dumpling/export/http_handler.go +++ b/dumpling/export/http_handler.go @@ -10,11 +10,10 @@ import ( "time" "github.com/pingcap/errors" - "github.com/prometheus/client_golang/prometheus/promhttp" - "github.com/soheilhy/cmux" - tcontext "github.com/pingcap/tidb/dumpling/context" "github.com/pingcap/tidb/dumpling/log" + "github.com/prometheus/client_golang/prometheus/promhttp" + "github.com/soheilhy/cmux" ) var cmuxReadTimeout = 10 * time.Second diff --git a/dumpling/export/ir.go b/dumpling/export/ir.go index b74d7dc1dfef3..4b98019605e9c 100644 --- a/dumpling/export/ir.go +++ b/dumpling/export/ir.go @@ -8,7 +8,6 @@ import ( "strings" "github.com/pingcap/errors" - tcontext "github.com/pingcap/tidb/dumpling/context" ) diff --git a/dumpling/export/ir_impl.go b/dumpling/export/ir_impl.go index 57a14f5a66d11..d51462b59ab01 100644 --- a/dumpling/export/ir_impl.go +++ b/dumpling/export/ir_impl.go @@ -7,9 +7,8 @@ import ( "strings" "github.com/pingcap/errors" - "go.uber.org/zap" - tcontext "github.com/pingcap/tidb/dumpling/context" + "go.uber.org/zap" ) // rowIter implements the SQLRowIter interface. diff --git a/dumpling/export/metadata.go b/dumpling/export/metadata.go index d4ffa3c7aa3d2..7d2cf53128688 100644 --- a/dumpling/export/metadata.go +++ b/dumpling/export/metadata.go @@ -11,11 +11,10 @@ import ( "time" "github.com/pingcap/errors" - "go.uber.org/zap" - "github.com/pingcap/tidb/br/pkg/storage" "github.com/pingcap/tidb/br/pkg/version" tcontext "github.com/pingcap/tidb/dumpling/context" + "go.uber.org/zap" ) type globalMetadata struct { diff --git a/dumpling/export/metadata_test.go b/dumpling/export/metadata_test.go index dc67c73628b87..90099bbf00acd 100644 --- a/dumpling/export/metadata_test.go +++ b/dumpling/export/metadata_test.go @@ -10,11 +10,10 @@ import ( "testing" "github.com/DATA-DOG/go-sqlmock" - "github.com/stretchr/testify/require" - "github.com/pingcap/tidb/br/pkg/storage" "github.com/pingcap/tidb/br/pkg/version" tcontext "github.com/pingcap/tidb/dumpling/context" + "github.com/stretchr/testify/require" ) const ( diff --git a/dumpling/export/prepare.go b/dumpling/export/prepare.go index 1d9d05304c7cf..0878ce31f8666 100644 --- a/dumpling/export/prepare.go +++ b/dumpling/export/prepare.go @@ -10,7 +10,6 @@ import ( "text/template" "github.com/pingcap/errors" - tcontext "github.com/pingcap/tidb/dumpling/context" ) diff --git a/dumpling/export/prepare_test.go b/dumpling/export/prepare_test.go index ad326a412c4a3..f9f559448d078 100644 --- a/dumpling/export/prepare_test.go +++ b/dumpling/export/prepare_test.go @@ -9,9 +9,8 @@ import ( "testing" "github.com/DATA-DOG/go-sqlmock" - "github.com/stretchr/testify/require" - tcontext "github.com/pingcap/tidb/dumpling/context" + "github.com/stretchr/testify/require" ) func TestPrepareDumpingDatabases(t *testing.T) { diff --git a/dumpling/export/retry.go b/dumpling/export/retry.go index f2bd998df8ccb..ad49f46b76c65 100644 --- a/dumpling/export/retry.go +++ b/dumpling/export/retry.go @@ -8,11 +8,10 @@ import ( "github.com/go-sql-driver/mysql" "github.com/pingcap/errors" - "github.com/pingcap/tidb/util/dbutil" - "go.uber.org/zap" - "github.com/pingcap/tidb/br/pkg/utils" tcontext "github.com/pingcap/tidb/dumpling/context" + "github.com/pingcap/tidb/util/dbutil" + "go.uber.org/zap" ) const ( diff --git a/dumpling/export/sql.go b/dumpling/export/sql.go index 9f866209241b5..ad78158bb27a8 100644 --- a/dumpling/export/sql.go +++ b/dumpling/export/sql.go @@ -17,9 +17,6 @@ import ( "github.com/go-sql-driver/mysql" "github.com/pingcap/errors" "github.com/pingcap/failpoint" - "go.uber.org/multierr" - "go.uber.org/zap" - "github.com/pingcap/tidb/br/pkg/version" dbconfig "github.com/pingcap/tidb/config" tcontext "github.com/pingcap/tidb/dumpling/context" @@ -27,6 +24,8 @@ import ( "github.com/pingcap/tidb/errno" "github.com/pingcap/tidb/parser/model" "github.com/pingcap/tidb/store/helper" + "go.uber.org/multierr" + "go.uber.org/zap" ) const ( diff --git a/dumpling/export/sql_test.go b/dumpling/export/sql_test.go index a752df681c146..d79f74993c10b 100644 --- a/dumpling/export/sql_test.go +++ b/dumpling/export/sql_test.go @@ -15,16 +15,14 @@ import ( "strings" "testing" - "github.com/go-sql-driver/mysql" - "github.com/pingcap/tidb/util/promutil" - "github.com/DATA-DOG/go-sqlmock" + "github.com/go-sql-driver/mysql" "github.com/pingcap/errors" - "github.com/stretchr/testify/require" - "github.com/pingcap/tidb/br/pkg/version" dbconfig "github.com/pingcap/tidb/config" tcontext "github.com/pingcap/tidb/dumpling/context" + "github.com/pingcap/tidb/util/promutil" + "github.com/stretchr/testify/require" ) var showIndexHeaders = []string{ diff --git a/dumpling/export/status.go b/dumpling/export/status.go index 4b7e706898724..e7d708bb49f2f 100644 --- a/dumpling/export/status.go +++ b/dumpling/export/status.go @@ -8,9 +8,8 @@ import ( "time" "github.com/docker/go-units" - "go.uber.org/zap" - tcontext "github.com/pingcap/tidb/dumpling/context" + "go.uber.org/zap" ) const logProgressTick = 2 * time.Minute diff --git a/dumpling/export/util.go b/dumpling/export/util.go index 1506a424946f3..5d43a1cf5a1d9 100644 --- a/dumpling/export/util.go +++ b/dumpling/export/util.go @@ -10,10 +10,9 @@ import ( "time" "github.com/pingcap/errors" - clientv3 "go.etcd.io/etcd/client/v3" - "github.com/pingcap/tidb/br/pkg/version" tcontext "github.com/pingcap/tidb/dumpling/context" + clientv3 "go.etcd.io/etcd/client/v3" ) const tidbServerInformationPath = "/tidb/server/info" diff --git a/dumpling/export/util_for_test.go b/dumpling/export/util_for_test.go index 67d7f62f9fcfb..739a431b2230e 100644 --- a/dumpling/export/util_for_test.go +++ b/dumpling/export/util_for_test.go @@ -9,7 +9,6 @@ import ( "fmt" "github.com/DATA-DOG/go-sqlmock" - tcontext "github.com/pingcap/tidb/dumpling/context" ) diff --git a/dumpling/export/util_test.go b/dumpling/export/util_test.go index 5932dcc9888f4..1686a24902825 100644 --- a/dumpling/export/util_test.go +++ b/dumpling/export/util_test.go @@ -6,9 +6,8 @@ import ( "fmt" "testing" - "github.com/stretchr/testify/require" - "github.com/pingcap/tidb/br/pkg/version" + "github.com/stretchr/testify/require" ) func TestRepeatableRead(t *testing.T) { diff --git a/dumpling/export/writer.go b/dumpling/export/writer.go index 06ba5a1605b29..d3545aa2e4f18 100644 --- a/dumpling/export/writer.go +++ b/dumpling/export/writer.go @@ -10,11 +10,10 @@ import ( "text/template" "github.com/pingcap/errors" - "go.uber.org/zap" - "github.com/pingcap/tidb/br/pkg/storage" "github.com/pingcap/tidb/br/pkg/utils" tcontext "github.com/pingcap/tidb/dumpling/context" + "go.uber.org/zap" ) // Writer is the abstraction that keep pulling data from database and write to files. diff --git a/dumpling/export/writer_serial_test.go b/dumpling/export/writer_serial_test.go index d08015cb9d0c0..2290ca86cdfa2 100644 --- a/dumpling/export/writer_serial_test.go +++ b/dumpling/export/writer_serial_test.go @@ -9,11 +9,10 @@ import ( "testing" "github.com/pingcap/errors" - "github.com/pingcap/tidb/util/promutil" - "github.com/stretchr/testify/require" - "github.com/pingcap/tidb/br/pkg/storage" tcontext "github.com/pingcap/tidb/dumpling/context" + "github.com/pingcap/tidb/util/promutil" + "github.com/stretchr/testify/require" ) func TestWriteMeta(t *testing.T) { diff --git a/dumpling/export/writer_test.go b/dumpling/export/writer_test.go index 93fde0ebdd5e4..4192a86179163 100644 --- a/dumpling/export/writer_test.go +++ b/dumpling/export/writer_test.go @@ -12,10 +12,9 @@ import ( "testing" "github.com/DATA-DOG/go-sqlmock" + tcontext "github.com/pingcap/tidb/dumpling/context" "github.com/pingcap/tidb/util/promutil" "github.com/stretchr/testify/require" - - tcontext "github.com/pingcap/tidb/dumpling/context" ) func TestWriteDatabaseMeta(t *testing.T) { diff --git a/dumpling/export/writer_util.go b/dumpling/export/writer_util.go index d3aa417d550b8..c43a1d140cab4 100755 --- a/dumpling/export/writer_util.go +++ b/dumpling/export/writer_util.go @@ -13,13 +13,12 @@ import ( "github.com/pingcap/errors" "github.com/pingcap/failpoint" - "github.com/prometheus/client_golang/prometheus" - "go.uber.org/zap" - "github.com/pingcap/tidb/br/pkg/storage" "github.com/pingcap/tidb/br/pkg/summary" tcontext "github.com/pingcap/tidb/dumpling/context" "github.com/pingcap/tidb/dumpling/log" + "github.com/prometheus/client_golang/prometheus" + "go.uber.org/zap" ) const lengthLimit = 1048576 diff --git a/executor/benchmark_test.go b/executor/benchmark_test.go index d843da3b3d415..bba2c176d2877 100644 --- a/executor/benchmark_test.go +++ b/executor/benchmark_test.go @@ -34,7 +34,6 @@ import ( "github.com/pingcap/tidb/parser/ast" "github.com/pingcap/tidb/parser/mysql" "github.com/pingcap/tidb/planner/core" - plannercore "github.com/pingcap/tidb/planner/core" "github.com/pingcap/tidb/planner/property" "github.com/pingcap/tidb/planner/util" "github.com/pingcap/tidb/sessionctx" @@ -567,8 +566,8 @@ func buildWindowExecutor(ctx sessionctx.Context, windowFunc string, funcs int, f plan = core.PhysicalShuffle{ Concurrency: concurrency, - Tails: []plannercore.PhysicalPlan{tail}, - DataSources: []plannercore.PhysicalPlan{src}, + Tails: []core.PhysicalPlan{tail}, + DataSources: []core.PhysicalPlan{src}, SplitterType: core.PartitionHashSplitterType, ByItemArrays: [][]expression.Expression{byItems}, }.Init(ctx, nil, 0) diff --git a/executor/opt_rule_blacklist.go b/executor/opt_rule_blacklist.go index 2b80adecd4383..f711d2feacb03 100644 --- a/executor/opt_rule_blacklist.go +++ b/executor/opt_rule_blacklist.go @@ -18,7 +18,6 @@ import ( "context" "github.com/pingcap/tidb/kv" - plannercore "github.com/pingcap/tidb/planner/core" "github.com/pingcap/tidb/sessionctx" "github.com/pingcap/tidb/util/chunk" diff --git a/executor/simple.go b/executor/simple.go index 5568ab5c5b4d9..a7794d4fb8498 100644 --- a/executor/simple.go +++ b/executor/simple.go @@ -39,7 +39,6 @@ import ( "github.com/pingcap/tidb/parser/model" "github.com/pingcap/tidb/parser/mysql" "github.com/pingcap/tidb/planner/core" - plannercore "github.com/pingcap/tidb/planner/core" "github.com/pingcap/tidb/plugin" "github.com/pingcap/tidb/privilege" "github.com/pingcap/tidb/sessionctx" @@ -953,13 +952,13 @@ func (e *SimpleExec) executeAlterUser(ctx context.Context, s *ast.AlterUserStmt) // For simplicity: RESTRICTED_USER_ADMIN also counts for SYSTEM_USER here. if !(hasCreateUserPriv || hasSystemSchemaPriv) { - return plannercore.ErrSpecificAccessDenied.GenWithStackByArgs("CREATE USER") + return core.ErrSpecificAccessDenied.GenWithStackByArgs("CREATE USER") } if checker.RequestDynamicVerificationWithUser("SYSTEM_USER", false, spec.User) && !(hasSystemUserPriv || hasRestrictedUserPriv) { - return plannercore.ErrSpecificAccessDenied.GenWithStackByArgs("SYSTEM_USER or SUPER") + return core.ErrSpecificAccessDenied.GenWithStackByArgs("SYSTEM_USER or SUPER") } if sem.IsEnabled() && checker.RequestDynamicVerificationWithUser("RESTRICTED_USER_ADMIN", false, spec.User) && !hasRestrictedUserPriv { - return plannercore.ErrSpecificAccessDenied.GenWithStackByArgs("RESTRICTED_USER_ADMIN") + return core.ErrSpecificAccessDenied.GenWithStackByArgs("RESTRICTED_USER_ADMIN") } } @@ -1270,7 +1269,7 @@ func (e *SimpleExec) executeDropUser(ctx context.Context, s *ast.DropUserStmt) e if _, err := sqlExecutor.ExecuteInternal(internalCtx, "rollback"); err != nil { return err } - return plannercore.ErrSpecificAccessDenied.GenWithStackByArgs("SYSTEM_USER or SUPER") + return core.ErrSpecificAccessDenied.GenWithStackByArgs("SYSTEM_USER or SUPER") } // begin a transaction to delete a user. @@ -1738,7 +1737,7 @@ func (e *SimpleExec) executeAdminFlushPlanCache(s *ast.AdminStmt) error { if s.StatementScope == ast.StatementScopeGlobal { return errors.New("Do not support the 'admin flush global scope.'") } - if !plannercore.PreparedPlanCacheEnabled() { + if !core.PreparedPlanCacheEnabled() { e.ctx.GetSessionVars().StmtCtx.AppendWarning(errors.New("The plan cache is disable. So there no need to flush the plan cache")) return nil } diff --git a/expression/constant_test.go b/expression/constant_test.go index 50e5c5ce98ecc..e118fdd6a125a 100644 --- a/expression/constant_test.go +++ b/expression/constant_test.go @@ -21,14 +21,13 @@ import ( "testing" "time" - "github.com/stretchr/testify/require" - "github.com/pingcap/tidb/parser/ast" "github.com/pingcap/tidb/parser/mysql" "github.com/pingcap/tidb/types" "github.com/pingcap/tidb/types/json" "github.com/pingcap/tidb/util/chunk" "github.com/pingcap/tidb/util/mock" + "github.com/stretchr/testify/require" ) func newColumn(id int) *Column { diff --git a/expression/helper_test.go b/expression/helper_test.go index beb69528b02cf..ed75ca82f2c25 100644 --- a/expression/helper_test.go +++ b/expression/helper_test.go @@ -20,8 +20,6 @@ import ( "testing" "time" - "github.com/stretchr/testify/require" - "github.com/pingcap/tidb/parser/ast" "github.com/pingcap/tidb/parser/charset" "github.com/pingcap/tidb/parser/model" @@ -30,6 +28,7 @@ import ( "github.com/pingcap/tidb/types" driver "github.com/pingcap/tidb/types/parser_driver" "github.com/pingcap/tidb/util/mock" + "github.com/stretchr/testify/require" ) func TestGetTimeValue(t *testing.T) { diff --git a/go.mod b/go.mod index 64211d111e21b..fdf5368b379f2 100644 --- a/go.mod +++ b/go.mod @@ -98,6 +98,7 @@ require ( require ( github.com/aliyun/alibaba-cloud-sdk-go v1.61.1581 github.com/charithe/durationcheck v0.0.9 + github.com/daixiang0/gci v0.3.4 github.com/golangci/gofmt v0.0.0-20190930125516-244bba706f1a github.com/golangci/misspell v0.3.5 github.com/golangci/unconvert v0.0.0-20180507085042-28b1c447d1f4 @@ -109,7 +110,10 @@ require ( honnef.co/go/tools v0.0.1-2020.1.4 ) -require github.com/kisielk/gotool v1.0.0 // indirect +require ( + github.com/hexops/gotextdiff v1.0.3 // indirect + github.com/kisielk/gotool v1.0.0 // indirect +) require ( cloud.google.com/go v0.100.2 // indirect diff --git a/go.sum b/go.sum index efdf0b627e904..5d700817a2652 100644 --- a/go.sum +++ b/go.sum @@ -193,6 +193,8 @@ github.com/cznic/mathutil v0.0.0-20181122101859-297441e03548 h1:iwZdTE0PVqJCos1v github.com/cznic/mathutil v0.0.0-20181122101859-297441e03548/go.mod h1:e6NPNENfs9mPDVNRekM7lKScauxd5kXTr1Mfyig6TDM= github.com/cznic/sortutil v0.0.0-20181122101858-f5f958428db8/go.mod h1:q2w6Bg5jeox1B+QkJ6Wp/+Vn0G/bo3f1uY7Fn3vivIQ= github.com/cznic/strutil v0.0.0-20171016134553-529a34b1c186/go.mod h1:AHHPPPXTw0h6pVabbcbyGRK1DckRn7r/STdZEeIDzZc= +github.com/daixiang0/gci v0.3.4 h1:+EZ83znNs73C9ZBTM7xhNagMP6gJs5wlptiFiuce5BM= +github.com/daixiang0/gci v0.3.4/go.mod h1:pB1j339Q+2sv/EyKd4dgvGXcaBGIErim+dlhLDtqeW4= github.com/danjacques/gofslock v0.0.0-20191023191349-0a45f885bc37 h1:X6mKGhCFOxrKeeHAjv/3UvT6e5RRxW6wRdlqlV6/H4w= github.com/danjacques/gofslock v0.0.0-20191023191349-0a45f885bc37/go.mod h1:DC3JtzuG7kxMvJ6dZmf2ymjNyoXwgtklr7FN+Um2B0U= github.com/davecgh/go-spew v1.1.0/go.mod h1:J7Y8YcW2NihsgmVo/mv3lAwl/skON4iLHjSsI+c5H38= @@ -450,6 +452,8 @@ github.com/hashicorp/logutils v1.0.0/go.mod h1:QIAnNjmIWmVIIkWDTG1z5v++HQmx9WQRO github.com/hashicorp/mdns v1.0.0/go.mod h1:tL+uN++7HEJ6SQLQ2/p+z2pH24WQKWjBPkE0mNTz8vQ= github.com/hashicorp/memberlist v0.1.3/go.mod h1:ajVTdAv/9Im8oMAAj5G31PhhMCZJV2pPBoIllUwCN7I= github.com/hashicorp/serf v0.8.2/go.mod h1:6hOLApaqBFA1NXqRQAsxw9QxuDEvNxSQRwA/JwenrHc= +github.com/hexops/gotextdiff v1.0.3 h1:gitA9+qJrrTCsiCl7+kh75nPqQt1cx4ZkudSTLoUqJM= +github.com/hexops/gotextdiff v1.0.3/go.mod h1:pSWU5MAI3yDq+fZBTazCSJysOMbxWL1BSow5/V2vxeg= github.com/hpcloud/tail v1.0.0/go.mod h1:ab1qPbhIpdTxEkNHXyeSf5vhxWSCs/tWer42PpOxQnU= github.com/hydrogen18/memlistener v0.0.0-20141126152155-54553eb933fb/go.mod h1:qEIFzExnS6016fRpRfxrExeVn2gbClQA99gQhnIcdhE= github.com/iancoleman/strcase v0.2.0 h1:05I4QRnGpI0m37iZQRuskXh+w77mr6Z41lwQzuHLwW0= diff --git a/infoschema/tables.go b/infoschema/tables.go index 622eb1ef9452c..b71646f1f0043 100644 --- a/infoschema/tables.go +++ b/infoschema/tables.go @@ -27,19 +27,15 @@ import ( "github.com/pingcap/errors" "github.com/pingcap/failpoint" "github.com/pingcap/kvproto/pkg/metapb" - "github.com/pingcap/tidb/parser/charset" - "github.com/pingcap/tidb/parser/model" - "github.com/pingcap/tidb/parser/mysql" - "github.com/pingcap/tidb/parser/terror" - "github.com/pingcap/tidb/util/logutil" - "github.com/pingcap/tidb/util/stmtsummary" - "go.uber.org/zap" - "github.com/pingcap/tidb/config" "github.com/pingcap/tidb/ddl/placement" "github.com/pingcap/tidb/domain/infosync" "github.com/pingcap/tidb/kv" "github.com/pingcap/tidb/meta/autoid" + "github.com/pingcap/tidb/parser/charset" + "github.com/pingcap/tidb/parser/model" + "github.com/pingcap/tidb/parser/mysql" + "github.com/pingcap/tidb/parser/terror" "github.com/pingcap/tidb/session/txninfo" "github.com/pingcap/tidb/sessionctx" "github.com/pingcap/tidb/sessionctx/variable" @@ -48,8 +44,11 @@ import ( "github.com/pingcap/tidb/util" "github.com/pingcap/tidb/util/deadlockhistory" "github.com/pingcap/tidb/util/execdetails" + "github.com/pingcap/tidb/util/logutil" "github.com/pingcap/tidb/util/pdapi" + "github.com/pingcap/tidb/util/stmtsummary" "github.com/tikv/client-go/v2/tikv" + "go.uber.org/zap" ) const ( diff --git a/owner/manager_test.go b/owner/manager_test.go index 3668c4d060ad2..24627750d855d 100644 --- a/owner/manager_test.go +++ b/owner/manager_test.go @@ -16,7 +16,6 @@ package owner_test import ( "context" - goctx "context" "fmt" "runtime" "testing" @@ -54,7 +53,7 @@ func TestSingle(t *testing.T) { defer cluster.Terminate(t) client := cluster.RandClient() - ctx := goctx.Background() + ctx := context.Background() ic := infoschema.NewCache(2) ic.Insert(infoschema.MockInfoSchemaWithSchemaVer(nil, 0), 0) d := NewDDL( @@ -69,13 +68,13 @@ func TestSingle(t *testing.T) { require.True(t, isOwner) // test for newSession failed - ctx, cancel := goctx.WithCancel(ctx) + ctx, cancel := context.WithCancel(ctx) manager := owner.NewOwnerManager(ctx, client, "ddl", "ddl_id", DDLOwnerKey) cancel() err = manager.CampaignOwner() comment := fmt.Sprintf("campaigned result don't match, err %v", err) - require.True(t, terror.ErrorEqual(err, goctx.Canceled) || terror.ErrorEqual(err, goctx.DeadlineExceeded), comment) + require.True(t, terror.ErrorEqual(err, context.Canceled) || terror.ErrorEqual(err, context.DeadlineExceeded), comment) isOwner = checkOwner(d, true) require.True(t, isOwner) @@ -88,7 +87,7 @@ func TestSingle(t *testing.T) { time.Sleep(200 * time.Millisecond) // err is ok to be not nil since we canceled the manager. - ownerID, _ := manager.GetOwnerID(goctx.Background()) + ownerID, _ := manager.GetOwnerID(context.Background()) require.Equal(t, "", ownerID) } @@ -118,7 +117,7 @@ func TestCluster(t *testing.T) { ic := infoschema.NewCache(2) ic.Insert(infoschema.MockInfoSchemaWithSchemaVer(nil, 0), 0) d := NewDDL( - goctx.Background(), + context.Background(), WithEtcdClient(cli), WithStore(store), WithLease(testLease), @@ -136,7 +135,7 @@ func TestCluster(t *testing.T) { ic2 := infoschema.NewCache(2) ic2.Insert(infoschema.MockInfoSchemaWithSchemaVer(nil, 0), 0) d1 := NewDDL( - goctx.Background(), + context.Background(), WithEtcdClient(cli1), WithStore(store), WithLease(testLease), @@ -163,7 +162,7 @@ func TestCluster(t *testing.T) { ic3 := infoschema.NewCache(2) ic3.Insert(infoschema.MockInfoSchemaWithSchemaVer(nil, 0), 0) d3 := NewDDL( - goctx.Background(), + context.Background(), WithEtcdClient(cli3), WithStore(store), WithLease(testLease), @@ -186,7 +185,7 @@ func TestCluster(t *testing.T) { election := concurrency.NewElection(session, DDLOwnerKey) logPrefix := fmt.Sprintf("[ddl] %s ownerManager %s", DDLOwnerKey, "useless id") logCtx := logutil.WithKeyValue(context.Background(), "owner info", logPrefix) - _, err = owner.GetOwnerInfo(goctx.Background(), logCtx, election, "useless id") + _, err = owner.GetOwnerInfo(context.Background(), logCtx, election, "useless id") require.Truef(t, terror.ErrorEqual(err, concurrency.ErrElectionNoLeader), "get owner info result don't match, err %v", err) } @@ -213,10 +212,10 @@ func deleteLeader(cli *clientv3.Client, prefixKey string) error { _ = session.Close() }() election := concurrency.NewElection(session, prefixKey) - resp, err := election.Leader(goctx.Background()) + resp, err := election.Leader(context.Background()) if err != nil { return errors.Trace(err) } - _, err = cli.Delete(goctx.Background(), string(resp.Kvs[0].Key)) + _, err = cli.Delete(context.Background(), string(resp.Kvs[0].Key)) return errors.Trace(err) } diff --git a/parser/ast/ddl_test.go b/parser/ast/ddl_test.go index beb3a27b97370..25af0b1fe0485 100644 --- a/parser/ast/ddl_test.go +++ b/parser/ast/ddl_test.go @@ -16,10 +16,9 @@ package ast_test import ( "testing" - "github.com/stretchr/testify/require" - . "github.com/pingcap/tidb/parser/ast" "github.com/pingcap/tidb/parser/format" + "github.com/stretchr/testify/require" ) func TestDDLVisitorCover(t *testing.T) { diff --git a/parser/charset/encoding_latin1.go b/parser/charset/encoding_latin1.go index db7b66ed101af..38f9bb601ac4e 100644 --- a/parser/charset/encoding_latin1.go +++ b/parser/charset/encoding_latin1.go @@ -15,6 +15,7 @@ package charset import ( "bytes" + "golang.org/x/text/encoding" ) diff --git a/parser/hintparser_test.go b/parser/hintparser_test.go index 5c252b2d4af78..2c9156f579128 100644 --- a/parser/hintparser_test.go +++ b/parser/hintparser_test.go @@ -16,12 +16,11 @@ package parser_test import ( "testing" - "github.com/stretchr/testify/require" - "github.com/pingcap/tidb/parser" "github.com/pingcap/tidb/parser/ast" "github.com/pingcap/tidb/parser/model" "github.com/pingcap/tidb/parser/mysql" + "github.com/stretchr/testify/require" ) func TestParseHint(t *testing.T) { diff --git a/parser/types/field_type_test.go b/parser/types/field_type_test.go index e258e75787313..6310dbb102bc9 100644 --- a/parser/types/field_type_test.go +++ b/parser/types/field_type_test.go @@ -21,11 +21,10 @@ import ( "github.com/pingcap/tidb/parser/ast" "github.com/pingcap/tidb/parser/charset" "github.com/pingcap/tidb/parser/mysql" - . "github.com/pingcap/tidb/parser/types" - "github.com/stretchr/testify/require" - // import parser_driver _ "github.com/pingcap/tidb/parser/test_driver" + . "github.com/pingcap/tidb/parser/types" + "github.com/stretchr/testify/require" ) func TestFieldType(t *testing.T) { diff --git a/planner/core/fragment_test.go b/planner/core/fragment_test.go index fa8ec9e99763c..ee01ddf8133c4 100644 --- a/planner/core/fragment_test.go +++ b/planner/core/fragment_test.go @@ -15,10 +15,10 @@ package core import ( + "testing" + "github.com/pingcap/tipb/go-tipb" "github.com/stretchr/testify/require" - - "testing" ) func TestFragmentInitSingleton(t *testing.T) { diff --git a/planner/core/planbuilder_test.go b/planner/core/planbuilder_test.go index e793d51a68f23..13494433f0fbe 100644 --- a/planner/core/planbuilder_test.go +++ b/planner/core/planbuilder_test.go @@ -21,7 +21,6 @@ import ( "strings" "testing" "unsafe" - _ "unsafe" // required by go:linkname "github.com/pingcap/errors" "github.com/pingcap/tidb/expression" diff --git a/server/server.go b/server/server.go index 63406905b5a3b..dde54399a7cd9 100644 --- a/server/server.go +++ b/server/server.go @@ -36,8 +36,7 @@ import ( "io" "math/rand" "net" - "net/http" - + "net/http" //nolint:goimports // For pprof _ "net/http/pprof" // #nosec G108 "os" diff --git a/session/session.go b/session/session.go index efaddb884b0f5..256d8255724f7 100644 --- a/session/session.go +++ b/session/session.go @@ -41,29 +41,6 @@ import ( "github.com/pingcap/errors" "github.com/pingcap/failpoint" "github.com/pingcap/kvproto/pkg/kvrpcpb" - "github.com/pingcap/tidb/parser" - "github.com/pingcap/tidb/parser/ast" - "github.com/pingcap/tidb/parser/auth" - "github.com/pingcap/tidb/parser/charset" - "github.com/pingcap/tidb/parser/model" - "github.com/pingcap/tidb/parser/mysql" - "github.com/pingcap/tidb/parser/terror" - "github.com/pingcap/tidb/sessionctx/sessionstates" - "github.com/pingcap/tidb/sessiontxn" - "github.com/pingcap/tidb/sessiontxn/staleread" - "github.com/pingcap/tidb/store/driver/txn" - "github.com/pingcap/tidb/store/helper" - "github.com/pingcap/tidb/table" - "github.com/pingcap/tidb/table/temptable" - "github.com/pingcap/tidb/util/logutil/consistency" - "github.com/pingcap/tidb/util/sem" - "github.com/pingcap/tidb/util/topsql" - topsqlstate "github.com/pingcap/tidb/util/topsql/state" - "github.com/pingcap/tidb/util/topsql/stmtstats" - "github.com/pingcap/tipb/go-binlog" - tikverr "github.com/tikv/client-go/v2/error" - "go.uber.org/zap" - "github.com/pingcap/tidb/bindinfo" "github.com/pingcap/tidb/config" "github.com/pingcap/tidb/ddl/placement" @@ -75,6 +52,13 @@ import ( "github.com/pingcap/tidb/meta" "github.com/pingcap/tidb/metrics" "github.com/pingcap/tidb/owner" + "github.com/pingcap/tidb/parser" + "github.com/pingcap/tidb/parser/ast" + "github.com/pingcap/tidb/parser/auth" + "github.com/pingcap/tidb/parser/charset" + "github.com/pingcap/tidb/parser/model" + "github.com/pingcap/tidb/parser/mysql" + "github.com/pingcap/tidb/parser/terror" "github.com/pingcap/tidb/planner" plannercore "github.com/pingcap/tidb/planner/core" "github.com/pingcap/tidb/plugin" @@ -83,11 +67,18 @@ import ( "github.com/pingcap/tidb/session/txninfo" "github.com/pingcap/tidb/sessionctx" "github.com/pingcap/tidb/sessionctx/binloginfo" + "github.com/pingcap/tidb/sessionctx/sessionstates" "github.com/pingcap/tidb/sessionctx/stmtctx" "github.com/pingcap/tidb/sessionctx/variable" + "github.com/pingcap/tidb/sessiontxn" + "github.com/pingcap/tidb/sessiontxn/staleread" "github.com/pingcap/tidb/statistics" "github.com/pingcap/tidb/statistics/handle" storeerr "github.com/pingcap/tidb/store/driver/error" + "github.com/pingcap/tidb/store/driver/txn" + "github.com/pingcap/tidb/store/helper" + "github.com/pingcap/tidb/table" + "github.com/pingcap/tidb/table/temptable" "github.com/pingcap/tidb/tablecodec" "github.com/pingcap/tidb/telemetry" "github.com/pingcap/tidb/types" @@ -98,14 +89,22 @@ import ( "github.com/pingcap/tidb/util/execdetails" "github.com/pingcap/tidb/util/kvcache" "github.com/pingcap/tidb/util/logutil" + "github.com/pingcap/tidb/util/logutil/consistency" + "github.com/pingcap/tidb/util/sem" "github.com/pingcap/tidb/util/sli" "github.com/pingcap/tidb/util/sqlexec" "github.com/pingcap/tidb/util/tableutil" "github.com/pingcap/tidb/util/timeutil" + "github.com/pingcap/tidb/util/topsql" + topsqlstate "github.com/pingcap/tidb/util/topsql/state" + "github.com/pingcap/tidb/util/topsql/stmtstats" + "github.com/pingcap/tipb/go-binlog" + tikverr "github.com/tikv/client-go/v2/error" tikvstore "github.com/tikv/client-go/v2/kv" "github.com/tikv/client-go/v2/oracle" "github.com/tikv/client-go/v2/tikv" tikvutil "github.com/tikv/client-go/v2/util" + "go.uber.org/zap" ) var ( diff --git a/sessiontxn/isolation/optimistic_test.go b/sessiontxn/isolation/optimistic_test.go index 134833c8823e8..d6b4d9b3b7d26 100644 --- a/sessiontxn/isolation/optimistic_test.go +++ b/sessiontxn/isolation/optimistic_test.go @@ -22,9 +22,8 @@ import ( "testing" "time" - "github.com/pingcap/tidb/config" - "github.com/pingcap/kvproto/pkg/kvrpcpb" + "github.com/pingcap/tidb/config" "github.com/pingcap/tidb/infoschema" "github.com/pingcap/tidb/kv" "github.com/pingcap/tidb/parser" diff --git a/store/mockstore/unistore/tikv/deadlock.go b/store/mockstore/unistore/tikv/deadlock.go index 90f0c5b226a56..7eeb1fb2c5b64 100644 --- a/store/mockstore/unistore/tikv/deadlock.go +++ b/store/mockstore/unistore/tikv/deadlock.go @@ -20,14 +20,13 @@ import ( "sync/atomic" "time" - "go.uber.org/zap" - "google.golang.org/grpc" - deadlockPb "github.com/pingcap/kvproto/pkg/deadlock" "github.com/pingcap/log" "github.com/pingcap/tidb/store/mockstore/unistore/pd" "github.com/pingcap/tidb/store/mockstore/unistore/tikv/kverrors" "github.com/pingcap/tidb/store/mockstore/unistore/util/lockwaiter" + "go.uber.org/zap" + "google.golang.org/grpc" ) // Follower will send detection rpc to Leader diff --git a/store/mockstore/unistore/tikv/mvcc/mvcc.go b/store/mockstore/unistore/tikv/mvcc/mvcc.go index a8ae16a9321d6..b7185766bfeeb 100644 --- a/store/mockstore/unistore/tikv/mvcc/mvcc.go +++ b/store/mockstore/unistore/tikv/mvcc/mvcc.go @@ -19,7 +19,6 @@ import ( "unsafe" "github.com/pingcap/kvproto/pkg/kvrpcpb" - "github.com/pingcap/tidb/util/codec" ) diff --git a/tidb-binlog/pump_client/client_test.go b/tidb-binlog/pump_client/client_test.go index 866c91693ff27..ac287773072ed 100644 --- a/tidb-binlog/pump_client/client_test.go +++ b/tidb-binlog/pump_client/client_test.go @@ -24,8 +24,7 @@ import ( "github.com/pingcap/errors" "github.com/pingcap/tidb/tidb-binlog/node" - binlog "github.com/pingcap/tipb/go-binlog" - pb "github.com/pingcap/tipb/go-binlog" + "github.com/pingcap/tipb/go-binlog" "github.com/stretchr/testify/require" "google.golang.org/grpc" ) @@ -36,7 +35,7 @@ var ( ) type testCase struct { - binlogs []*pb.Binlog + binlogs []*binlog.Binlog choosePumps []*PumpStatus setAvliable []bool setNodeID []string @@ -61,7 +60,7 @@ func testSelector(t *testing.T, strategy string) { pump.NodeID = fmt.Sprintf("pump%d", i) pump.State = node.Offline // set pump client to avoid create grpc client. - pump.Client = pb.NewPumpClient(nil) + pump.Client = binlog.NewPumpClient(nil) } for _, pump := range pumps { @@ -71,26 +70,26 @@ func testSelector(t *testing.T, strategy string) { tCase := &testCase{} - tCase.binlogs = []*pb.Binlog{ + tCase.binlogs = []*binlog.Binlog{ { - Tp: pb.BinlogType_Prewrite, + Tp: binlog.BinlogType_Prewrite, StartTs: 1, }, { - Tp: pb.BinlogType_Commit, + Tp: binlog.BinlogType_Commit, StartTs: 1, CommitTs: 2, }, { - Tp: pb.BinlogType_Prewrite, + Tp: binlog.BinlogType_Prewrite, StartTs: 3, }, { - Tp: pb.BinlogType_Commit, + Tp: binlog.BinlogType_Commit, StartTs: 3, CommitTs: 4, }, { - Tp: pb.BinlogType_Prewrite, + Tp: binlog.BinlogType_Prewrite, StartTs: 5, }, { - Tp: pb.BinlogType_Commit, + Tp: binlog.BinlogType_Commit, StartTs: 5, CommitTs: 6, }, @@ -111,12 +110,12 @@ func testSelector(t *testing.T, strategy string) { } for j := 0; j < 10; j++ { - prewriteBinlog := &pb.Binlog{ - Tp: pb.BinlogType_Prewrite, + prewriteBinlog := &binlog.Binlog{ + Tp: binlog.BinlogType_Prewrite, StartTs: int64(j), } - commitBinlog := &pb.Binlog{ - Tp: pb.BinlogType_Commit, + commitBinlog := &binlog.Binlog{ + Tp: binlog.BinlogType_Commit, StartTs: int64(j), } @@ -180,20 +179,20 @@ func TestWriteBinlog(t *testing.T) { clientCon, err := grpc.Dial(cfg.addr, opt, grpc.WithInsecure()) require.NoError(t, err) require.NotNil(t, clientCon) - pumpClient := mockPumpsClient(pb.NewPumpClient(clientCon), true) + pumpClient := mockPumpsClient(binlog.NewPumpClient(clientCon), true) // test binlog size bigger than grpc's MaxRecvMsgSize - blog := &pb.Binlog{ - Tp: pb.BinlogType_Prewrite, + blog := &binlog.Binlog{ + Tp: binlog.BinlogType_Prewrite, PrewriteValue: make([]byte, testMaxRecvMsgSize+1), } err = pumpClient.WriteBinlog(blog) require.Error(t, err) for i := 0; i < 10; i++ { - // test binlog size small than grpc's MaxRecvMsgSize - blog = &pb.Binlog{ - Tp: pb.BinlogType_Prewrite, + // test binlog size smaller than grpc's MaxRecvMsgSize + blog = &binlog.Binlog{ + Tp: binlog.BinlogType_Prewrite, PrewriteValue: make([]byte, 1), } err = pumpClient.WriteBinlog(blog) @@ -204,13 +203,13 @@ func TestWriteBinlog(t *testing.T) { require.Len(t, pumpClient.Pumps.UnAvaliablePumps, 1) // test write commit binlog, will not return error although write binlog failed. - preWriteBinlog := &pb.Binlog{ - Tp: pb.BinlogType_Prewrite, + preWriteBinlog := &binlog.Binlog{ + Tp: binlog.BinlogType_Prewrite, StartTs: 123, PrewriteValue: make([]byte, 1), } - commitBinlog := &pb.Binlog{ - Tp: pb.BinlogType_Commit, + commitBinlog := &binlog.Binlog{ + Tp: binlog.BinlogType_Commit, StartTs: 123, CommitTs: 123, PrewriteValue: make([]byte, 1), @@ -284,14 +283,14 @@ func createMockPumpServer(addr string, mode string, withError bool) (*mockPumpSe server: serv, withError: withError, } - pb.RegisterPumpServer(serv, pump) + binlog.RegisterPumpServer(serv, pump) go serv.Serve(l) return pump, nil } // mockPumpsClient creates a PumpsClient, used for test. -func mockPumpsClient(client pb.PumpClient, withBadPump bool) *PumpsClient { +func mockPumpsClient(client binlog.PumpClient, withBadPump bool) *PumpsClient { // add a available pump nodeID1 := "pump-1" pump1 := &PumpStatus{ diff --git a/util/generatedexpr/gen_expr_test.go b/util/generatedexpr/gen_expr_test.go index 484453466e7eb..6b1d39f2c2490 100644 --- a/util/generatedexpr/gen_expr_test.go +++ b/util/generatedexpr/gen_expr_test.go @@ -18,9 +18,8 @@ import ( "testing" "github.com/pingcap/tidb/parser/ast" - "github.com/stretchr/testify/require" - _ "github.com/pingcap/tidb/types/parser_driver" + "github.com/stretchr/testify/require" ) func TestParseExpression(t *testing.T) { diff --git a/util/logutil/hex_test.go b/util/logutil/hex_test.go index c9ce8713fe7ea..9351b0216cf52 100644 --- a/util/logutil/hex_test.go +++ b/util/logutil/hex_test.go @@ -21,10 +21,9 @@ import ( "testing" "github.com/pingcap/kvproto/pkg/metapb" - "github.com/stretchr/testify/require" - "github.com/pingcap/tidb/kv" "github.com/pingcap/tidb/util/logutil" + "github.com/stretchr/testify/require" ) func TestHex(t *testing.T) { diff --git a/util/mock/iter_test.go b/util/mock/iter_test.go index 9e142973185ea..e99e9d17745cb 100644 --- a/util/mock/iter_test.go +++ b/util/mock/iter_test.go @@ -17,7 +17,6 @@ import ( "testing" "github.com/pingcap/tidb/kv" - "github.com/stretchr/testify/assert" ) diff --git a/util/sem/sem_test.go b/util/sem/sem_test.go index f7f8d20f6ee89..cb0c47ad9225d 100644 --- a/util/sem/sem_test.go +++ b/util/sem/sem_test.go @@ -19,7 +19,6 @@ import ( "github.com/pingcap/tidb/parser/mysql" "github.com/pingcap/tidb/sessionctx/variable" - "github.com/stretchr/testify/assert" )