zerodha/dungbeetle

Adding new drivers

Closed this issue · 5 comments

Hi, i'm playing with sql-jobber ... and asking if could be interesting adding new drivers

https://github.com/sijms/go-ora
_ "github.com/sijms/go-ora/v2" // sql.Open("oracle", "oracle://OT:yourpassword@localhost/XE")

https://github.com/mithrandie/csvq-driver
_ "github.com/mithrandie/csvq-driver" // sql.Open("csvq", "/path/to/data/directory")

And even.. https://pkg.go.dev/cloud.google.com/go/bigquery

I played successfully with the 2 first (i consider use sql-jobber + csvq as option to generate csv files from queries... a prof of concept :) but this leadme to change sqldb.go (backend) in some place to make it csvq dialect compatible with the actual code ... adding new drivers may lead to spread ifs like this

// This will be filled by the driver. if w.backend.opt.DBType == dbTypePostgres { // Postgres placeholders are $1, $2 ... colValHolder[i] = fmt.Sprintf("$%d", i+1) } else { colValHolder[i] = "?" }
or maybe refactor in some way, maybe adding new backend for "non standard sql databases"...

what do you think about it?... if consider interesting i can submit a PR.

knadh commented

Hi @josejuanmontiel. This is definitely interesting and the more database SQL jobber can support, the more useful it is. Do you have any ideas for an architectural change? I guess there's no way but to hardcode support for multiple DBs into the core. If only Go had a usable plugin system!

Hi... i think we could talk the architectural here because all options incluide change and organice the code in some way to differenciate for specific cases and reuse where it't make sense... i'll try to open a PR in draft in the next days to see this changes in my case (oracle and csvq)... and talk about those...

Hi! Sorry for the delay... the idea could be something like this... #21 still need some refactor... but this could be the idea to reuse the base "sql code" in another kind of tricky sql database.. what do you think?

knadh commented

Hi @josejuanmontiel. A little confused by the PR. This seems to have a new CSV backend too. Could you describe the PR in some detail?

I commented... hope that understand... but, in resumen i decided implement new CSV backend... better than ifs ... to later import dinamically this code as plugin... and better readability i think :)