gobuffalo / pop

A Tasty Treat For All Your Database Needs
MIT License
1.44k stars 243 forks source link

Support MS SQL Server #40

Open marpio opened 6 years ago

marpio commented 6 years ago

It's an common DB in the enterprise and now also available for Linux.

markbates commented 6 years ago

Completely agree, it just needs someone to step up and implement it. :)

marpio commented 6 years ago

Well, I'm interested. Although I probably would need some guidance.

markbates commented 6 years ago

Of course! I’m happy to offer guidance. @mclark4386 implemented cockroach support recently. I’m sure he’ll be happy to offer some guidance along the way too!

marpio commented 6 years ago

@markbates So I took a glance at the cockroach commits and if I understand correctly the main task would be to implement sqlserver.go and fizz/translators/sqlserver.go similar to mysql/postgres/cockroach. Is that right?

markbates commented 6 years ago

That’s the basics of it, yep. I would pick the implementation that’s closest to MS, I’ve never used it, and use that as a starting point. Rename a few things, point them at the MS DB and see what the tests yell at you about.

marpio commented 6 years ago

@markbates hmm... travis doesn't support ms sql server. would you consider to add appveyor integration? https://www.appveyor.com/

markbates commented 6 years ago

Sure. We use it for Buffalo too. Travis also supports docker, if that helps.


Mark Bates

On Mar 22, 2018, 4:59 PM -0400, marpio notifications@github.com, wrote:

@markbates hmm... travis doesn't support ms sql server. would you consider to add appveyor integration? https://www.appveyor.com/ — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub, or mute the thread.

marpio commented 6 years ago

@markbates true - i've found this: https://github.com/aspnet/EntityFrameworkCore/pull/7842/files I'll go with travis then.

mclark4386 commented 6 years ago

@marpio happy to help if you have questions! Sounds like you have a good start going.

marpio commented 6 years ago

@markbates short question - what is the purpose of the {db}_meta.go files in the fiz/translators package? Why doesn't postgres need it?

markbates commented 6 years ago

Some of the translators, like SQLite (for example), need additional meta information about the table(s), indexes, etc… to create the proper sql for things such as renaming a column.

Postgres is a nice db and things just work. :)

marpio commented 6 years ago

@markbates thanks

marpio commented 6 years ago

I'm having some difficulties - sqlx doesn't support the sqlserver parameter syntax so:

// this doesn't work
tx.MustExec("INSERT INTO person (first_name, last_name, email) VALUES ($1, $2, $3)", "Jason", "Moiron", "jmoiron@jmoiron.net")
// this works
tx.MustExec("INSERT INTO person (first_name, last_name, email) VALUES (@p1, @p2, @p3)", "Jason", "Moiron", "jmoiron@jmoiron.net")

// but this doesn't
tx.NamedExec("INSERT INTO person (first_name, last_name, email) VALUES (:first_name, :last_name, :email)", &Person{"Jane", "Citizen", "jane.citzen@example.com"}) 
// neither does this
tx.NamedExec("INSERT INTO person (first_name, last_name, email) VALUES (@first_name, @last_name, @email)", &Person{"Jane", "Citizen", "jane.citzen@example.com"}) 

there is this issue jmoiron/sqlx#374 regarding this problem but it doesn't seem to be active.

The second problem is that sqlserver doesn't support the usual LIMIT and OFFSET syntax so i would need to somehow override the buildPaginationClauses function...

Any ideas how this could be solved?

markbates commented 6 years ago

You probably need to implement your own TranslateSQL function like Postgres does https://github.com/gobuffalo/pop/blob/master/postgresql.go

marpio commented 6 years ago

I did that but the problem is, sqlx can not map to the fields in an struct, if i pass the params in form '@field_name' instead of ':field_name' so things like: tx.NamedExec("INSERT INTO person (first_name, last_name, email) VALUES (@first_name, @last_name, @email)", &Person{"Jane", "Citizen", "jane.citzen@example.com"}) sqlx does not recognize the '@field_name' and can't bind it. this doesn't work either:

st, _ := tx.PrepareNamed("INSERT INTO person (first_name, last_name, email) VALUES (@first_name, @last_name, @email)")
st.Exec(&Person{"Jane", "Citizen", "jane.citzen@example.com"})
marpio commented 6 years ago

Waiting for jmoiron/sqlx#406

stanislas-m commented 5 years ago

@marpio jmoiron/sqlx#406 was merged, and I just merged your PR in fizz. :)

marpio commented 5 years ago

Awesome @stanislas-m I will need some time to make it work since a lot changed in buffalo/pop since March.

stanislas-m commented 5 years ago

No problem, take your time!

Norris1z commented 5 years ago

@marpio any update on this?

marpio commented 5 years ago

@Norris1z I'm quite busy these days and not sure when I get to it. Probably in 2-3 months. Sorry for that. Feel free to take this one if you're interested!

andyedison commented 5 years ago

I'm interesting in this. I'm a little green when it comes to Go, and would want to chat about what the scope of work looks like before volunteering to take this on. Gopher slack the best place for that?

stanislas-m commented 5 years ago

@andyedison Sure, feel free to join the buffalo-dev channel on Gopher slack. :)