squirrel icon indicating copy to clipboard operation
squirrel copied to clipboard

Auto-parenthesis for Where()

Open dany74q opened this issue 7 months ago • 0 comments

Greets !

Quick suggestion to hear the community's thoughts before opening a PR:

I recently noticed that if one would use .Where() and pass it multiple OR clauses, say, without using proper sq.Or, the statement would not be parenthesized:

	sql, _, _ := sq.Select("*").
		From("table").
		Where("a = ?", 1).
		Where("b = ? OR c = ?", 1, 2, 3).
		ToSql()

	println(sql)

---
SELECT * FROM table WHERE a = ? AND b = ? OR c = ?

This is unlike some other query builders, for instance in gorm, the same usage is parenthesized:

	db, _ := gorm.Open("postgres", "...")

	expr := db.Select("*").
		Table("table").
		Where("a = ?", 1).
		Where("b = ? OR c = ?", 2, 3).
		QueryExpr()

	field := reflect.ValueOf(expr).Elem().FieldByName("expr")
	fieldPtr := unsafe.Pointer(field.UnsafeAddr())

	fieldRef := reflect.NewAt(field.Type(), fieldPtr)
	println(fieldRef.Elem().Interface().(string))
---
SELECT * FROM "table"  WHERE (a = ?) AND (b = ? OR c = ?)

Even though sq providers a proper Or primitive, the above can still be mistakenly used, which could lead to insidious bugs where the latter part of the OR statement overrides predicates before it.

I was wondering if, maybe via an opt-in flag to be backwards compatible, it would make sense to auto-parenthesize statements inside Where() - it could be done for all statements, or only for those that contains "OR" statements.

Let me know the sentiment if this is something we're willing to push, and I'll open a PR.

dany74q avatar Jul 21 '24 16:07 dany74q