goqu rework & Support for PostegreSQL, MySQL/MariaDB
CrescentKohana opened this issue · 19 comments
Hi! I'm looking for a OSS project to contribute to, and a manga reader is actually what I thought of creating. 😃
Regarding the issue, my Go experience is still limited, but I'm pretty sure that providing support to other DBMSs is more complicated than just modifying sql.Open("sqlite3", config.BuildDataPath("mangatsu.sqlite")).
I guess that the most difficult part would be configuring migrations, as the current ones are SQLite-specific. It would be great to have some kind of schema builder that allows for DB-agnostic migration generation - Laravel does that, and I think it works pretty well.
And I can see that some kind of SQL query builder is used, but github.com/mattn/go-sqlite3 is still imported in pkg/db/gallery.go and other files. I'm not sure, but creating query adapters for each DBMS could be a solution 😄
Looking forward to your reply!
Hi, and thanks for showing interest in this project!
Migrations
but I'm pretty sure that providing support to other DBMSs is more complicated than just modifying sql.Open("sqlite3", config.BuildDataPath("mangatsu.sqlite")).
Correct. Like you said, configuring the migrations would probably require some work as there are some differences in SQLite, MySQL and PSQL. PSQL could be easier to implement as SQLite somewhat follows its syntax IIRC.
Then again migrations in this project aren't that complicated, so it wouldn't probably take that much time to just create separate migrations for all three DBMSs if needed.
It would be great to have some kind of schema builder that allows for DB-agnostic migration generation - Laravel does that, and I think it works pretty well.
Currently I'm using Goose for the migrations. But I'm also open to changing this completely as it really doesn't affect the application itself. It also supports Go code in the migrations but I haven't looked into it that deeply.
Query Builder
And I can see that some kind of SQL query builder is used
Yes, I'm using Jet. Basically, it reads the schema of the initiated database and generates models and tables for the application. It's used to build the queries in pkg/db/*.
As a side-note, I didn't want to use an ORM for performance reasons so I went for a pure query builder based solution.
but
github.com/mattn/go-sqlite3is still imported in pkg/db/gallery.go and other files.
Those imports seem to be unnecessary and can be removed in all three files: gallery.go, library.go, user.go 😅.
Having said that, I think the real challenge is here: SQL statements and other db related functions are imported from Jet: github.com/go-jet/jet/v2/sqlite which complicate stuff from a syntax standpoint. Examples:
- InsertStatement is actually
sqlite.InsertStatement. I've just used a dot import to not have to prefix everything with sqlite. - Same with String (
sqlite.String) which is a internal type for Jet.
There are also some SQLite specific logic being used in the code like IFNULL (that seems to be in SQLite and MySQL but not PSQL).
Solution
creating query adapters for each DBMS
So yes, I think this is the best solution 😉. To make it more clear:
First we should probably create an abstraction layer which would be used in API and other packages. An environment variable would determine which adapter the abstraction layer uses at a given time.
We could create 3 directories: db/sqlite, db/mysql, db/psql all including the same statements with different Jet imports github.com/go-jet/jet/v2/sqlite, github.com/go-jet/jet/v2/mysql, github.com/go-jet/jet/v2/psql respectively with required changes to the code.
The most obvious negative with this would be the duplicate code, but that's inevitable. Maintainability shouldn't be a problem as the Jet models would help make sure that every query adapter has feature parity. Of course it'd be good idea to write tests for the query adapters as well.
Also, we would have to generate different types for all three query adapters but that shouldn't be a problem as it's just the matter of setting up the dbs and running commands. Structure should probably follow query adapters: types/sqlite/, types/mysql and types/psql.
.
I'll probably get back to this tomorrow and see how much changes would migrations and queries require.
We could also replace Jet to something else but that would be even larger task and I'm not sure if there are any other non-ORM query builders for Go.
// Made some edits to clear up my thoughts about this.
Okay I'm done with work stuff for today, time for Go 😄
After some searching I found goqu, a query builder that appears to be much simpler than Jet (it just generates SQL and that's it) - however, it allows for SQL generation for different SQL dialects - SQLite, PostgreSQL etc.
goqu appears to be more popular on GitHub than Jet, and in my opinion it looks a lot like Knex, a popular SQL builder for Node.js.
However, it comes with a cost - since it only generates SQL, no extra tooling is provided, so gallery.go, library.go, user.go would have to be rewritten in goqu.
The most obvious negative with this would be the duplicate code, but that's inevitable.
That's why I brought a different query builder in the first place, we can do something like this:
(note that I didn't test the following snippets)
// import database/sql, goqu, goqu dialects and DB drivers
// Database wraps around the sql.DB handle and a query builder
type Database struct {
Dialect string
Handle *sql.DB
QB goqu.DialectWrapper
}
// CreateDatabase creates a new Database object
func CreateDatabase(dialect string, connString string) (*Database, error) {
qb := goqu.Dialect(dialect)
handle, err := sql.Open(dialect, connString)
if err != nil {
return nil, err
}
db := &Database{dialect, handle, qb}
return db, nil
}
// GetQB returns the database's query builder
func (db *Database) GetQB() goqu.DialectWrapper {
return db.QB
}And then for queries (this is a quite verbose way of doing things, but I didn't want to change existing code ^^;):
// SelectQuery performs a SELECT query to the database with a given
// dataset built by the query builder
// (replace []interface{} with a concrete type, consider using generics for code reuse)
func (db *Database) SelectQuery(queryObject *SelectDataset) ([]interface{}, error) {
// there is a more idiomatic way of making queries with goqu:
// http://doug-martin.github.io/goqu/docs/dialect.html#executing-queries
sql, args, err := queryObject.ToSQL()
if err != nil {
return []interface{}, err
}
rows, err := db.Handle.Query(sql, ...args)
if err != nil {
return []interface{}, err
}
defer rows.Close()
// handle rows...
return result, nil
}And so, it's not database-dependent anymore 🙂
(Using an ORM would be the obvious solution, but if you don't want one, then it's totally fine, a query builder would do as well)
As for migrations, I haven't had looked too much into that. Maybe we could create a Makefile or something, that builds migrations for all supported DBMSs?
There's also migrate, where you can then apply them either by a CLI or by accessing the migrate library in Go.
Maybe Goose has a similar API for programmatic migration application?
import (
"database/sql"
_ "github.com/lib/pq"
"github.com/golang-migrate/migrate/v4"
"github.com/golang-migrate/migrate/v4/database/postgres"
_ "github.com/golang-migrate/migrate/v4/source/file"
)
func main() {
db, err := sql.Open("postgres", "postgres://localhost:5432/database?sslmode=enable")
// or in our case, `database.Handle` instead of `db`
driver, err := postgres.WithInstance(db, &postgres.Config{})
m, err := migrate.NewWithDatabaseInstance(
"file:///migrations",
"postgres", driver)
m.Up()
}After some searching I found goqu
Interesting. The downside with this would probably be the fact that it can't generate types, but to be fair they aren't that complex to begin with. Writing them manually would be pretty easy.
Using an ORM would be the obvious solution, but if you don't want one, then it's totally fine, a query builder would do as well
We could benchmark how much the difference would actually be with large collections of manga and tags. Though the performance is not the only reason why I'm relunctact in using an ORM. With "pure SQL", it's easier to fine tune and debug the queries. In the end SQL is a well designed language and doesn't really need a complex abstraction on top of it. But I'm not completely turning this idea away either.
And then for queries (this is a quite verbose way of doing things, but I didn't want to change existing code ^^;)
Yeah, I wasn't probably using the best practices when writing that, so a refactor isn't a bad idea. This is my second time writing Go for a project 😁.
goqu seems great and I'd say lets at least try that. For migrations, I'll test some things out with Goose, the migrate you linked and some other libraries as well.
It was pretty annoying to write migrations in pure SQL anyway, at least for SQLite, as in some cases I had to drop and create the whole table like when renaming a column.
Pushed some basic envs for databases.
Feel free to assign me to a task if there's any, I'll fork the repo and make a PR.
In the end SQL is a well designed language and doesn't really need a complex abstraction on top of it.
I think that a query builder has the advantage of being database-agnostic, which is of course great for this issue. Writing SQL by hand for all supported DBMSs would be horribly tedious.
Feel free to assign me to a task if there's any, I'll fork the repo and make a PR.
You could start by migrating db.go and library.go to goqu, and see how suitable it would be in reality for this project. They shouldn't have too much code to work with even if it doesn't turn out to be a success. I can create an issue for them.
I think that a query builder has the advantage of being database-agnostic, which is of course great for this issue. Writing SQL by hand for all supported DBMSs would be horribly tedious.
Yes, of course. That's why query builder is good here.
By the way, I had this thought that supporting both PostgreSQL and MySQL/MariaDB isn't too important currently. One of them is enough, as nowadays people use stuff like Docker which makes it really easy to just fire up any DB.
But of course it'd be great to have support for all three. Just saying this if there are any problems, (temporarily) dropping support for either one isn't a problem.
I can create an issue for them.
OK 😄
By the way, I had this thought that supporting both PostgreSQL and MySQL/MariaDB isn't too important currently. One of them is enough, as nowadays people use stuff like Docker which makes it really easy to just fire up any DB.
If I were to choose one, I'd pick PostgreSQL. People might want to deploy the server to Heroku, where Postgres is natively supported.
If I were to choose one, I'd pick PostgreSQL. People might want to deploy the server to Heroku, where Postgres is natively supported.
Agreed.
Sorry if it's a stupid question, but should I start working on #14, as you said in #5 (comment)? There are some new tasks in the OP.
Edit: I have just noticed that you assigned me to it 😃
Yeah I just created all relevant tasks. Also I just realized that we might not need any abstractions if goqu solves that. Anyway, there's the issue (#17) for the abstractions as well if needed.
Here are some problems with M$ Windows 😅
- In
example.env, relative paths seem to not work with Windows 10. - The
MTSU_BASE_PATHSlooks like this:
MTSU_BASE_PATHS: freeform1;/home/user/doujinshi;;structured2:/home/user/manga
and should look like this:
MTSU_BASE_PATHS: freeform1;/home/user/doujinshi;;structured2;/home/user/manga
(colon replaced with a semicolon afterstructured2)
Edit: Maybe there should be a new issue on it?
- These work on my Windows machine
MTSU_BASE_PATHS=freeform1;devarchive/freeform;;structured2;devarchive/structured- devarchive/freeform is in the same level as pkg, docs etc.
- Yes that should be a semicolon. I'll fix it.
@karmek-k Forgot to mention, for Postgres we should probably use pgx as the driver. The other large one lib/pq is also recommending it. Though it's not that big deal.
IDK if goqu works with pgx, the README says that it doesn't implement database/sql. Right now I'm rewriting library.go, I'd just stick with lib/pq for now, we probably don't need blazing fast speeds 😃
And pgx also says in the README that it's not recommended for applications supporting more than PostgreSQL.
Yup. Lets go with lib/pq 👍.
Made a new branch where we can merge the rework more freely: https://github.com/Mangatsu/server/tree/goqu-rework