-
Notifications
You must be signed in to change notification settings - Fork 1.4k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Add MS SQL Server Support #222
Conversation
Pull Request Test Coverage Report for Build 404
💛 - Coveralls |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for the PR!
Could you also add MS SQL Server
to the list of supported DBs in the README
I don't know MS SQL Server well enough to validate the queries, but the tests seem to provide decent coverage...
database/mssql/mssql_test.go
Outdated
Env: map[string]string{"ACCEPT_EULA": "Y", "SA_PASSWORD": saPassword, "MSSQL_PID": "Express"}, | ||
PortRequired: true, ReadyFunc: isReady, | ||
} | ||
// Supported versions: https://www.mysql.com/support/supportedplatforms/database.html |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Also add this link (lists available image tags) as a comment: https://mcr.microsoft.com/v2/mssql/server/tags/list
database/mssql/mssql.go
Outdated
} else if err != nil { | ||
return &database.Error{OrigErr: err, Err: "try lock failed", Query: []byte(query)} | ||
} else { | ||
return &database.Error{Err: fmt.Sprintf("try lock failed with error %v", lockErrorMap[int(status)]), Query: []byte(query)} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Also expose the actual mssql.ReturnStatus
in case the mapping doesn't contain the return status.
database/mssql/mssql.go
Outdated
ErrDatabaseDirty = fmt.Errorf("database is dirty") | ||
) | ||
|
||
var lockErrorMap = map[int]string{ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Change this to map[mssql.ReturnStatus]string
Finished the work started by @bwiggs and added tests.