*: adjust solution for TOOL-1420 and add a test case (#214)

Move the ToLower operation from (*TableRestore).initializeColumns() to (*ChunkParser).ReadRow(). In fact this is the same as the CSV parser.
pingcap · Jul 18, 2019 · 5218dd6 · 5218dd6
1 parent f3af17a
commit 5218dd6
Show file tree

Hide file tree

Showing 9 changed files with 89 additions and 39 deletions.
diff --git a/lightning/kv/sql2kv.go b/lightning/kv/sql2kv.go
@@ -104,7 +104,7 @@ func (row rowArrayMarshaler) MarshalLogArray(encoder zapcore.ArrayEncoder) error
 
 func logKVConvertFailed(logger log.Logger, row []types.Datum, j int, colInfo *model.ColumnInfo, err error) error {
 	var original types.Datum
-	if j < len(row) {
+	if 0 <= j && j < len(row) {
 		original = row[j]
 		row = row[j : j+1]
 	}

diff --git a/lightning/mydump/parser.go b/lightning/mydump/parser.go
@@ -102,6 +102,9 @@ type Parser interface {
 	Close() error
 	ReadRow() error
 	LastRow() Row
+
+	// Columns returns the _lower-case_ column names corresponding to values in
+	// the LastRow.
 	Columns() []string
 }
 
@@ -412,7 +415,7 @@ func (parser *ChunkParser) ReadRow() error {
 			case tokRowEnd:
 				st = stateValues
 			case tokUnquoted, tokDoubleQuoted, tokBackQuoted:
-				columnName := parser.unescapeString(string(content))
+				columnName := strings.ToLower(parser.unescapeString(string(content)))
 				parser.columns = append(parser.columns, columnName)
 			default:
 				return errors.Errorf(

diff --git a/lightning/mydump/parser_test.go b/lightning/mydump/parser_test.go
@@ -396,42 +396,42 @@ func (s *testMydumpParserSuite) TestPseudoKeywords(c *C) {
 	parser := mydump.NewChunkParser(mysql.ModeNone, reader, config.ReadBlockSize, s.ioWorkers)
 	c.Assert(parser.ReadRow(), IsNil)
 	c.Assert(parser.Columns(), DeepEquals, []string{
-		"c", "C",
-		"co", "CO",
-		"con", "CON",
-		"conv", "CONV",
-		"conve", "CONVE",
-		"conver", "CONVER",
-		"convert", "CONVERT",
-		"u", "U",
-		"us", "US",
-		"usi", "USI",
-		"usin", "USIN",
-		"ut", "UT",
-		"utf", "UTF",
-		"utf8", "UTF8",
-		"utf8m", "UTF8M",
-		"utf8mb", "UTF8MB",
-		"utf8mb4", "UTF8MB4",
-		"t", "T",
-		"tr", "TR",
-		"tru", "TRU",
-		"f", "F",
-		"fa", "FA",
-		"fal", "FAL",
-		"fals", "FALS",
-		"n", "N",
-		"nu", "NU",
-		"nul", "NUL",
-		"v", "V",
-		"va", "VA",
-		"val", "VAL",
-		"valu", "VALU",
-		"value", "VALUE",
-		"i", "I",
-		"ins", "INS",
-		"inse", "INSE",
-		"inser", "INSER",
+		"c", "c",
+		"co", "co",
+		"con", "con",
+		"conv", "conv",
+		"conve", "conve",
+		"conver", "conver",
+		"convert", "convert",
+		"u", "u",
+		"us", "us",
+		"usi", "usi",
+		"usin", "usin",
+		"ut", "ut",
+		"utf", "utf",
+		"utf8", "utf8",
+		"utf8m", "utf8m",
+		"utf8mb", "utf8mb",
+		"utf8mb4", "utf8mb4",
+		"t", "t",
+		"tr", "tr",
+		"tru", "tru",
+		"f", "f",
+		"fa", "fa",
+		"fal", "fal",
+		"fals", "fals",
+		"n", "n",
+		"nu", "nu",
+		"nul", "nul",
+		"v", "v",
+		"va", "va",
+		"val", "val",
+		"valu", "valu",
+		"value", "value",
+		"i", "i",
+		"ins", "ins",
+		"inse", "inse",
+		"inser", "inser",
 	})
 }
 

diff --git a/lightning/restore/restore.go b/lightning/restore/restore.go
@@ -1203,6 +1203,8 @@ func (t *TableRestore) populateChunks(cfg *config.Config, cp *TableCheckpoint) e
 // - column `d` is at position 0
 //
 // The column permutation of (d, b, a) is set to be [2, 1, -1, 0].
+//
+// The argument `columns` _must_ be in lower case.
 func (t *TableRestore) initializeColumns(columns []string, ccp *ChunkCheckpoint) {
 	colPerm := make([]int, 0, len(t.tableInfo.Core.Columns)+1)
 	shouldIncludeRowID := !t.tableInfo.Core.PKIsHandle
@@ -1218,7 +1220,7 @@ func (t *TableRestore) initializeColumns(columns []string, ccp *ChunkCheckpoint)
 	} else {
 		columnMap := make(map[string]int)
 		for i, column := range columns {
-			columnMap[strings.ToLower(column)] = i
+			columnMap[column] = i
 		}
 		for _, colInfo := range t.tableInfo.Core.Columns {
 			if i, ok := columnMap[colInfo.Name.L]; ok {

diff --git a/tests/tool_1420/config.toml b/tests/tool_1420/config.toml
@@ -0,0 +1,16 @@
+[lightning]
+file = "/tmp/lightning_test_result/lightning.log"
+
+[tikv-importer]
+addr = "127.0.0.1:8808"
+
+[mydumper]
+data-source-dir = "tests/tool_1420/data"
+
+[tidb]
+host = "127.0.0.1"
+port = 4000
+user = "root"
+status-port = 10080
+pd-addr = "127.0.0.1:2379"
+log-level = "error"
diff --git a/tests/tool_1420/data/EE1420-schema-create.sql b/tests/tool_1420/data/EE1420-schema-create.sql
@@ -0,0 +1 @@
+CREATE DATABASE `EE1420`;
diff --git a/tests/tool_1420/data/EE1420.pt_role-schema.sql b/tests/tool_1420/data/EE1420.pt_role-schema.sql
@@ -0,0 +1,3 @@
+CREATE TABLE `pt_role` (
+  `ROLE_ID` varchar(50) NOT NULL
+);
diff --git a/tests/tool_1420/data/EE1420.pt_role.sql b/tests/tool_1420/data/EE1420.pt_role.sql
@@ -0,0 +1 @@
+INSERT INTO `pt_role` (`ROLE_ID`) VALUES ("1");
diff --git a/tests/tool_1420/run.sh b/tests/tool_1420/run.sh
@@ -0,0 +1,24 @@
+#!/bin/sh
+#
+# Copyright 2019 PingCAP, Inc.
+#
+# Licensed under the Apache License, Version 2.0 (the "License");
+# you may not use this file except in compliance with the License.
+# You may obtain a copy of the License at
+#
+#     http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS,
+# See the License for the specific language governing permissions and
+# limitations under the License.
+
+# This test verifies if TOOL-1420 is fixed.
+# It involves column names not in lower-case.
+
+set -eu
+
+run_sql 'DROP DATABASE IF EXISTS `EE1420`;'
+run_lightning
+run_sql 'SELECT `ROLE_ID` FROM `EE1420`.`pt_role`;'
+check_contains 'ROLE_ID: 1'