go-jet
Query builder should add components rather than replacing them. Example: Calling WHERE multiple times should add additional clauses, rather than replace the only clause
Describe the bug The query builder replaces rather than adds components and clauses. This goes against the behavior of every single query builder I've ever worked with, and is rather surprising and unintuitive, while also making some situations difficult or impossible.
Environment (please complete the following information):
- OS: linux and mac
- Database: postgres
- Database driver: pgx v5
- Jet version: 2.11.1
Code snippet
func (d DAO) SelectAllAccountsByFilter(ctx context.Context, filters models.Filters) ([]model.Accounts, error) {
query := SELECT(
Accounts.AllColumns,
).FROM(
Accounts,
)
if len(filters.Names) > 0 {
query = query.WHERE(Accounts.Name.IN(Strings(filters.Names)...))
}
if filters.Active != nil {
query = query.WHERE(Accounts.Active.EQ(Bool(*filters.Active)))
}
if len(filters.FavColors) > 0 {
query = query.WHERE(Accounts.FavColor.IN(Strings(filters.FavColors)...))
}
Expected behavior
When passed a Filters struct with all three values set, I expected to see an SQL statement that included 3 clauses in the WHERE portion.
Instead, only the last one was set.
I expect this kind of "builder" behavior for all parts of the query, including the SELECT, the FROM, the ORDER_BY, everything. (I should be able to call SELECT multiple times, to add new columns to be selected. Same for FROM, ORDER_BY, everything, etc.)
Why? Because building complex queries or queries for complex api's, are often dependent on logic, so there is often a need to change parts of the query based if statements.
Yes, I realize an ugly workaround would be to build it separately, then pass it into JET when finished, but besides being ugly, it defeats a lot of the purpose of a query builder.
What you describe as expected behavior is not an SQL builder but an ORM. SQL builder tries to mimics the underling sql without hiding any query details.
For instance:
if len(filters.Names) > 0 {
query = query.WHERE(Accounts.Name.IN(Strings(filters.Names)...))
}
if filters.Active != nil {
query = query.WHERE(Accounts.Active.EQ(Bool(*filters.Active)))
}
ORM will add silently AND between these two conditions, but sql builder should not. My only regret is not adding a panic if WHERE or any statement clause is called twice.
SQL builder tries to map golang DLS code to sql, one to one. Since sql SELECT query can have only one WHERE clause, sql builder SELECT statement should have only one WHERE clause.
condition := Bool(true)
if len(filters.Names) > 0 {
condition = condition.AND(Accounts.Name.IN(Strings(filters.Names)...)) // AND is not hidden
}
if filters.Active != nil {
condition = condition.AND(Accounts.Active.EQ(Bool(*filters.Active)))
}
if len(filters.FavColors) > 0 {
condition = condition.AND(Accounts.FavColor.IN(Strings(filters.FavColors)...))
}
// nice and clean
query := SELECT(
Accounts.AllColumns,
).FROM(
Accounts,
).WHERE(
condition,
)
Yes, I realize an ugly workaround would be to build it separately, then pass it into JET when finished, but besides being ugly, it defeats a lot of the purpose of a query builder.
Couldn't disagree more. When you are calling a go function, do you first call a function, and then modify it parameters, or you first calculate parameters and then call a function. It seems to me you spend to much time using GORM.
I have never used GORM, and I dislike ORM's quite a bit for many reasons.
I would consider an SQL builder to be something that builds an SQL statement.
Here, lets take github.com/Masterminds/squirrel for a try. It is a SQL builder, and that is all it does:
func (d DAO) SelectAllAccountsByFilter(ctx context.Context, filters models.Filters) ([]models.Account, error) {
query := sq.
Select(
"id",
"name",
"email",
"active",
"fav_color",
"fav_numbers",
"properties",
"created_at").
From("accounts").
OrderBy("id")
if len(filters.Names) > 0 {
query = query.Where(sq.Eq{"name": filters.Names})
}
if filters.Active != nil {
query = query.Where(sq.Eq{"active": *filters.Active})
}
if len(filters.FavColors) > 0 {
query = query.Where(sq.Eq{"fav_color": filters.FavColors})
}
sqlStr, args, err := query.PlaceholderFormat(sq.Dollar).ToSql()
if err != nil {
return nil, err
}
The result is as expected, if there are multiple filters, each of them will be added onto the WHERE statement.
This is one of the main purposes of an SQL builder, to let you build out an SQL statement following business logic that often requires you to add to the SQL based on the results of if statements.
If I knew exactly what the SQL should look like, without any if statements involved, then I would just write straight SQL myself without any need for involvement by an SQL builder. (I often already do that everywhere there isn't any logic involved.)
Having to "build" the SQL statement myself manually, before passing it into Jet's "sql builder", seems like a big miss.
But speaking practically, maybe there is an opportunity for something like a WHERE_AND function.
If you want to quote the SQL builder library, at least quote the real SQL builder library. For example, this is JOOQ for Java:
public Condition condition(HttpServletRequest request) {
Condition result = noCondition();
if (request.getParameter("title") != null)
result = result.and(BOOK.TITLE.like("%" + request.getParameter("title") + "%"));
if (request.getParameter("author") != null)
result = result.and(BOOK.AUTHOR_ID.in(
select(AUTHOR.ID).from(AUTHOR).where(
AUTHOR.FIRST_NAME.like("%" + request.getParameter("author") + "%")
.or(AUTHOR.LAST_NAME .like("%" + request.getParameter("author") + "%"))
)
));
return result;
}
// And then:
create.select()
.from(BOOK)
.where(condition(httpRequest))
.fetch();
By the looks of it, squirrel is more of an ORM than a SQL builder.
The result is as expected, if there are multiple filters, each of them will be added onto the WHERE statement.
Probably for the ORM users.
But speaking practically, maybe there is an opportunity for something like a WHERE_AND function.
For start WHERE_AND is not an SQL keyword. In some cases, it is worth adding a new statement method if there is a clear benefit. For example, the MODEL method of insert statement for type safety benefits. But, adding a new method just to support SQL builder anti-pattern programming is a big NO.
Squirrel is probably the most popular Golang SQL Builder library. It is not an ORM at all.
I don't think I would call squirrel a complete SQL Builder. Except for some basic sql keywords(like select, where, order by...), everything else is still basically a raw string. Meaning the risk of sql injection attacks is only slightly reduced compared to raw string.
Regarding WHERE_AND you can get something similar using global AND function:
conditions := []BoolExpression{Bool(true)}
if len(filters.Names) > 0 {
conditions = append(conditions, Accounts.Name.IN(Strings(filters.Names)...))
}
if filters.Active != nil {
conditions = append(conditions, Accounts.Active.EQ(Bool(*filters.Active)))
}
if len(filters.FavColors) > 0 {
conditions = append(conditions, Accounts.FavColor.IN(Strings(filters.FavColors)...))
}
query := SELECT(
Accounts.AllColumns,
).FROM(
Accounts,
).WHERE(AND(
conditions...,
),
)
If squirrel always adds AND after each Where, it means it is impossible to completely replace the where condition during SQL building phase. That feels like a limiting factor.
Squirrel leads to some truly unmaintainable code, was not a good experience in production.
I'm with jet on this one, though I can see where you're coming from. Squirrel is more sql generation using strings with go operations. We're targeting a transparent type safe way to build sql queries and scan them into native objects. Rather than simply utilizing go operations, go-jet takes a more go idiomatic / philosophical approach to sql generation.
ie. you can't write WHERE condition WHERE condition in SQL so you shouldn't be able to write it in Jet.
Squirrel is more like a generator where you throw it some inputs and get an output.
Unrelated and not a bash at squirrel but it seems to take all of the cons of SQL and Jet and mash them together, I hope we don't transition in that direction. The areas where squirrel shines makes for unmaintainable code as it's too complex to reason or makes a "don't touch this" query.
Squirrel chops up a string into go operations. It's not really fluent. And doesn't seem to serve much purpose for this tradeoff. As a "feature" you can do multiple .Where() and write raw sql again, which segments out a raw strings that are partial sql but also violate SQL. Worst of all worlds which I'll never understand.
SQL native.
SELECT * FROM elephants WHERE name IN ('Dumbo', 'Verna') AND age > 5;
Squirrel which chops sql strings and lets you violate sql as practice?
psql.Select("*").
From("elephants").
Where("name IN (?,?)", "Dumbo", "Verna").
Where("age > ?", 5).
ToSql()
Jet which doesn't chop strings and mimics SQL 1:1 + type safety (the reason for not writing sql)
SELECT(table.Elephants.AllColumns).
FROM(table.Elephants).
WHERE(table.Elephants.Name.In(names...).
AND(table.Elephants.Age.GT(5)
I think the trade-off is between doing things the old way (multiple WHERE allowed) and doing things the SQL way (one WHERE allowed). The single source of truth in go-jet is the database, hence the SQL way makes more sense.
@veqryn concern is valid though, a lot more people are used to the old way, not the SQL way.
I would suggest adding an entry in Wiki/FAQ/README, either explaining some design choices in go-jet, or documenting differences with other ORM.
Well, I guess I'll just leave it with this: It would be nice to have a WHERE_AND() function, ideally one that can be called without having to call WHERE first, so that you can put it in if/switch statements.