From b4229306172900cc0bce6eab4e0036b3649e4432 Mon Sep 17 00:00:00 2001 From: Andrew Thornton Date: Tue, 8 Sep 2020 01:36:50 +0000 Subject: [PATCH] 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 Merge branch 'master' into fix-1782-MSSQL-nvarchar-fixes Postgres (and cockroachDB by inheritance) maps Char to Varchar Signed-off-by: Andrew Thornton 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 Add tests for Test and Char columns Signed-off-by: Andrew Thornton prod CI fix test Signed-off-by: Andrew Thornton Remove the duplicate mssql drone test and add a specific sync test Signed-off-by: Andrew Thornton add depends_on (2) Signed-off-by: Andrew Thornton add depends_on Signed-off-by: Andrew Thornton add nvarchar as a testcase Signed-off-by: Andrew Thornton Set defaultVarchar to upper case Signed-off-by: Andrew Thornton Fix length issue Signed-off-by: Andrew Thornton schemas.Text should map to db.defaultVarchar Signed-off-by: Andrew Thornton Add failing test Signed-off-by: Andrew Thornton Reviewed-on: https://gitea.com/xorm/xorm/pulls/1783 Reviewed-by: Lunny Xiao --- Makefile | 3 ++ dialects/mssql.go | 32 ++++++++++++-- dialects/postgres.go | 6 ++- integrations/session_schema_test.go | 66 +++++++++++++++++++++++++++++ integrations/tests.go | 1 + 5 files changed, 104 insertions(+), 4 deletions(-) diff --git a/Makefile b/Makefile index ed873883..092f23b3 100644 --- a/Makefile +++ b/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 diff --git a/dialects/mssql.go b/dialects/mssql.go index d76a8c6c..8e76e538 100644 --- a/dialects/mssql.go +++ b/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} diff --git a/dialects/postgres.go b/dialects/postgres.go index 3c1d53c0..a2c0de74 100644 --- a/dialects/postgres.go +++ b/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": diff --git a/integrations/session_schema_test.go b/integrations/session_schema_test.go index 005b6619..3f8f810b 100644 --- a/integrations/session_schema_test.go +++ b/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()) diff --git a/integrations/tests.go b/integrations/tests.go index b135bd90..31fa99bf 100644 --- a/integrations/tests.go +++ b/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")