Add an opt-in query mod that scopes Expr paren suppression

queries/qm/query_mods.go

@@ -193,6 +193,21 @@ func Distinct(clause string) QueryMod {
 	}
 }
 
+type scopedExprParensQueryMod struct{}
+
+// Apply implements QueryMod.Apply.
+func (qm scopedExprParensQueryMod) Apply(q *queries.Query) {
+	queries.SetScopedExprParens(q)
+}
+
+// ScopedExprParens confines the suppression of automatic where-clause
+// parenthesization to the conditions inside the Expr that requested it.
+// Without it a single Expr leaves every other condition unbracketed, so a
+// condition holding a bare OR binds at the top level and swallows the rest.
+func ScopedExprParens() QueryMod {
+	return scopedExprParensQueryMod{}
+}
+
 type withQueryMod struct {
 	clause string
 	args   []interface{}

queries/query.go

@@ -44,6 +44,8 @@ type Query struct {
 	offset     int
 	forlock    string
 	distinct   string
+
+	scopedExprParens bool
 }
 
 // Applicator exists only to allow
@@ -244,6 +246,13 @@ func SetDistinct(q *Query, distinct string) {
 	q.distinct = distinct
 }
 
+// SetScopedExprParens confines the suppression of automatic where-clause
+// parenthesization to the conditions inside the Expr that requested it,
+// instead of disabling it for the whole query.
+func SetScopedExprParens(q *Query) {
+	q.scopedExprParens = true
+}
+
 // SetCount on the query.
 func SetCount(q *Query) {
 	q.count = true

queries/query_builders.go

@@ -337,13 +337,18 @@ func whereClause(q *Query, startAt int) (string, []interface{}) {
 		return "", nil
 	}
 
+	// Without scopedExprParens a single paren marker anywhere turns automatic
+	// bracketing off for every condition; with it, bracketing is decided per
+	// condition from the nesting depth, so the pre-scan has nothing to answer.
 	manualParens := false
-ManualParen:
-	for _, w := range q.where {
-		switch w.kind {
-		case whereKindLeftParen, whereKindRightParen:
-			manualParens = true
-			break ManualParen
+	if !q.scopedExprParens {
+	ManualParen:
+		for _, w := range q.where {
+			switch w.kind {
+			case whereKindLeftParen, whereKindRightParen:
+				manualParens = true
+				break ManualParen
+			}
 		}
 	}
 
@@ -352,6 +357,7 @@ ManualParen:
 	var args []interface{}
 
 	notFirstExpression := false
+	depth := 0
 	buf.WriteString(" WHERE ")
 	for _, where := range q.where {
 		if notFirstExpression && where.kind != whereKindRightParen {
@@ -364,9 +370,15 @@ ManualParen:
 			notFirstExpression = true
 		}
 
+		// A condition inside a paren group is already bounded by that group.
+		autoWrap := !manualParens
+		if q.scopedExprParens {
+			autoWrap = depth == 0
+		}
+
 		switch where.kind {
 		case whereKindNormal:
-			if !manualParens {
+			if autoWrap {
 				buf.WriteByte('(')
 			}
 			if q.dialect.UseIndexPlaceholders {
@@ -376,15 +388,17 @@ ManualParen:
 			} else {
 				buf.WriteString(where.clause)
 			}
-			if !manualParens {
+			if autoWrap {
 				buf.WriteByte(')')
 			}
 			args = append(args, where.args...)
 		case whereKindLeftParen:
 			buf.WriteByte('(')
+			depth++
 			notFirstExpression = false
 		case whereKindRightParen:
 			buf.WriteByte(')')
+			depth--
 		case whereKindIn:
 			ln := len(where.args)
 			// WHERE IN () is invalid sql, so it is difficult to simply run code like:
@@ -404,11 +418,11 @@ ManualParen:
 			// probably needs adjustment, or the user is passing in invalid clauses.
 			if matches == nil {
 				clause, count := convertInQuestionMarks(q.dialect.UseIndexPlaceholders, where.clause, startAt, 1, ln)
-				if !manualParens {
+				if autoWrap {
 					buf.WriteByte('(')
 				}
 				buf.WriteString(clause)
-				if !manualParens {
+				if autoWrap {
 					buf.WriteByte(')')
 				}
 				args = append(args, where.args...)
@@ -440,13 +454,13 @@ ManualParen:
 				leftClause = strings.Join(cols, ",")
 			}
 			rightClause, rightCount := convertInQuestionMarks(q.dialect.UseIndexPlaceholders, rightSide, startAt+leftCount, groupAt, ln-leftCount)
-			if !manualParens {
+			if autoWrap {
 				buf.WriteByte('(')
 			}
 			buf.WriteString(leftClause)
 			buf.WriteString(" IN ")
 			buf.WriteString(rightClause)
-			if !manualParens {
+			if autoWrap {
 				buf.WriteByte(')')
 			}
 			startAt += leftCount + rightCount

queries/scoped_expr_parens_test.go

@@ -0,0 +1,89 @@
+package queries_test
+
+import (
+	"testing"
+
+	"github.com/volatiletech/sqlboiler/drivers"
+	"github.com/volatiletech/sqlboiler/queries"
+	"github.com/volatiletech/sqlboiler/queries/qm"
+)
+
+// selectPrefix is what a select with no columns and one FROM table renders
+// before its WHERE clause.
+const selectPrefix = `SELECT * FROM "t"`
+
+// TestScopedExprParens pins both halves of the ScopedExprParens contract: a
+// query that does not apply the mod renders exactly as it did before the mod
+// existed, and one that does keeps automatic parentheses on every condition
+// outside the Expr group that suppressed them.
+//
+// The file is package queries_test because queries/qm imports queries, so an
+// in-package test could not reach the mod constructors.
+func TestScopedExprParens(t *testing.T) {
+	t.Parallel()
+
+	// An Expr group holding a bare OR. Every case carries it, so the
+	// suppression that ScopedExprParens confines is always triggered.
+	exprGroup := qm.Expr(qm.Where("a = 1"), qm.Or2(qm.Where("b = 2")))
+
+	tests := []struct {
+		name   string
+		optIn  bool
+		mods   []qm.QueryMod
+		expect string
+	}{
+		{
+			name:   "without the mod an Expr unbrackets the whole query",
+			optIn:  false,
+			mods:   []qm.QueryMod{exprGroup, qm.Where("c = 3 or d = 4")},
+			expect: ` WHERE (a = 1 OR b = 2) AND c = 3 or d = 4`,
+		},
+		{
+			name:   "with the mod conditions outside the group keep brackets",
+			optIn:  true,
+			mods:   []qm.QueryMod{exprGroup, qm.Where("c = 3 or d = 4")},
+			expect: ` WHERE (a = 1 OR b = 2) AND (c = 3 or d = 4)`,
+		},
+		{
+			name:  "nested groups gain brackets only at the top level",
+			optIn: true,
+			mods: []qm.QueryMod{
+				qm.Expr(qm.Where("a = 1"), qm.Expr(qm.Where("b = 2"))),
+				qm.Where("c = 3 or d = 4"),
+			},
+			expect: ` WHERE (a = 1 AND (b = 2)) AND (c = 3 or d = 4)`,
+		},
+		{
+			name:   "an IN condition outside the group keeps brackets",
+			optIn:  true,
+			mods:   []qm.QueryMod{exprGroup, qm.WhereIn("c in ?", 1, 2)},
+			expect: ` WHERE (a = 1 OR b = 2) AND ("c" IN ($1,$2))`,
+		},
+		{
+			// The empty-IN path writes its own brackets and returns early,
+			// so it must not pick up a second pair.
+			name:   "an empty IN condition stays singly bracketed",
+			optIn:  true,
+			mods:   []qm.QueryMod{exprGroup, qm.WhereIn("c in ?")},
+			expect: ` WHERE (a = 1 OR b = 2) AND (1=0)`,
+		},
+	}
+
+	for _, test := range tests {
+		t.Run(test.name, func(t *testing.T) {
+			q := &queries.Query{}
+			queries.SetDialect(q, &drivers.Dialect{LQ: '"', RQ: '"', UseIndexPlaceholders: true})
+			queries.SetFrom(q, "t")
+			qm.Apply(q, test.mods...)
+			if test.optIn {
+				qm.Apply(q, qm.ScopedExprParens())
+			}
+
+			result, _ := queries.BuildQuery(q)
+			expect := selectPrefix + test.expect + ";"
+			if result != expect {
+				t.Errorf("Mismatch between expect and result:\n%s\n%s\n", expect, result)
+			}
+		})
+	}
+}