From 949fa1ad0994450630c48f521916296dd49f800b Mon Sep 17 00:00:00 2001 From: pingcap-github-bot Date: Tue, 28 Jul 2020 22:22:03 +0800 Subject: [PATCH] *: fix a bug caused by the wrong collation setting which leads to the wrong result of collation function (#17116) (#17231) Signed-off-by: sre-bot --- executor/set.go | 2 +- expression/builtin_info.go | 1 + expression/integration_test.go | 46 +++------------------------------- session/session.go | 7 +----- sessionctx/variable/session.go | 21 +++++++++++++++- 5 files changed, 26 insertions(+), 51 deletions(-) diff --git a/executor/set.go b/executor/set.go index 3ecc8fba9a938..0fed40596cb11 100644 --- a/executor/set.go +++ b/executor/set.go @@ -253,7 +253,7 @@ func (e *SetExecutor) setCharset(cs, co string) error { return errors.Trace(err) } } - return errors.Trace(sessionVars.SetSystemVar(variable.CollationConnection, co)) + return sessionVars.SetSystemVar(variable.CollationConnection, co) } func (e *SetExecutor) getVarValue(v *expression.VarAssignment, sysVar *variable.SysVar) (value types.Datum, err error) { diff --git a/expression/builtin_info.go b/expression/builtin_info.go index 422829bd32053..db7050a81ed5e 100644 --- a/expression/builtin_info.go +++ b/expression/builtin_info.go @@ -673,6 +673,7 @@ func (c *collationFunctionClass) getFunction(ctx sessionctx.Context, args []Expr if err != nil { return nil, err } + bf.tp.Charset, bf.tp.Collate = ctx.GetSessionVars().GetCharsetInfo() sig := &builtinCollationSig{bf} return sig, nil } diff --git a/expression/integration_test.go b/expression/integration_test.go index 45516dd621534..65e52613dc5c1 100755 --- a/expression/integration_test.go +++ b/expression/integration_test.go @@ -6481,50 +6481,10 @@ func (s *testIntegrationSuite) TestIssue16697(c *C) { } } -func (s *testIntegrationSerialSuite) TestIssue17176(c *C) { - collate.SetNewCollationEnabledForTest(true) - defer collate.SetNewCollationEnabledForTest(false) - - tk := testkit.NewTestKit(c, s.store) - tk.MustExec("use test") - tk.MustExec("drop table if exists t") - tk.MustGetErrMsg("create table t(a enum('a', 'a ')) charset utf8 collate utf8_bin;", "[types:1291]Column 'a' has duplicated value 'a ' in ENUM") - tk.MustGetErrMsg("create table t(a enum('a', 'Á')) charset utf8 collate utf8_general_ci;", "[types:1291]Column 'a' has duplicated value 'Á' in ENUM") - tk.MustGetErrMsg("create table t(a enum('a', 'a ')) charset utf8mb4 collate utf8mb4_bin;", "[types:1291]Column 'a' has duplicated value 'a ' in ENUM") - tk.MustExec("create table t(a enum('a', 'A')) charset utf8 collate utf8_bin;") - tk.MustExec("drop table t;") - tk.MustExec("create table t3(a enum('a', 'A')) charset utf8mb4 collate utf8mb4_bin;") -} - -func (s *testIntegrationSuite) TestIndexedVirtualGeneratedColumnTruncate(c *C) { +func (s *testIntegrationSuite) TestIssue17115(c *C) { tk := testkit.NewTestKit(c, s.store) - tk.MustExec("use test") - tk.MustExec("drop table if exists t1") - tk.MustExec("create table t(a int, b tinyint as(a+100) unique key)") - tk.MustExec("insert ignore into t values(200, default)") - tk.MustExec("update t set a=1 where a=200") - tk.MustExec("admin check table t") - tk.MustExec("delete from t") - tk.MustExec("insert ignore into t values(200, default)") - tk.MustExec("admin check table t") - tk.MustExec("insert ignore into t values(200, default) on duplicate key update a=100") - tk.MustExec("admin check table t") - tk.MustExec("delete from t") - tk.MustExec("admin check table t") - - tk.MustExec("begin") - tk.MustExec("insert ignore into t values(200, default)") - tk.MustExec("update t set a=1 where a=200") - tk.MustExec("admin check table t") - tk.MustExec("delete from t") - tk.MustExec("insert ignore into t values(200, default)") - tk.MustExec("admin check table t") - tk.MustExec("insert ignore into t values(200, default) on duplicate key update a=100") - tk.MustExec("admin check table t") - tk.MustExec("delete from t") - tk.MustExec("admin check table t") - tk.MustExec("commit") - tk.MustExec("admin check table t") + tk.MustQuery("select collation(user());").Check(testkit.Rows("utf8mb4_bin")) + tk.MustQuery("select collation(compress('abc'));").Check(testkit.Rows("binary")) } func (s *testIntegrationSuite) TestIssue17287(c *C) { diff --git a/session/session.go b/session/session.go index acbae2cf08027..25b1e88f83062 100644 --- a/session/session.go +++ b/session/session.go @@ -318,12 +318,7 @@ func (s *session) SetCollation(coID int) error { for _, v := range variable.SetNamesVariables { terror.Log(s.sessionVars.SetSystemVar(v, cs)) } - err = s.sessionVars.SetSystemVar(variable.CollationConnection, co) - if err != nil { - // Some clients may use the unsupported collations, such as utf8mb4_0900_ai_ci, We shouldn't return error or use the ERROR level log. - logutil.BgLogger().Warn(err.Error()) - } - return nil + return s.sessionVars.SetSystemVar(variable.CollationConnection, co) } func (s *session) PreparedPlanCache() *kvcache.SimpleLRUCache { diff --git a/sessionctx/variable/session.go b/sessionctx/variable/session.go index 70e24677082e3..c7f3072770a1c 100644 --- a/sessionctx/variable/session.go +++ b/sessionctx/variable/session.go @@ -29,6 +29,7 @@ import ( "github.com/pingcap/errors" "github.com/pingcap/parser/ast" "github.com/pingcap/parser/auth" + "github.com/pingcap/parser/charset" "github.com/pingcap/parser/mysql" "github.com/pingcap/parser/terror" pumpcli "github.com/pingcap/tidb-tools/tidb-binlog/pump_client" @@ -1262,7 +1263,25 @@ func (s *SessionVars) SetSystemVar(name string, val string) error { s.MetricSchemaRangeDuration = tidbOptInt64(val, DefTiDBMetricSchemaRangeDuration) case CollationConnection, CollationDatabase, CollationServer: if _, err := collate.GetCollationByName(val); err != nil { - return errors.Trace(err) + var ok bool + var charsetVal string + var err2 error + if name == CollationConnection { + charsetVal, ok = s.systems[CharacterSetConnection] + } else if name == CollationDatabase { + charsetVal, ok = s.systems[CharsetDatabase] + } else { + // CollationServer + charsetVal, ok = s.systems[CharacterSetServer] + } + if !ok { + return err + } + val, err2 = charset.GetDefaultCollation(charsetVal) + if err2 != nil { + return err2 + } + logutil.BgLogger().Warn(err.Error()) } case TiDBSlowLogThreshold: atomic.StoreUint64(&config.GetGlobalConfig().Log.SlowThreshold, uint64(tidbOptInt64(val, logutil.DefaultSlowThreshold)))