-
-
Notifications
You must be signed in to change notification settings - Fork 6
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
goqu rework & Support for PostegreSQL, MySQL/MariaDB #5
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 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 Looking forward to your reply! |
Hi, and thanks for showing interest in this project! Migrations
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.
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
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 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.
Those imports seem to be unnecessary and can be removed in all three files: Having said that, I think the real challenge is here: SQL statements and other db related functions are imported from 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
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: 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: .I'll probably get back to this tomorrow and see how much changes would migrations and queries require.
// 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. However, it comes with a cost - since it only generates SQL, no extra tooling is provided, so
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 🙂 As for migrations, I haven't had looked too much into that. Maybe we could create a There's also migrate, where you can then apply them either by a CLI or by accessing the 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()
} |
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.
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.
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. |
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. |
You could start by migrating
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. |
OK 😄
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 😅
Edit: Maybe there should be a new issue on it? |
|
IDK if goqu works with pgx, the README says that it doesn't implement And pgx also says in the README that it's not recommended for applications supporting more than PostgreSQL. |
Yup. Lets go with |
Made a new branch where we can merge the rework more freely: https://github.com/Mangatsu/server/tree/goqu-rework |
The rework is being work on in https://github.com/Mangatsu/server/tree/goqu-rework
Currently only SQLite3 is supported. Support for more databases would be nice.
Supported databases
Tasks
The text was updated successfully, but these errors were encountered: