sysvar: add parsing phase for setting 'init_connect' (#36865)

close #35324
pingcap · Aug 5, 2022 · 0a8611d · 0a8611d
1 parent 8da64ab
commit 0a8611d
Show file tree

Hide file tree

Showing 18 changed files with 37 additions and 17 deletions.
diff --git a/br/pkg/lightning/backend/local/BUILD.bazel b/br/pkg/lightning/backend/local/BUILD.bazel
@@ -92,6 +92,7 @@ go_test(
     embed = [":local"],
     flaky = True,
     deps = [
+        "//br/pkg/errors",
         "//br/pkg/lightning/backend",
         "//br/pkg/lightning/backend/kv",
         "//br/pkg/lightning/common",

diff --git a/ddl/util/BUILD.bazel b/ddl/util/BUILD.bazel
@@ -24,7 +24,6 @@ go_library(
         "@com_github_pingcap_errors//:errors",
         "@com_github_pingcap_failpoint//:failpoint",
         "@com_github_tikv_client_go_v2//tikvrpc",
-        "@io_etcd_go_etcd_api_v3//v3rpc/rpctypes",
         "@io_etcd_go_etcd_client_v3//:client",
         "@io_etcd_go_etcd_client_v3//concurrency",
         "@org_uber_go_atomic//:atomic",

diff --git a/executor/seqtest/BUILD.bazel b/executor/seqtest/BUILD.bazel
@@ -15,6 +15,7 @@ go_test(
         "//ddl/util",
         "//errno",
         "//executor",
+        "//expression",
         "//infoschema",
         "//kv",
         "//meta/autoid",
@@ -34,7 +35,6 @@ go_test(
         "//testkit",
         "//testkit/testsetup",
         "//testkit/testutil",
-        "//types",
         "//util",
         "//util/gcutil",
         "//util/kvcache",

diff --git a/executor/set_test.go b/executor/set_test.go
@@ -750,6 +750,11 @@ func TestSetVar(t *testing.T) {
 	tk.MustQuery("select @@session.tidb_enable_analyze_snapshot").Check(testkit.Rows("1"))
 	tk.MustExec("set session tidb_enable_analyze_snapshot = 0")
 	tk.MustQuery("select @@session.tidb_enable_analyze_snapshot").Check(testkit.Rows("0"))
+
+	// test variables `init_connect'
+	tk.MustGetErrCode("set global init_connect = '-1'", mysql.ErrWrongTypeForVar)
+	tk.MustGetErrCode("set global init_connect = 'invalidstring'", mysql.ErrWrongTypeForVar)
+	tk.MustExec("set global init_connect = 'select now(); select timestamp()'")
 }
 
 func TestGetSetNoopVars(t *testing.T) {

diff --git a/planner/core/BUILD.bazel b/planner/core/BUILD.bazel
@@ -31,6 +31,7 @@ go_library(
         "plan.go",
         "plan_cache.go",
         "plan_cost.go",
+        "plan_cost_detail.go",
         "plan_stats.go",
         "plan_to_pb.go",
         "planbuilder.go",
@@ -175,6 +176,7 @@ go_test(
         "partition_pruning_test.go",
         "physical_plan_test.go",
         "physical_plan_trace_test.go",
+        "plan_cost_detail_test.go",
         "plan_cost_test.go",
         "plan_stats_test.go",
         "plan_test.go",

diff --git a/server/BUILD.bazel b/server/BUILD.bazel
@@ -31,6 +31,7 @@ go_library(
         "//domain/infosync",
         "//errno",
         "//executor",
+        "//expression",
         "//infoschema",
         "//kv",
         "//meta",
@@ -148,6 +149,7 @@ go_test(
         "//domain/infosync",
         "//errno",
         "//executor",
+        "//expression",
         "//infoschema",
         "//kv",
         "//meta",

diff --git a/server/server_test.go b/server/server_test.go
@@ -2198,8 +2198,6 @@ func (cli *testServerClient) runTestInitConnect(t *testing.T) {
 		require.NoError(t, err)
 		require.Equal(t, "", a)
 		require.NoError(t, rows.Close())
-		// change the init-connect to invalid.
-		dbt.MustExec(`SET GLOBAL init_connect="invalidstring"`)
 	})
 	// set global init_connect to empty to avoid fail other tests
 	defer cli.runTests(t, func(config *mysql.Config) {
@@ -2212,10 +2210,8 @@ func (cli *testServerClient) runTestInitConnect(t *testing.T) {
 	db, err := sql.Open("mysql", cli.getDSN(func(config *mysql.Config) {
 		config.User = "init_nonsuper"
 	}))
-	require.NoError(t, err)      // doesn't fail because of lazy loading
-	defer db.Close()             // may already be closed
-	_, err = db.Exec("SELECT 1") // fails because of init sql
-	require.Error(t, err)
+	require.NoError(t, err) // doesn't fail because of lazy loading
+	defer db.Close()        // may already be closed
 }
 
 // Client errors are only incremented when using the TiDB Server protocol,

diff --git a/session/BUILD.bazel b/session/BUILD.bazel
@@ -128,6 +128,7 @@ go_test(
         "//domain",
         "//errno",
         "//executor",
+        "//expression",
         "//infoschema",
         "//kv",
         "//meta",

diff --git a/session/session_test/BUILD.bazel b/session/session_test/BUILD.bazel
@@ -14,6 +14,7 @@ go_test(
         "//config",
         "//domain",
         "//errno",
+        "//expression",
         "//kv",
         "//parser/ast",
         "//parser/auth",

diff --git a/sessionctx/sessionstates/BUILD.bazel b/sessionctx/sessionstates/BUILD.bazel
@@ -34,12 +34,12 @@ go_test(
     deps = [
         "//config",
         "//errno",
+        "//expression",
         "//parser/mysql",
         "//parser/terror",
         "//server",
         "//sessionctx/variable",
         "//testkit",
-        "//types",
         "//util",
         "//util/sem",
         "@com_github_pingcap_errors//:errors",

diff --git a/sessionctx/variable/BUILD.bazel b/sessionctx/variable/BUILD.bazel
@@ -35,6 +35,7 @@ go_library(
         "//sessionctx/variable/featuretag/concurrencyddl",
         "//tidb-binlog/pump_client",
         "//types",
+        "//types/parser_driver",
         "//util",
         "//util/chunk",
         "//util/collate",

diff --git a/sessionctx/variable/sysvar.go b/sessionctx/variable/sysvar.go
@@ -28,11 +28,13 @@ import (
 	"github.com/pingcap/tidb/config"
 	"github.com/pingcap/tidb/kv"
 	"github.com/pingcap/tidb/metrics"
+	"github.com/pingcap/tidb/parser"
 	"github.com/pingcap/tidb/parser/charset"
 	"github.com/pingcap/tidb/parser/mysql"
 	"github.com/pingcap/tidb/sessionctx/sessionstates"
 	"github.com/pingcap/tidb/sessionctx/stmtctx"
 	"github.com/pingcap/tidb/types"
+	_ "github.com/pingcap/tidb/types/parser_driver" // for parser driver
 	"github.com/pingcap/tidb/util/collate"
 	"github.com/pingcap/tidb/util/logutil"
 	"github.com/pingcap/tidb/util/mathutil"
@@ -450,7 +452,17 @@ var defaultSysVars = []*SysVar{
 
 	/* The system variables below have GLOBAL scope  */
 	{Scope: ScopeGlobal, Name: MaxPreparedStmtCount, Value: strconv.FormatInt(DefMaxPreparedStmtCount, 10), Type: TypeInt, MinValue: -1, MaxValue: 1048576},
-	{Scope: ScopeGlobal, Name: InitConnect, Value: ""},
+	{Scope: ScopeGlobal, Name: InitConnect, Value: "", Validation: func(vars *SessionVars, normalizedValue string, originalValue string, scope ScopeFlag) (string, error) {
+		p := parser.New()
+		p.SetSQLMode(vars.SQLMode)
+		p.SetParserConfig(vars.BuildParserConfig())
+		_, _, err := p.ParseSQL(normalizedValue)
+		if err != nil {
+			return normalizedValue, ErrWrongTypeForVar.GenWithStackByArgs(InitConnect)
+		}
+		return normalizedValue, nil
+	}},
+
 	/* TiDB specific variables */
 	{Scope: ScopeGlobal, Name: TiDBTSOClientBatchMaxWaitTime, Value: strconv.FormatFloat(DefTiDBTSOClientBatchMaxWaitTime, 'f', -1, 64), Type: TypeFloat, MinValue: 0, MaxValue: 10,
 		GetGlobal: func(sv *SessionVars) (string, error) {

diff --git a/sessiontxn/BUILD.bazel b/sessiontxn/BUILD.bazel
@@ -29,6 +29,7 @@ go_test(
     deps = [
         ":sessiontxn",
         "//domain",
+        "//expression",
         "//infoschema",
         "//kv",
         "//parser/ast",
@@ -42,7 +43,6 @@ go_test(
         "//testkit/testfork",
         "//testkit/testsetup",
         "//tests/realtikvtest",
-        "//types",
         "@com_github_pingcap_failpoint//:failpoint",
         "@com_github_stretchr_testify//require",
         "@com_github_tikv_client_go_v2//oracle",

diff --git a/sessiontxn/isolation/BUILD.bazel b/sessiontxn/isolation/BUILD.bazel
@@ -50,6 +50,7 @@ go_test(
         ":isolation",
         "//config",
         "//executor",
+        "//expression",
         "//infoschema",
         "//kv",
         "//parser",
@@ -61,7 +62,6 @@ go_test(
         "//testkit",
         "//testkit/testfork",
         "//testkit/testsetup",
-        "//types",
         "@com_github_pingcap_errors//:errors",
         "@com_github_pingcap_failpoint//:failpoint",
         "@com_github_pingcap_kvproto//pkg/kvrpcpb",

diff --git a/testkit/BUILD.bazel b/testkit/BUILD.bazel
@@ -16,6 +16,7 @@ go_library(
     deps = [
         "//ddl/schematracker",
         "//domain",
+        "//expression",
         "//kv",
         "//parser/ast",
         "//parser/terror",
@@ -24,7 +25,6 @@ go_library(
         "//sessionctx/variable",
         "//store/driver",
         "//store/mockstore",
-        "//types",
         "//util",
         "//util/breakpoint",
         "//util/sqlexec",

diff --git a/tests/realtikvtest/BUILD.bazel b/tests/realtikvtest/BUILD.bazel
@@ -10,15 +10,15 @@ go_library(
         "//domain",
         "//kv",
         "//session",
+        "//sessionctx/variable",
         "//store/driver",
         "//store/mockstore",
+        "//testkit",
         "//testkit/testmain",
         "//testkit/testsetup",
         "@com_github_stretchr_testify//require",
         "@com_github_tikv_client_go_v2//tikv",
         "@com_github_tikv_client_go_v2//txnkv/transaction",
-        "@io_etcd_go_etcd_client_v3//:client",
-        "@org_golang_google_grpc//:grpc",
         "@org_uber_go_goleak//:goleak",
     ],
 )
diff --git a/tests/realtikvtest/pessimistictest/BUILD.bazel b/tests/realtikvtest/pessimistictest/BUILD.bazel
@@ -11,6 +11,7 @@ go_test(
     deps = [
         "//config",
         "//domain",
+        "//expression",
         "//kv",
         "//parser",
         "//parser/auth",
@@ -24,7 +25,6 @@ go_test(
         "//tablecodec",
         "//testkit",
         "//tests/realtikvtest",
-        "//types",
         "//util/codec",
         "//util/deadlockhistory",
         "@com_github_pingcap_errors//:errors",

diff --git a/tests/realtikvtest/txntest/BUILD.bazel b/tests/realtikvtest/txntest/BUILD.bazel
@@ -11,12 +11,12 @@ go_test(
     ],
     flaky = True,
     deps = [
+        "//expression",
         "//kv",
         "//parser",
         "//session/txninfo",
         "//testkit",
         "//tests/realtikvtest",
-        "//types",
         "//util",
         "@com_github_pingcap_failpoint//:failpoint",
         "@com_github_stretchr_testify//require",