Skip to content
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 Sqlserver database support #51

Open
wants to merge 5 commits into
base: master
Choose a base branch
from

Conversation

tendant
Copy link

@tendant tendant commented Dec 6, 2022

Add SQL Server database support

vendor/ folder is not included in this patch.

@fraenky8
Copy link
Owner

Thanks for that! Will take a look soon, probably next week!

Copy link
Owner

@fraenky8 fraenky8 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Mainly some naming and vendor changes.

Did you try your changes? Does it work? Since I don't have any experience working with SQLServer I have to trust you on this one.

Comment on lines +13 to +14
// Sqlserver implements the Database interface with help of GeneralDatabase.
type Sqlserver struct {
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Lets use consistently SQLServer as name. (ref https://github.com/golang/go/wiki/CodeReviewComments#initialisms)

}

// DSN creates the DSN String to connect to this database.
func (ss *Sqlserver) DSN() string {
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Lets add a unit test for that? I see the other database implementation have one, just for the sake of consistency?

if ss.Settings.User != "" {
user = ss.Settings.User
}
return fmt.Sprintf("server=%s;port=%s;user=%s;database=%s;password=%s",
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is there a way to connect via a socket to SQLServer? I see MySQL and PG have this option. Can you check and add it here as well?

// columns of a specific table for a given database.
func (ss *Sqlserver) PrepareGetColumnsOfTableStmt() (err error) {

ss.GetColumnsOfTableStmt, err = ss.Preparex(`
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just to be double sure, since I don't have any experience with SQLServer - this statement is the same as for PG except the placeholder format?

@@ -14,7 +14,11 @@ require (

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you run go mod tidy and go mod vendor to include the vendor changes?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants