*: fix utf8 charset upgrade compatibility #9820

crazycs520 · 2019-03-20T04:32:05Z

What problem does this PR solve?

In TiDB v2.0.8

tidb > create table t (a varchar(10)) charset=utf8mb4;
Query OK, 0 rows affected
tidb > insert into t values(x'f09f8c80');
Query OK, 1 row affected

$ curl "http://$IP:10080/schema/test/t"
...
"cols": [
      {
          ...
           "name": {
               "L": "a",
               "O": "a"
          },
        ...
           "type": {
               "Charset": "utf8",
               "Collate": "utf8_bin",

In TiDB v2.0.8, create table with specified table charset, the column charset won't use the table charset, but use UTF8 charset. It is a bug, and already fixed.

Then, after upgrade to TiDB Master,

tidb > show create table t
+-------+--------------------------------------------------------------------+
| Table | Create Table                                                       |
+-------+--------------------------------------------------------------------+
| t     | CREATE TABLE `t` (                                                 |
|       |   `a` varchar(10) CHARACTER SET utf8 COLLATE utf8_bin DEFAULT NULL |
|       | ) ENGINE=InnoDB DEFAULT CHARSET=utf8mb4 COLLATE=utf8mb4_bin        |
+-------+--------------------------------------------------------------------+

tidb > insert into t values(x'f09f8c80');
(1366, u'incorrect utf8 value f09f8c80(\U0001f300) for column a')

TiDB master have tidb_check_mb4_value_in_utf8 check, so insert x'f09f8c80' to utf8 column will return error.

For the client that upgrade from v2.0.8 to master, It will look like not compatibility. Because insert `x'f09f8c80' will successful in TiDB v2.0.8 but failed in TiDB master.

What is changed and how it works?

Add treat-old-version-utf8-as-utf8mb4 variable to toml config file and session variable (actual is system scope variable) and set default is true.

treat-old-version-utf8-as-utf8mb4 use for upgrade compatibility. Set to true will tread old version table/column UTF8 charset as UTF8MB4.

How to judge the table/column is new or old version?

Use tableInfo.Version to judge table is new or old version.
Use ColumnInfo.Version to judge column is new or old version.

This PR also increase the table and column version, Please merge parser PR: pingcap/parser#254 first.

drawback
The treat-old-version-utf8-as-utf8mb4 variable is not friendly for some user that specially specified the column charset to utf8.

another
Maybe a better way to fix this compatibility problem is to rebuild table info schema charset by get all ddl histofy and parse it and rebuild for charset. This may spend a little long time and will change the stored table schema data.

Check List

Tests

Unit test

Code changes

Has exported function/method change

Side effects

Related changes

Need to cherry-pick to the release branch 2.1

…how create table

codecov · 2019-03-20T04:46:34Z

Codecov Report

Merging #9820 into master will decrease coverage by 0.0353%.
The diff coverage is 47.2222%.

@@               Coverage Diff                @@
##             master      #9820        +/-   ##
================================================
- Coverage   67.3615%   67.3261%   -0.0354%     
================================================
  Files           383        383                
  Lines         80353      80370        +17     
================================================
- Hits          54127      54110        -17     
- Misses        21397      21416        +19     
- Partials       4829       4844        +15

table/column.go

ddl/column.go

winkyao · 2019-03-22T02:21:49Z

server/http_handler.go

@@ -743,6 +743,17 @@ func (h settingsHandler) ServeHTTP(w http.ResponseWriter, req *http.Request) {
 				return
 			}
 		}
+		if treadOldVersionUTF8AsUTF8MB4 := req.Form.Get("treat-old-version-utf8-as-utf8mb4"); treadOldVersionUTF8AsUTF8MB4 != "" {


add a session variable to set this config.

But the "session variable" actually is system scope, not session.

delete the session variable for need reload schema after change this variable.

config/config.go

bb7133 · 2019-03-22T07:05:37Z

sessionctx/variable/tidb_vars.go

@@ -131,6 +131,9 @@ const (

 	// TiDBCheckMb4ValueInUtf8 is used to control whether to enable the check wrong utf8 value.
 	TiDBCheckMb4ValueInUtf8 = "tidb_check_mb4_value_in_utf8"
+
+	// TiDBCheckMb4ValueInUtf8 is used to control whether to enable the check wrong utf8 value.


Please fix this comment

sessionctx/variable/tidb_vars.go

config/config.go

winkyao · 2019-03-22T09:34:53Z

table/column.go

@@ -183,6 +183,12 @@ func CastValue(ctx sessionctx.Context, val types.Datum, col *model.ColumnInfo) (
 	}
 	str := casted.GetString()
 	utf8Charset := col.Charset == mysql.UTF8Charset
+	doUTF8Check := utf8Charset && config.GetGlobalConfig().CheckMb4ValueInUtf8


this should be doMB4CharCheck

winkyao

reset LGTM

config/config.go

…ge tidb_treat_old_version_utf8_as_utf8mb4 take effect

…on variable

ddl/ddl_api.go

domain/domain.go

bb7133 · 2019-03-25T08:49:27Z

LGTM

xiekeyi98

LGTM

xiekeyi98

LGTM

…rset-upgrade

into fix-charset-upgrade

crazycs520 · 2019-03-25T09:24:45Z

/run-all-tests

crazycs520 · 2019-03-25T09:31:48Z

/run-all-tests

zimulala

Do we need to update the column's or table's version when we executing AlterTableCharsetAndCollate or Change/Modify column? Because we need to add a tool to update old TiDB table versions to the new version.

winkyao · 2019-03-25T09:43:08Z

@zimulala Not necessary, we can just change the charset to utf8mb4, and keep the table version unchanged, that is ok.

zimulala

~~We use TableInfoVersion2 to check, but we don't set table versions to TableInfoVersion2 or CurrLatestTableInfoVersion~~

not necessary

crazycs520 added 3 commits March 20, 2019 10:28

add ignore-column-utf8-charset to toml config file and default is true

3924856

make ignore-column-utf8-charset take effect in check no-utf8mb4 and s…

d96be67

…how create table

add test and fix test

3a77bc6

zimulala reviewed Mar 20, 2019

View reviewed changes

table/column.go Outdated Show resolved Hide resolved

crazycs520 added status/WIP type/compatibility labels Mar 20, 2019

crazycs520 added 3 commits March 20, 2019 19:50

treat utf8 as utf8mb4 for old version table/column

16494b2

add test and modify/change column should not change column version

c8e5b9d

update variable name

15c43af

crazycs520 removed the status/WIP label Mar 21, 2019

crazycs520 added 3 commits March 21, 2019 12:53

refine code

18125dc

refine code

40c68dc

add column version for view column

e4f71a1

winkyao reviewed Mar 21, 2019

View reviewed changes

ddl/column.go Show resolved Hide resolved

crazycs520 mentioned this pull request Mar 21, 2019

model: increase table and column version pingcap/parser#254

Merged

increase table and column version

1ed6ef6

winkyao reviewed Mar 22, 2019

View reviewed changes

add tidb_treat_old_version_utf8_as_utf8mb4 system variable

655fc6b

bb7133 reviewed Mar 22, 2019

View reviewed changes

config/config.go Outdated Show resolved Hide resolved

bb7133 reviewed Mar 22, 2019

View reviewed changes

config/config.go Outdated Show resolved Hide resolved

bb7133 reviewed Mar 22, 2019

View reviewed changes

sessionctx/variable/tidb_vars.go Outdated Show resolved Hide resolved

winkyao reviewed Mar 22, 2019

View reviewed changes

config/config.go Outdated Show resolved Hide resolved

winkyao reviewed Mar 22, 2019

View reviewed changes

crazycs520 added 2 commits March 22, 2019 17:45

address comment

adc67a2

address comment

786c6e8

zimulala reviewed Mar 22, 2019

View reviewed changes

config/config.go Show resolved Hide resolved

winkyao added the priority/release-blocker This issue blocks a release. Please solve it ASAP. label Mar 25, 2019

crazycs520 added 5 commits March 25, 2019 14:20

ConvertOldVersionUTF8AsUTF8MB4IfNeed when load schema, TODO: fix chan…

b0512cf

…ge tidb_treat_old_version_utf8_as_utf8mb4 take effect

add comment

99de30a

*: remove http api modify treat-old-version-utf8-as-utf8mb4 and sessi…

e757f21

…on variable

remove redundancy code

62d49b8

fix alter table charset

d6b6280

winkyao reviewed Mar 25, 2019

View reviewed changes

ddl/ddl_api.go Show resolved Hide resolved

winkyao reviewed Mar 25, 2019

View reviewed changes

domain/domain.go Show resolved Hide resolved

add comment

57f22e6

xiekeyi98 previously approved these changes Mar 25, 2019

View reviewed changes

Merge branch 'master' into fix-charset-upgrade

2d226d1

winkyao dismissed xiekeyi98’s stale review via 2d226d1 March 25, 2019 09:21

xiekeyi98 approved these changes Mar 25, 2019

View reviewed changes

crazycs520 added 2 commits March 25, 2019 17:23

Merge branch 'master' of https://github.com/pingcap/tidb into fix-cha…

e520900

…rset-upgrade

Merge branch 'fix-charset-upgrade' of https://github.com/crazycs520/tidb

44912c8

into fix-charset-upgrade

xiekeyi98 added the status/LGT3 The PR has already had 3 LGTM. label Mar 25, 2019

zimulala previously requested changes Mar 25, 2019

View reviewed changes

zimulala reviewed Mar 25, 2019

View reviewed changes

winkyao approved these changes Mar 25, 2019

View reviewed changes

winkyao merged commit c451f00 into pingcap:master Mar 25, 2019

crazycs520 added a commit to crazycs520/tidb that referenced this pull request Mar 25, 2019

*: fix utf8 charset upgrade compatibility (pingcap#9820)

a9f6a0e

crazycs520 mentioned this pull request Mar 25, 2019

*: fix utf8 charset upgrade compatibility (#9820) #9887

Merged

winkyao pushed a commit that referenced this pull request Mar 26, 2019

*: fix utf8 charset upgrade compatibility (#9820) (#9887)

41d90ef

crazycs520 mentioned this pull request Jun 27, 2019

faq/upgrade: update upgrade faq pingcap/docs-cn#1527

Merged

5 tasks

you06 added the sig/sql-infra SIG: SQL Infra label Mar 4, 2020

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

*: fix utf8 charset upgrade compatibility #9820

*: fix utf8 charset upgrade compatibility #9820

crazycs520 commented Mar 20, 2019 •

edited

Loading

codecov bot commented Mar 20, 2019 •

edited

Loading

winkyao Mar 22, 2019

crazycs520 Mar 22, 2019

crazycs520 Mar 22, 2019

crazycs520 Mar 25, 2019

bb7133 Mar 22, 2019

crazycs520 Mar 22, 2019

winkyao Mar 22, 2019

crazycs520 Mar 22, 2019

winkyao left a comment

bb7133 commented Mar 25, 2019

xiekeyi98 left a comment

xiekeyi98 left a comment

crazycs520 commented Mar 25, 2019

crazycs520 commented Mar 25, 2019

zimulala left a comment •

edited

Loading

winkyao commented Mar 25, 2019

zimulala left a comment •

edited

Loading

*: fix utf8 charset upgrade compatibility #9820

*: fix utf8 charset upgrade compatibility #9820

Conversation

crazycs520 commented Mar 20, 2019 • edited Loading

What problem does this PR solve?

What is changed and how it works?

Check List

codecov bot commented Mar 20, 2019 • edited Loading

Codecov Report

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 left a comment

Choose a reason for hiding this comment

bb7133 commented Mar 25, 2019

xiekeyi98 left a comment

Choose a reason for hiding this comment

xiekeyi98 left a comment

Choose a reason for hiding this comment

crazycs520 commented Mar 25, 2019

crazycs520 commented Mar 25, 2019

zimulala left a comment • edited Loading

Choose a reason for hiding this comment

winkyao commented Mar 25, 2019

zimulala left a comment • edited Loading

Choose a reason for hiding this comment

crazycs520 commented Mar 20, 2019 •

edited

Loading

codecov bot commented Mar 20, 2019 •

edited

Loading

zimulala left a comment •

edited

Loading

zimulala left a comment •

edited

Loading