resolve comments

pingcap · Dec 25, 2020 · 551ce93 · 551ce93
1 parent 76b2023
commit 551ce93
Show file tree

Hide file tree

Showing 3 changed files with 20 additions and 14 deletions.
diff --git a/lightning/backend/tidb.go b/lightning/backend/tidb.go
@@ -242,9 +242,8 @@ func (*tidbEncoder) Close() {}
 func getColumnByIndex(cols []*table.Column, index int) *table.Column {
 	if index == len(cols) {
 		return extraHandleTableColumn
-	} else {
-		return cols[index]
 	}
+	return cols[index]
 }
 
 func (enc *tidbEncoder) Encode(logger log.Logger, row []types.Datum, _ int64, columnPermutation []int) (Row, error) {
@@ -267,7 +266,7 @@ func (enc *tidbEncoder) Encode(logger log.Logger, row []types.Datum, _ int64, co
 	// See: tests/generated_columns/data/gencol.various_types.0.sql this sql has no columns, so encodeLoop will fill the
 	// column permutation with default, thus enc.columnCnt > len(row).
 	if len(row) > enc.columnCnt {
-		log.L().Error("column count mismatch", zap.Ints("column_permutation", columnPermutation),
+		logger.Error("column count mismatch", zap.Ints("column_permutation", columnPermutation),
 			zap.Array("data", rowArrayMarshaler(row)))
 		return nil, errors.Errorf("column count mismatch, expected %d, got %d", enc.columnCnt, len(row))
 	}

diff --git a/lightning/restore/tidb.go b/lightning/restore/tidb.go
@@ -54,8 +54,6 @@ var (
 		"default_week_format":     "0",
 		"block_encryption_mode":   "aes-128-ecb",
 		"group_concat_max_len":    "1024",
-		// allow use _tidb_rowid in sql statement
-		"tidb_opt_write_row_id": "1",
 	}
 )
 
@@ -100,6 +98,8 @@ func DBFromConfig(dsn config.DBStore) (*sql.DB, error) {
 			// after https://github.com/pingcap/tidb/pull/17102 merge,
 			// we need set session to true for insert auto_random value in TiDB Backend
 			"allow_auto_random_explicit_insert": "1",
+			// allow use _tidb_rowid in sql statement
+			"tidb_opt_write_row_id": "1",
 		},
 	}
 	db, err := param.Connect()

diff --git a/tests/tidb_rowid/run.sh b/tests/tidb_rowid/run.sh
@@ -18,19 +18,26 @@
 set -eu
 
 for BACKEND in local importer tidb; do
+  if [ "$BACKEND" = 'local' ]; then
+      check_cluster_version 4 0 0 'local backend' || continue
+  fi
+
   run_sql 'DROP DATABASE IF EXISTS rowid;'
   run_lightning -backend $BACKEND
   echo 'Import finished'
 
-  run_sql 'SELECT count(*), max(id), min(_tidb_rowid), max(_tidb_rowid) FROM rowid.`non_pk_auto_inc`'
-  check_contains 'count(*): 22'
-  check_contains 'max(id): 37'
-  check_contains 'min(_tidb_rowid): 1'
-  check_contains 'max(_tidb_rowid): 22'
-  run_sql 'INSERT INTO rowid.`non_pk_auto_inc` (`pk`) VALUES ("?")'
-  run_sql 'SELECT id > 37, _tidb_rowid > 22 FROM rowid.`non_pk_auto_inc` WHERE `pk` = "?"'
-  check_contains 'id > 37: 1'
-  check_contains '_tidb_rowid > 22: 1'
+  # we can't determine the exact `_tidb_row_id` alloc logic, so just skip this check with tidb backend.
+  if [ "$BACKEND" != 'tidb' ]; then
+    run_sql 'SELECT count(*), max(id), min(_tidb_rowid), max(_tidb_rowid) FROM rowid.`non_pk_auto_inc`'
+    check_contains 'count(*): 22'
+    check_contains 'max(id): 37'
+    check_contains 'min(_tidb_rowid): 1'
+    check_contains 'max(_tidb_rowid): 22'
+    run_sql 'INSERT INTO rowid.`non_pk_auto_inc` (`pk`) VALUES ("?")'
+    run_sql 'SELECT id > 37, _tidb_rowid > 22 FROM rowid.`non_pk_auto_inc` WHERE `pk` = "?"'
+    check_contains 'id > 37: 1'
+    check_contains '_tidb_rowid > 22: 1'
+  fi
 
   for table_name in non_pk explicit_tidb_rowid; do
       run_sql "SELECT count(*), min(_tidb_rowid), max(_tidb_rowid) FROM rowid.${table_name}"