*: change default charset and collation from 'utf8 utf8_bin' to 'utf8mb4 utf8mb4_bin' #7965

winkyao · 2018-10-19T10:06:06Z

What problem does this PR solve?

Change TiDB default charset and collation to "utf8mb4 utf8mb4_bin", TiDB treat all the data as utf8mb4 actually, but the previous default charset is "utf8", insert the 4 bytes unicode string into TiDB will be ok, but if we use mysqldump to restore the data back into mysql, the charset will be utf8, and it will report an error ERROR 1366 (HY000): Incorrect string value: '\xF0\xA4\x8B\xAE' for column 'v' at row 1.

how it works?

change mysql.DefaultCharset from UTF8Charset to UTF8MB4Charset.
change mysql.DefaultCollationName from UTF8DefaultCollation to UTF8MB4DefaultCollation.
Find all places that used charset.CharsetUTF8 and CollationUTF8, modify them to charset.CharsetUTF8MB4 or mysql.DefaultCharset.

Then fix corresponding test cases.

Check List

Tests

exits test cases

Code changes

Has exported variable/fields change

Related changes

Need to update the documentation
Need to be included in the release note

…mb4 utf8mb4_bin'

shenli · 2018-10-19T10:55:37Z

types/field_type.go

@@ -365,8 +365,8 @@ func DefaultTypeForValue(value interface{}, tp *FieldType) {
 		// TODO: tp.Flen should be len(x) * 3 (max bytes length of CharsetUTF8)
 		tp.Flen = len(x)
 		tp.Decimal = UnspecifiedLength
-		tp.Charset = mysql.DefaultCharset
-		tp.Collate = mysql.DefaultCollationName
+		tp.Charset = charset.CharsetUTF8MB4


Can we use GetDefaultCharsetAndCollate?

This change makes sense to me for this PR. My PR will change it back though :)

gregwebs

LGTM. However, I think we are treading on very dangerous ground with change until we just make utf8mb4 the default: #7757

winkyao · 2018-10-22T03:11:40Z

@shenli PTAL

winkyao · 2018-11-05T09:20:22Z

The ci will be fixed after pingcap/parser#13 merged.

…arset_mb4

winkyao · 2018-11-08T12:23:49Z

/run-all-tests

winkyao · 2018-11-08T13:24:20Z

/run-all-tests -tidb-test=pr/646

winkyao · 2018-11-09T01:50:21Z

/run-integration-ddl-test -tidb-test=pr/646

winkyao · 2018-11-09T02:05:41Z

/run-sqllogic-test -tidb-test=pr/646

winkyao · 2018-11-09T02:14:35Z

/run-integration-ddl-test -tidb-test=pr/646

XuHuaiyu

LGTM

zz-jason · 2018-11-09T02:50:08Z

expression/aggregation/descriptor.go

-	a.RetTp.Charset = charset.CharsetUTF8
-	a.RetTp.Collate = charset.CollationUTF8
+	a.RetTp.Charset = charset.CharsetUTF8MB4
+	a.RetTp.Collate = charset.CollationUTF8MB4


s/charset.CharsetUTF8MB4/mysql.DefaultCharset/
s/charset.CollationUTF8MB4/mysql.DefaultCollationName/

or: charset.GetDefaultCharsetAndCollate()

zz-jason · 2018-11-09T02:51:10Z

expression/builtin.go

 			Flag:    mysql.BinaryFlag,
 		}
 	}
 	if mysql.HasBinaryFlag(fieldType.Flag) && fieldType.Tp != mysql.TypeJSON {
 		fieldType.Charset, fieldType.Collate = charset.CharsetBin, charset.CollationBin
 	} else {
-		fieldType.Charset, fieldType.Collate = charset.CharsetUTF8, charset.CharsetUTF8
+		fieldType.Charset, fieldType.Collate = mysql.DefaultCharset, mysql.DefaultCollationName


how about: charset.GetDefaultCharsetAndCollate()

zz-jason · 2018-11-09T02:51:15Z

expression/builtin.go

@@ -199,7 +199,7 @@ func (b *baseBuiltinFunc) getRetTp() *types.FieldType {
 			b.tp.Tp = mysql.TypeMediumBlob
 		}
 		if len(b.tp.Charset) <= 0 {
-			b.tp.Charset, b.tp.Collate = charset.CharsetUTF8, charset.CollationUTF8
+			b.tp.Charset, b.tp.Collate = mysql.DefaultCharset, mysql.DefaultCollationName


zz-jason · 2018-11-09T02:51:22Z

expression/builtin_cast.go

-		Charset: charset.CharsetUTF8,
-		Collate: charset.CollationUTF8,
+		Charset: charset.CharsetUTF8MB4,
+		Collate: charset.CollationUTF8MB4,


zz-jason · 2018-11-09T02:51:26Z

expression/builtin_cast.go

@@ -1741,7 +1741,7 @@ func WrapWithCastAsString(ctx sessionctx.Context, expr Expression) Expression {
 		argLen = mysql.MaxIntWidth
 	}
 	tp := types.NewFieldType(mysql.TypeVarString)
-	tp.Charset, tp.Collate = charset.CharsetUTF8, charset.CollationUTF8
+	tp.Charset, tp.Collate = charset.CharsetUTF8MB4, charset.CollationUTF8MB4


crazycs520 · 2018-11-09T02:59:56Z

expression/builtin.go

@@ -141,15 +141,15 @@ func newBaseBuiltinFuncWithTp(ctx sessionctx.Context, args []Expression, retType
 			Tp:      mysql.TypeJSON,
 			Flen:    mysql.MaxBlobWidth,
 			Decimal: 0,
-			Charset: charset.CharsetUTF8,
-			Collate: charset.CollationUTF8,
+			Charset: mysql.DefaultCharset,


Why not charset.CharsetUTF8MB4?

crazycs520 · 2018-11-09T03:00:30Z

expression/builtin.go

-			Charset: charset.CharsetUTF8,
-			Collate: charset.CollationUTF8,
+			Charset: mysql.DefaultCharset,
+			Collate: mysql.DefaultCollationName,


why not charset.CollationUTF8MB4

crazycs520 · 2018-11-09T03:00:55Z

expression/builtin.go

 			Flag:    mysql.BinaryFlag,
 		}
 	}
 	if mysql.HasBinaryFlag(fieldType.Flag) && fieldType.Tp != mysql.TypeJSON {
 		fieldType.Charset, fieldType.Collate = charset.CharsetBin, charset.CollationBin
 	} else {
-		fieldType.Charset, fieldType.Collate = charset.CharsetUTF8, charset.CharsetUTF8
+		fieldType.Charset, fieldType.Collate = mysql.DefaultCharset, mysql.DefaultCollationName


crazycs520 · 2018-11-09T03:01:37Z

expression/builtin.go

@@ -199,7 +199,7 @@ func (b *baseBuiltinFunc) getRetTp() *types.FieldType {
 			b.tp.Tp = mysql.TypeMediumBlob
 		}
 		if len(b.tp.Charset) <= 0 {
-			b.tp.Charset, b.tp.Collate = charset.CharsetUTF8, charset.CollationUTF8
+			b.tp.Charset, b.tp.Collate = mysql.DefaultCharset, mysql.DefaultCollationName


crazycs520 · 2018-11-09T03:03:28Z

expression/builtin_control.go

 			if mysql.HasBinaryFlag(rhs.Flag) || !types.IsNonBinaryStr(lhs) {
 				resultFieldType.Flag |= mysql.BinaryFlag
 			}
 		} else if types.IsBinaryStr(lhs) || types.IsBinaryStr(rhs) || !evalType.IsStringKind() {
 			types.SetBinChsClnFlag(resultFieldType)
 		} else {
-			resultFieldType.Charset, resultFieldType.Collate, resultFieldType.Flag = charset.CharsetUTF8, charset.CollationUTF8, 0
+			resultFieldType.Charset, resultFieldType.Collate, resultFieldType.Flag = mysql.DefaultCharset, mysql.DefaultCollationName, 0


crazycs520 · 2018-11-09T03:08:16Z

expression/builtin_control.go

@@ -164,7 +164,7 @@ func (c *caseWhenFunctionClass) getFunction(ctx sessionctx.Context, args []Expre
 	}
 	fieldTp.Decimal, fieldTp.Flen = decimal, flen
 	if fieldTp.EvalType().IsStringKind() && !isBinaryStr {
-		fieldTp.Charset, fieldTp.Collate = mysql.DefaultCharset, mysql.DefaultCollationName
+		fieldTp.Charset, fieldTp.Collate = charset.CharsetUTF8MB4, charset.CollationUTF8MB4


/cc @XuHuaiyu , use mysql.DefaultCharset or charset.CharsetUTF8MB4?

Will mysql.DefaultCharset change? If so, we should we utf8mb4.

@XuHuaiyu, DefaultCharset is better. It hides the implementation details, once we change the default charset again, the code modification can be minimized.

no, this place should be utf8mb4, if we did not set the charset, it can be default charset, but in here, it should definitely be utf8mb4.

winkyao · 2018-11-09T03:32:31Z

@zz-jason @crazycs520 PTAL

zz-jason · 2018-11-09T03:35:52Z

executor/show.go

@@ -618,7 +618,7 @@ func (e *ShowExec) fetchShowCreateTable() error {
 	buf.WriteString(") ENGINE=InnoDB")
 	charsetName := tb.Meta().Charset
 	if len(charsetName) == 0 {
-		charsetName = charset.CharsetUTF8
+		charsetName = charset.CharsetUTF8MB4


s/charset.CharsetUTF8MB4/mysql.DefaultCharset/

zz-jason · 2018-11-09T03:36:55Z

expression/builtin_control.go

@@ -81,19 +81,19 @@ func inferType4ControlFuncs(lhs, rhs *types.FieldType) *types.FieldType {
 			}
 		}
 		if types.IsNonBinaryStr(lhs) && !types.IsBinaryStr(rhs) {
-			resultFieldType.Charset, resultFieldType.Collate, resultFieldType.Flag = charset.CharsetUTF8, charset.CollationUTF8, 0
+			resultFieldType.Charset, resultFieldType.Collate, resultFieldType.Flag = charset.CharsetUTF8MB4, charset.CollationUTF8MB4, 0


use mysql.DefaultCharset, mysql.DefaultCollationName instead.

zz-jason · 2018-11-09T03:37:57Z

planner/core/planbuilder.go

-		cs = mysql.DefaultCharset
-		cl = mysql.DefaultCollationName
+		cs = charset.CharsetUTF8MB4
+		cl = charset.CollationUTF8MB4


cs, cl = charset.GetDefaultCharsetAndCollate()

zz-jason · 2018-11-09T03:38:08Z

infoschema/tables.go

-		mCharset = mysql.DefaultCharset
-		mCollation = mysql.DefaultCollationName
+		mCharset = charset.CharsetUTF8MB4
+		mCollation = charset.CollationUTF8MB4


charset.GetDefaultCharsetAndCollate()

here should be definitely utf8mb4

crazycs520 · 2018-11-09T05:51:55Z

server/server.go

@@ -153,6 +153,7 @@ func NewServer(cfg *config.Config, driver IDriver) (*Server, error) {
 	var err error
 	if cfg.Socket != "" {
 		if s.listener, err = net.Listen("unix", cfg.Socket); err == nil {
+			// job.SnapshotVer == 0 means


crazycs520

Reset LGTM

zz-jason

LGTM

…mb4 utf8mb4_bin' (#7965) (#8590)

*: change default charset and collation from 'utf8 utf8_bin' to 'utf8…

3435aae

…mb4 utf8mb4_bin'

winkyao added the release-note Denotes a PR that will be considered when it comes time to generate release notes. label Oct 19, 2018

winkyao mentioned this pull request Oct 19, 2018

Set the default charset at startup #7757

Closed

3 tasks

shenli reviewed Oct 19, 2018

View reviewed changes

gregwebs reviewed Oct 19, 2018

View reviewed changes

morgo mentioned this pull request Oct 19, 2018

MySQL 8.0 Compatibility #7968

Open

69 tasks

address comment

cd01346

winkyao added 2 commits October 22, 2018 15:17

Merge remote-tracking branch 'upstream/master' into charset_mb4

6587484

Merge remote-tracking branch 'upstream/master' into charset_mb4

f4f840f

winkyao added the status/DNM label Oct 26, 2018

winkyao mentioned this pull request Nov 5, 2018

*: change default charset from utf8 to utf8mb4 pingcap/parser#13

Merged

Merge remote-tracking branch 'upstream/master' into charset_mb4

cd57638

winkyao added 4 commits November 5, 2018 17:21

remove parser

d30424c

remove util/charset

a7e2183

remove mysql

2ae3370

fix ci

e54162e

winkyao removed the status/DNM label Nov 5, 2018

winkyao added 4 commits November 8, 2018 19:32

Merge branch 'master' into charset_mb4

16c8cb0

update go module

fdbc6e4

Merge branch 'charset_mb4' of https://github.com/winkyao/tidb into ch…

27ad600

…arset_mb4

fix ci

68ec462

zimulala mentioned this pull request Nov 8, 2018

executor: support for "admin show next_row_id" #8242

Merged

winkyao added the status/all tests passed label Nov 9, 2018

XuHuaiyu reviewed Nov 9, 2018

View reviewed changes

zz-jason reviewed Nov 9, 2018

View reviewed changes

crazycs520 reviewed Nov 9, 2018

View reviewed changes

address comments

c0995b7

zz-jason reviewed Nov 9, 2018

View reviewed changes

address comment

5c1678d

crazycs520 reviewed Nov 9, 2018

View reviewed changes

cleanup

90b3a60

zz-jason approved these changes Nov 9, 2018

View reviewed changes

Merge branch 'master' into charset_mb4

8efce03

zz-jason added the status/LGT2 Indicates that a PR has LGTM 2. label Nov 9, 2018

zz-jason merged commit 29f14d4 into pingcap:master Nov 9, 2018

winkyao deleted the charset_mb4 branch November 9, 2018 11:03

morgo mentioned this pull request Nov 26, 2018

sql: Default changes from TIDB master pingcap/docs#771

Closed

winkyao mentioned this pull request Dec 6, 2018

*: change default charset and collation from 'utf8 utf8_bin' to 'utf8mb4 utf8mb4_bin' (#7965) #8590

Merged

winkyao added a commit that referenced this pull request Dec 6, 2018

*: change default charset and collation from 'utf8 utf8_bin' to 'utf8…

d9d153e

…mb4 utf8mb4_bin' (#7965) (#8590)

*: change default charset and collation from 'utf8 utf8_bin' to 'utf8mb4 utf8mb4_bin' #7965

*: change default charset and collation from 'utf8 utf8_bin' to 'utf8mb4 utf8mb4_bin' #7965

Conversation

winkyao commented Oct 19, 2018 • edited Loading

What problem does this PR solve?

how it works?

Check List

Choose a reason for hiding this comment

Choose a reason for hiding this comment

gregwebs left a comment

Choose a reason for hiding this comment

winkyao commented Oct 22, 2018

winkyao commented Nov 5, 2018

winkyao commented Nov 8, 2018

winkyao commented Nov 8, 2018

winkyao commented Nov 9, 2018

winkyao commented Nov 9, 2018

winkyao commented Nov 9, 2018

XuHuaiyu left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

winkyao commented Nov 9, 2018

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

crazycs520 left a comment

Choose a reason for hiding this comment

zz-jason left a comment

Choose a reason for hiding this comment

winkyao commented Oct 19, 2018 •

edited

Loading