Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

sessionctx, executor: Add correctness check when set system variables #7117

Merged
merged 19 commits into from
Jul 24, 2018
Merged

sessionctx, executor: Add correctness check when set system variables #7117

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
19 commits
Select commit Hold shift + click to select a range
fa16c09
sessionctx, executor: Add correctness check when set system variables
spongedu Jul 20, 2018
394a7ad
Add tests
spongedu Jul 20, 2018
88a630a
Code format refine
spongedu Jul 20, 2018
ba9834f
fix spell
spongedu Jul 21, 2018
6070034
Take some more sysvars into check
spongedu Jul 21, 2018
87fc695
Merge remote-tracking branch 'upstream/master' into dc_0720
spongedu Jul 21, 2018
fb7929b
fix spell
spongedu Jul 21, 2018
128ffdb
Merge remote-tracking branch 'upstream/master' into dc_0720
spongedu Jul 22, 2018
1aed328
Merge remote-tracking branch 'upstream/master' into dc_0720
spongedu Jul 22, 2018
fba76e3
1. support set default 2. add more checks
spongedu Jul 22, 2018
71ff2ac
Fix build
spongedu Jul 23, 2018
65b88ae
Merge remote-tracking branch 'upstream/master' into dc_0720
spongedu Jul 23, 2018
bfd3b4a
Merge remote-tracking branch 'upstream/master' into dc_0720
spongedu Jul 23, 2018
ad2f279
Merge remote-tracking branch 'upstream/master' into dc_0720
spongedu Jul 23, 2018
3273f75
refine ValidateSetSystemVar
spongedu Jul 23, 2018
57961c6
return UnknownSystemVar error if sysvar not found
spongedu Jul 24, 2018
fa4f79e
Merge remote-tracking branch 'upstream/master' into dc_0720
spongedu Jul 24, 2018
ab814bd
Add check for time_zone
spongedu Jul 24, 2018
5ab0cb7
Merge branch 'master' into dc_0720
spongedu Jul 24, 2018
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
13 changes: 6 additions & 7 deletions executor/ddl_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@ import (
"github.com/pingcap/tidb/plan"
"github.com/pingcap/tidb/sessionctx/variable"
"github.com/pingcap/tidb/table"
"github.com/pingcap/tidb/terror"
"github.com/pingcap/tidb/types"
"github.com/pingcap/tidb/util/chunk"
"github.com/pingcap/tidb/util/testkit"
Expand Down Expand Up @@ -409,17 +410,15 @@ func (s *testSuite) TestSetDDLReorgWorkerCnt(c *C) {
c.Assert(variable.GetDDLReorgWorkerCounter(), Equals, int32(1))
tk.MustExec("set tidb_ddl_reorg_worker_cnt = 100")
c.Assert(variable.GetDDLReorgWorkerCounter(), Equals, int32(100))
tk.MustExec("set tidb_ddl_reorg_worker_cnt = invalid_val")
c.Assert(variable.GetDDLReorgWorkerCounter(), Equals, int32(variable.DefTiDBDDLReorgWorkerCount))
_, err := tk.Exec("set tidb_ddl_reorg_worker_cnt = invalid_val")
c.Assert(terror.ErrorEqual(err, variable.ErrWrongTypeForVar), IsTrue)
tk.MustExec("set tidb_ddl_reorg_worker_cnt = 100")
c.Assert(variable.GetDDLReorgWorkerCounter(), Equals, int32(100))
tk.MustExec("set tidb_ddl_reorg_worker_cnt = -1")
c.Assert(variable.GetDDLReorgWorkerCounter(), Equals, int32(variable.DefTiDBDDLReorgWorkerCount))
_, err = tk.Exec("set tidb_ddl_reorg_worker_cnt = -1")
c.Assert(terror.ErrorEqual(err, variable.ErrWrongValueForVar), IsTrue)

res := tk.MustQuery("select @@tidb_ddl_reorg_worker_cnt")
res.Check(testkit.Rows("-1"))
tk.MustExec("set tidb_ddl_reorg_worker_cnt = 100")
res = tk.MustQuery("select @@tidb_ddl_reorg_worker_cnt")
res := tk.MustQuery("select @@tidb_ddl_reorg_worker_cnt")
res.Check(testkit.Rows("100"))

res = tk.MustQuery("select @@global.tidb_ddl_reorg_worker_cnt")
Expand Down
85 changes: 75 additions & 10 deletions executor/set_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,9 @@ import (
. "github.com/pingcap/check"
"github.com/pingcap/tidb/sessionctx"
"github.com/pingcap/tidb/sessionctx/variable"
"github.com/pingcap/tidb/terror"
"github.com/pingcap/tidb/util/testkit"
"github.com/pingcap/tidb/util/testutil"
"golang.org/x/net/context"
)

Expand Down Expand Up @@ -88,16 +90,16 @@ func (s *testSuite) TestSetVar(c *C) {
// Set default
// {ScopeGlobal | ScopeSession, "low_priority_updates", "OFF"},
// For global var
tk.MustQuery(`select @@global.low_priority_updates;`).Check(testkit.Rows("OFF"))
tk.MustQuery(`select @@global.low_priority_updates;`).Check(testkit.Rows("0"))
tk.MustExec(`set @@global.low_priority_updates="ON";`)
tk.MustQuery(`select @@global.low_priority_updates;`).Check(testkit.Rows("ON"))
tk.MustQuery(`select @@global.low_priority_updates;`).Check(testkit.Rows("1"))
tk.MustExec(`set @@global.low_priority_updates=DEFAULT;`) // It will be set to compiled-in default value.
tk.MustQuery(`select @@global.low_priority_updates;`).Check(testkit.Rows("OFF"))
tk.MustQuery(`select @@global.low_priority_updates;`).Check(testkit.Rows("0"))
// For session
tk.MustQuery(`select @@session.low_priority_updates;`).Check(testkit.Rows("OFF"))
tk.MustQuery(`select @@session.low_priority_updates;`).Check(testkit.Rows("0"))
tk.MustExec(`set @@global.low_priority_updates="ON";`)
tk.MustExec(`set @@session.low_priority_updates=DEFAULT;`) // It will be set to global var value.
tk.MustQuery(`select @@session.low_priority_updates;`).Check(testkit.Rows("ON"))
tk.MustQuery(`select @@session.low_priority_updates;`).Check(testkit.Rows("1"))

// For mysql jdbc driver issue.
tk.MustQuery(`select @@session.tx_read_only;`).Check(testkit.Rows("0"))
Expand Down Expand Up @@ -212,15 +214,15 @@ func (s *testSuite) TestSetVar(c *C) {
tk.MustQuery("select @@session.tx_isolation").Check(testkit.Rows("READ-COMMITTED"))

tk.MustExec("set global avoid_temporal_upgrade = on")
tk.MustQuery(`select @@global.avoid_temporal_upgrade;`).Check(testkit.Rows("ON"))
tk.MustQuery(`select @@global.avoid_temporal_upgrade;`).Check(testkit.Rows("1"))
tk.MustExec("set @@global.avoid_temporal_upgrade = off")
tk.MustQuery(`select @@global.avoid_temporal_upgrade;`).Check(testkit.Rows("off"))
tk.MustQuery(`select @@global.avoid_temporal_upgrade;`).Check(testkit.Rows("0"))
tk.MustExec("set session sql_log_bin = on")
tk.MustQuery(`select @@session.sql_log_bin;`).Check(testkit.Rows("ON"))
tk.MustQuery(`select @@session.sql_log_bin;`).Check(testkit.Rows("1"))
tk.MustExec("set sql_log_bin = off")
tk.MustQuery(`select @@session.sql_log_bin;`).Check(testkit.Rows("off"))
tk.MustQuery(`select @@session.sql_log_bin;`).Check(testkit.Rows("0"))
tk.MustExec("set @@sql_log_bin = on")
tk.MustQuery(`select @@session.sql_log_bin;`).Check(testkit.Rows("ON"))
tk.MustQuery(`select @@session.sql_log_bin;`).Check(testkit.Rows("1"))
}

func (s *testSuite) TestSetCharset(c *C) {
Expand All @@ -247,3 +249,66 @@ func (s *testSuite) TestSetCharset(c *C) {
// Issue 1523
tk.MustExec(`SET NAMES binary`)
}

func (s *testSuite) TestValidateSetVar(c *C) {
tk := testkit.NewTestKit(c, s.store)

_, err := tk.Exec("set global tidb_distsql_scan_concurrency='fff';")
c.Assert(terror.ErrorEqual(err, variable.ErrWrongTypeForVar), IsTrue)

_, err = tk.Exec("set global tidb_distsql_scan_concurrency=-1;")
c.Assert(terror.ErrorEqual(err, variable.ErrWrongValueForVar), IsTrue)

_, err = tk.Exec("set @@tidb_distsql_scan_concurrency='fff';")
c.Assert(terror.ErrorEqual(err, variable.ErrWrongTypeForVar), IsTrue)

_, err = tk.Exec("set @@tidb_distsql_scan_concurrency=-1;")
c.Assert(terror.ErrorEqual(err, variable.ErrWrongValueForVar), IsTrue)

_, err = tk.Exec("set @@tidb_batch_delete='ok';")
c.Assert(terror.ErrorEqual(err, variable.ErrWrongValueForVar), IsTrue)

tk.MustExec("set @@tidb_batch_delete='On';")
tk.MustExec("set @@tidb_batch_delete='oFf';")
tk.MustExec("set @@tidb_batch_delete=1;")
tk.MustExec("set @@tidb_batch_delete=0;")

_, err = tk.Exec("set @@tidb_batch_delete=3;")
c.Assert(terror.ErrorEqual(err, variable.ErrWrongValueForVar), IsTrue)

_, err = tk.Exec("set @@tidb_mem_quota_mergejoin='tidb';")
c.Assert(terror.ErrorEqual(err, variable.ErrWrongValueForVar), IsTrue)

tk.MustExec("set @@group_concat_max_len=1")
tk.MustQuery("show warnings").Check(testutil.RowsWithSep("|", "Warning|1292|Truncated incorrect group_concat_max_len value: '1'"))
result := tk.MustQuery("select @@group_concat_max_len;")
result.Check(testkit.Rows("4"))

_, err = tk.Exec("set @@group_concat_max_len = 18446744073709551616")
c.Assert(terror.ErrorEqual(err, variable.ErrWrongTypeForVar), IsTrue)

// Test illegal type
_, err = tk.Exec("set @@group_concat_max_len='hello'")
c.Assert(terror.ErrorEqual(err, variable.ErrWrongTypeForVar), IsTrue)

tk.MustExec("set @@default_week_format=-1")
tk.MustQuery("show warnings").Check(testutil.RowsWithSep("|", "Warning|1292|Truncated incorrect default_week_format value: '-1'"))
result = tk.MustQuery("select @@default_week_format;")
result.Check(testkit.Rows("0"))

tk.MustExec("set @@default_week_format=9")
tk.MustQuery("show warnings").Check(testutil.RowsWithSep("|", "Warning|1292|Truncated incorrect default_week_format value: '9'"))
result = tk.MustQuery("select @@default_week_format;")
result.Check(testkit.Rows("7"))

_, err = tk.Exec("set @@error_count = 0")
c.Assert(terror.ErrorEqual(err, variable.ErrReadOnly), IsTrue)

_, err = tk.Exec("set @@warning_count = 0")
c.Assert(terror.ErrorEqual(err, variable.ErrReadOnly), IsTrue)

tk.MustExec("set time_zone='SySTeM'")
result = tk.MustQuery("select @@time_zone;")
result.Check(testkit.Rows("SYSTEM"))

}
12 changes: 9 additions & 3 deletions session/session.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -689,16 +689,22 @@ func (s *session) GetGlobalSysVar(name string) (string, error) {
}

// SetGlobalSysVar implements GlobalVarAccessor.SetGlobalSysVar interface.
func (s *session) SetGlobalSysVar(name string, value string) error {
func (s *session) SetGlobalSysVar(name, value string) error {
if name == variable.SQLModeVar {
value = mysql.FormatSQLModeStr(value)
if _, err := mysql.GetSQLMode(value); err != nil {
return errors.Trace(err)
}
}
var sVal string
var err error
sVal, err = variable.ValidateSetSystemVar(s.sessionVars, name, value)
if err != nil {
return errors.Trace(err)
}
sql := fmt.Sprintf(`REPLACE %s.%s VALUES ('%s', '%s');`,
mysql.SystemDB, mysql.GlobalVariablesTable, strings.ToLower(name), value)
_, _, err := s.ExecRestrictedSQL(s, sql)
mysql.SystemDB, mysql.GlobalVariablesTable, strings.ToLower(name), sVal)
_, _, err = s.ExecRestrictedSQL(s, sql)
return errors.Trace(err)
}

Expand Down
3 changes: 1 addition & 2 deletions sessionctx/variable/session.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -545,8 +545,7 @@ func (s *SessionVars) SetSystemVar(name string, val string) error {
case TiDBEnableTablePartition:
s.EnableTablePartition = TiDBOptOn(val)
case TiDBDDLReorgWorkerCount:
workerCnt := tidbOptPositiveInt32(val, DefTiDBDDLReorgWorkerCount)
SetDDLReorgWorkerCounter(int32(workerCnt))
SetDDLReorgWorkerCounter(int32(tidbOptPositiveInt32(val, DefTiDBDDLReorgWorkerCount)))
}
s.systems[name] = val
return nil
Expand Down
Loading