Fix warnings with schema Sync2 with default varchar as NVARCHAR #1783
No reviewers
Labels
No Label
backport/done
backport/v1
blocked
db
oracle
db
sqlserver
duplicate
feature
cache
frontport/done
frontport/main
invalid
kind
breaking
kind
bug
kind
build
kind
dependencies
kind
docs
kind
driver
kind
enhancement
kind
feature
kind
performance
kind
proposal
kind
question
kind
refactor
kind
testing
need
feedback
need
test
proposal:accepted
RaspBerry Pi
regression
skip-changelog
upstream
wip
wontfix
No Milestone
No Assignees
2 Participants
Notifications
Due Date
No due date set.
Dependencies
No dependencies set.
Reference: xorm/xorm#1783
Loading…
Reference in New Issue
Block a user
No description provided.
Delete Branch "zeripath/xorm:fix-1782-MSSQL-nvarchar-fixes"
Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?
sys.columns sets the max_length as double that which it really is for NVARCHAR and NCHAR.
Also sets Text, MediumText and LongText as NVARCHAR(MAX).
Adds a drone test running on MSSSQL tests with NVARCHAR and adjusts test cases to force them to fail with #1782
Fix #1782
Hmm adding drone for mssql-nvarchar significantly lengthens your CI - I'm not certain if you want to do that.
I could just add a few choice mssql specific tests?
OK I've removed the test-mssql-nvarchar drone task and added another sync test that can be switched on or off with a flag and sets the params as appropriate.
@ -424,8 +424,14 @@ 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}
col.Length /= 2
Why we need that?
https://social.microsoft.com/Forums/Windows/en-US/be4ad98b-473a-4183-a541-71c3fd79d958/length-of-nvarchar-column-in-system-tables?forum=transactsql
So potentially there is an issue with /2 in the case of MAXs but the test appears to work for TEXT
hehe - it worked for TEXT because I hadn't tested it... Now... it works for TEXT.
I've just realised I haven't tested the Text or Char places I'll get another few commits off
Last CI failures are not related to this PR.
test-postgres
andtest-cockroach
failures are related.Ah the changes in TestSyncTable3 have revealed a similar bug in postgres. I'm gonna push a few more test cases up in case they reveal any more interesting information.
OK it appears that postgres and cockroachdb map Char(n)/NChar(n) as Varchar(n) because there is no benefit for char so I've just forciby mapped Char as Varchar
@ -857,6 +857,10 @@ func (db *postgres) SQLType(c *schemas.Column) string {
res = schemas.Real
case schemas.TinyText, schemas.MediumText, schemas.LongText:
res = schemas.Text
case schemas.Char:
I don't think we should convert Char to Varchar since postgres support
CHAR
type. We just needOK I've changed it instead. When
dialect.GetColumns(...)
reads a column with "character" it will get mapped to "CHAR" SQL type.