Fix warnings with schema Sync2 with default varchar as NVARCHAR (#1783)

prod CI

Merge branch 'fix-1782-MSSQL-nvarchar-fixes' of gitea.com:zeripath/xorm into fix-1782-MSSQL-nvarchar-fixes

map "character" to Char for postgres

Signed-off-by: Andrew Thornton <art27@cantab.net>

Merge branch 'master' into fix-1782-MSSQL-nvarchar-fixes

Postgres (and cockroachDB by inheritance) maps Char to Varchar

Signed-off-by: Andrew Thornton <art27@cantab.net>

fix test

Add a few more column testcases in light of postgres weirdness

prod CI

prod CI

prod CI

Properly handle MAX

Signed-off-by: Andrew Thornton <art27@cantab.net>

Add tests for Test and Char columns

Signed-off-by: Andrew Thornton <art27@cantab.net>

prod CI

fix test

Signed-off-by: Andrew Thornton <art27@cantab.net>

Remove the duplicate mssql drone test and add a specific sync test

Signed-off-by: Andrew Thornton <art27@cantab.net>

add depends_on (2)

Signed-off-by: Andrew Thornton <art27@cantab.net>

add depends_on

Signed-off-by: Andrew Thornton <art27@cantab.net>

add nvarchar as a testcase

Signed-off-by: Andrew Thornton <art27@cantab.net>

Set defaultVarchar to upper case

Signed-off-by: Andrew Thornton <art27@cantab.net>

Fix length issue

Signed-off-by: Andrew Thornton <art27@cantab.net>

schemas.Text should map to db.defaultVarchar

Signed-off-by: Andrew Thornton <art27@cantab.net>

Add failing test

Signed-off-by: Andrew Thornton <art27@cantab.net>

Reviewed-on: https://gitea.com/xorm/xorm/pulls/1783
Reviewed-by: Lunny Xiao <xiaolunwen@gmail.com>

Makefile

@@ -22,6 +22,7 @@ TEST_MSSQL_USERNAME ?= sa
 TEST_MSSQL_PASSWORD ?= MwantsaSecurePassword1
 TEST_MSSQL_DEFAULT_VARCHAR ?= varchar
 TEST_MSSQL_DEFAULT_CHAR ?= char
+TEST_MSSQL_DO_NVARCHAR_OVERRIDE_TEST ?= true
 
 TEST_MYSQL_HOST ?= mysql:3306
 TEST_MYSQL_CHARSET ?= utf8
@@ -147,6 +148,7 @@ test-mssql: go-check
 	$(GO) test $(INTEGRATION_PACKAGES) -v -race -db=mssql -cache=$(TEST_CACHE_ENABLE) -quote=$(TEST_QUOTE_POLICY) \
 	-conn_str="server=$(TEST_MSSQL_HOST);user id=$(TEST_MSSQL_USERNAME);password=$(TEST_MSSQL_PASSWORD);database=$(TEST_MSSQL_DBNAME)" \
 	-default_varchar=$(TEST_MSSQL_DEFAULT_VARCHAR) -default_char=$(TEST_MSSQL_DEFAULT_CHAR) \
+	-do_nvarchar_override_test=$(TEST_MSSQL_DO_NVARCHAR_OVERRIDE_TEST) \
 	-coverprofile=mssql.$(TEST_QUOTE_POLICY).$(TEST_CACHE_ENABLE).coverage.out -covermode=atomic
 
 .PNONY: test-mssql\#%
@@ -154,6 +156,7 @@ test-mssql\#%: go-check
 	$(GO) test $(INTEGRATION_PACKAGES) -v -race -run $* -db=mssql -cache=$(TEST_CACHE_ENABLE) -quote=$(TEST_QUOTE_POLICY) \
 	-conn_str="server=$(TEST_MSSQL_HOST);user id=$(TEST_MSSQL_USERNAME);password=$(TEST_MSSQL_PASSWORD);database=$(TEST_MSSQL_DBNAME)" \
 	-default_varchar=$(TEST_MSSQL_DEFAULT_VARCHAR) -default_char=$(TEST_MSSQL_DEFAULT_CHAR) \
+	-do_nvarchar_override_test=$(TEST_MSSQL_DO_NVARCHAR_OVERRIDE_TEST) \
 	-coverprofile=mssql.$(TEST_QUOTE_POLICY).$(TEST_CACHE_ENABLE).coverage.out -covermode=atomic
 
 .PNONY: test-mymysql

dialects/mssql.go

@@ -229,7 +229,7 @@ func (db *mssql) SetParams(params map[string]string) {
 		var t = strings.ToUpper(defaultVarchar)
 		switch t {
 		case "NVARCHAR", "VARCHAR":
-			db.defaultVarchar = defaultVarchar
+			db.defaultVarchar = t
 		default:
 			db.defaultVarchar = "VARCHAR"
 		}
@@ -242,7 +242,7 @@ func (db *mssql) SetParams(params map[string]string) {
 		var t = strings.ToUpper(defaultChar)
 		switch t {
 		case "NCHAR", "CHAR":
-			db.defaultChar = defaultChar
+			db.defaultChar = t
 		default:
 			db.defaultChar = "CHAR"
 		}
@@ -285,7 +285,7 @@ func (db *mssql) SQLType(c *schemas.Column) string {
 	case schemas.MediumInt:
 		res = schemas.Int
 	case schemas.Text, schemas.MediumText, schemas.TinyText, schemas.LongText, schemas.Json:
-		res = schemas.Varchar + "(MAX)"
+		res = db.defaultVarchar + "(MAX)"
 	case schemas.Double:
 		res = schemas.Real
 	case schemas.Uuid:
@@ -297,10 +297,26 @@ func (db *mssql) SQLType(c *schemas.Column) string {
 	case schemas.BigInt:
 		res = schemas.BigInt
 		c.Length = 0
+	case schemas.NVarchar:
+		res = t
+		if c.Length == -1 {
+			res += "(MAX)"
+		}
 	case schemas.Varchar:
 		res = db.defaultVarchar
+		if c.Length == -1 {
+			res += "(MAX)"
+		}
 	case schemas.Char:
 		res = db.defaultChar
+		if c.Length == -1 {
+			res += "(MAX)"
+		}
+	case schemas.NChar:
+		res = t
+		if c.Length == -1 {
+			res += "(MAX)"
+		}
 	default:
 		res = t
 	}
@@ -424,8 +440,18 @@ func (db *mssql) GetColumns(queryer core.Queryer, ctx context.Context, tableName
 			col.SQLType = schemas.SQLType{Name: schemas.TimeStampz, DefaultLength: 0, DefaultLength2: 0}
 		case "NVARCHAR":
 			col.SQLType = schemas.SQLType{Name: schemas.NVarchar, DefaultLength: 0, DefaultLength2: 0}
+			if col.Length > 0 {
+				col.Length /= 2
+				col.Length2 /= 2
+			}
 		case "IMAGE":
 			col.SQLType = schemas.SQLType{Name: schemas.VarBinary, DefaultLength: 0, DefaultLength2: 0}
+		case "NCHAR":
+			if col.Length > 0 {
+				col.Length /= 2
+				col.Length2 /= 2
+			}
+			fallthrough
 		default:
 			if _, ok := schemas.SqlTypes[ct]; ok {
 				col.SQLType = schemas.SQLType{Name: ct, DefaultLength: 0, DefaultLength2: 0}

dialects/postgres.go

@@ -857,6 +857,8 @@ func (db *postgres) SQLType(c *schemas.Column) string {
 		res = schemas.Real
 	case schemas.TinyText, schemas.MediumText, schemas.LongText:
 		res = schemas.Text
+	case schemas.NChar:
+		res = schemas.Char
 	case schemas.NVarchar:
 		res = schemas.Varchar
 	case schemas.Uuid:
@@ -1086,8 +1088,10 @@ WHERE n.nspname= s.table_schema AND c.relkind = 'r'::char AND c.relname = $1%s A
 		col.Nullable = (isNullable == "YES")
 
 		switch strings.ToLower(dataType) {
-		case "character varying", "character", "string":
+		case "character varying", "string":
 			col.SQLType = schemas.SQLType{Name: schemas.Varchar, DefaultLength: 0, DefaultLength2: 0}
+		case "character":
+			col.SQLType = schemas.SQLType{Name: schemas.Char, DefaultLength: 0, DefaultLength2: 0}
 		case "timestamp without time zone":
 			col.SQLType = schemas.SQLType{Name: schemas.DateTime, DefaultLength: 0, DefaultLength2: 0}
 		case "timestamp with time zone":

integrations/session_schema_test.go

@@ -102,6 +102,9 @@ func TestSyncTable(t *testing.T) {
 	assert.NoError(t, err)
 	assert.EqualValues(t, 1, len(tables))
 	assert.EqualValues(t, "sync_table1", tables[0].Name)
+	tableInfo, err := testEngine.TableInfo(new(SyncTable1))
+	assert.NoError(t, err)
+	assert.EqualValues(t, testEngine.Dialect().SQLType(tables[0].GetColumn("name")), testEngine.Dialect().SQLType(tableInfo.GetColumn("name")))
 
 	assert.NoError(t, testEngine.Sync2(new(SyncTable2)))
 
@@ -109,6 +112,9 @@ func TestSyncTable(t *testing.T) {
 	assert.NoError(t, err)
 	assert.EqualValues(t, 1, len(tables))
 	assert.EqualValues(t, "sync_table1", tables[0].Name)
+	tableInfo, err = testEngine.TableInfo(new(SyncTable2))
+	assert.NoError(t, err)
+	assert.EqualValues(t, testEngine.Dialect().SQLType(tables[0].GetColumn("name")), testEngine.Dialect().SQLType(tableInfo.GetColumn("name")))
 
 	assert.NoError(t, testEngine.Sync2(new(SyncTable3)))
 
@@ -116,6 +122,9 @@ func TestSyncTable(t *testing.T) {
 	assert.NoError(t, err)
 	assert.EqualValues(t, 1, len(tables))
 	assert.EqualValues(t, "sync_table1", tables[0].Name)
+	tableInfo, err = testEngine.TableInfo(new(SyncTable3))
+	assert.NoError(t, err)
+	assert.EqualValues(t, testEngine.Dialect().SQLType(tables[0].GetColumn("name")), testEngine.Dialect().SQLType(tableInfo.GetColumn("name")))
 }
 
 func TestSyncTable2(t *testing.T) {
@@ -143,6 +152,63 @@ func TestSyncTable2(t *testing.T) {
 	assert.EqualValues(t, colMapper.Obj2Table("NewCol"), tables[0].Columns()[3].Name)
 }
 
+func TestSyncTable3(t *testing.T) {
+	type SyncTable5 struct {
+		Id         int64
+		Name       string
+		Text       string   `xorm:"TEXT"`
+		Char       byte     `xorm:"CHAR(1)"`
+		TenChar    [10]byte `xorm:"CHAR(10)"`
+		TenVarChar string   `xorm:"VARCHAR(10)"`
+	}
+
+	assert.NoError(t, PrepareEngine())
+
+	assert.NoError(t, testEngine.Sync2(new(SyncTable5)))
+
+	tables, err := testEngine.DBMetas()
+	assert.NoError(t, err)
+	tableInfo, err := testEngine.TableInfo(new(SyncTable5))
+	assert.NoError(t, err)
+	assert.EqualValues(t, testEngine.Dialect().SQLType(tableInfo.GetColumn("name")), testEngine.Dialect().SQLType(tables[0].GetColumn("name")))
+	assert.EqualValues(t, testEngine.Dialect().SQLType(tableInfo.GetColumn("text")), testEngine.Dialect().SQLType(tables[0].GetColumn("text")))
+	assert.EqualValues(t, testEngine.Dialect().SQLType(tableInfo.GetColumn("char")), testEngine.Dialect().SQLType(tables[0].GetColumn("char")))
+	assert.EqualValues(t, testEngine.Dialect().SQLType(tableInfo.GetColumn("ten_char")), testEngine.Dialect().SQLType(tables[0].GetColumn("ten_char")))
+	assert.EqualValues(t, testEngine.Dialect().SQLType(tableInfo.GetColumn("ten_var_char")), testEngine.Dialect().SQLType(tables[0].GetColumn("ten_var_char")))
+
+	if *doNVarcharTest {
+		var oldDefaultVarchar string
+		var oldDefaultChar string
+		oldDefaultVarchar, *defaultVarchar = *defaultVarchar, "nvarchar"
+		oldDefaultChar, *defaultChar = *defaultChar, "nchar"
+		testEngine.Dialect().SetParams(map[string]string{
+			"DEFAULT_VARCHAR": *defaultVarchar,
+			"DEFAULT_CHAR":    *defaultChar,
+		})
+		defer func() {
+			*defaultVarchar = oldDefaultVarchar
+			*defaultChar = oldDefaultChar
+			testEngine.Dialect().SetParams(map[string]string{
+				"DEFAULT_VARCHAR": *defaultVarchar,
+				"DEFAULT_CHAR":    *defaultChar,
+			})
+		}()
+		assert.NoError(t, PrepareEngine())
+
+		assert.NoError(t, testEngine.Sync2(new(SyncTable5)))
+
+		tables, err := testEngine.DBMetas()
+		assert.NoError(t, err)
+		tableInfo, err := testEngine.TableInfo(new(SyncTable5))
+		assert.NoError(t, err)
+		assert.EqualValues(t, testEngine.Dialect().SQLType(tableInfo.GetColumn("name")), testEngine.Dialect().SQLType(tables[0].GetColumn("name")))
+		assert.EqualValues(t, testEngine.Dialect().SQLType(tableInfo.GetColumn("text")), testEngine.Dialect().SQLType(tables[0].GetColumn("text")))
+		assert.EqualValues(t, testEngine.Dialect().SQLType(tableInfo.GetColumn("char")), testEngine.Dialect().SQLType(tables[0].GetColumn("char")))
+		assert.EqualValues(t, testEngine.Dialect().SQLType(tableInfo.GetColumn("ten_char")), testEngine.Dialect().SQLType(tables[0].GetColumn("ten_char")))
+		assert.EqualValues(t, testEngine.Dialect().SQLType(tableInfo.GetColumn("ten_var_char")), testEngine.Dialect().SQLType(tables[0].GetColumn("ten_var_char")))
+	}
+}
+
 func TestIsTableExist(t *testing.T) {
 	assert.NoError(t, PrepareEngine())
 

integrations/tests.go

@@ -35,6 +35,7 @@ var (
 	schema             = flag.String("schema", "", "specify the schema")
 	ignoreSelectUpdate = flag.Bool("ignore_select_update", false, "ignore select update if implementation difference, only for tidb")
 	ingoreUpdateLimit  = flag.Bool("ignore_update_limit", false, "ignore update limit if implementation difference, only for cockroach")
+	doNVarcharTest     = flag.Bool("do_nvarchar_override_test", false, "do nvarchar override test in sync table, only for mssql")
 	quotePolicyStr     = flag.String("quote", "always", "quote could be always, none, reversed")
 	defaultVarchar     = flag.String("default_varchar", "varchar", "default varchar type, mssql only, could be varchar or nvarchar, default is varchar")
 	defaultChar        = flag.String("default_char", "char", "default char type, mssql only, could be char or nchar, default is char")