fix(psql): address immutable query review feedback

Author: jay-babu

dialect/psql/dialect/delete.go

@@ -24,7 +24,8 @@ type DeleteQuery struct {
 }
 
 func (d *DeleteQuery) SetTargetOnly(only bool) {
-	d.Table.SetOnly(only)
+	d.Only = only
+	d.Table.SetOnly(false)
 }
 
 func (d *DeleteQuery) SetTargetTable(table any) {

dialect/psql/dialect/derive.go

@@ -97,6 +97,7 @@ func (base *DeleteQuery) Derive(queryMods ...bob.Mod[*DeleteQuery]) (*DeleteQuer
 			appendDerived[bob.Expression](&next.With.CTEs, base.With.CTEs, &cloneWith, m())
 		case mods.TargetOnly[*DeleteQuery]:
 			next.Only = bool(m)
+			next.Table.Only = false
 		case mods.TargetTable[*DeleteQuery]:
 			next.Table = cloneTableRef(clause.TableRef(m))
 		case mods.Where[*DeleteQuery]:

dialect/psql/dialect/update.go

@@ -25,7 +25,8 @@ type UpdateQuery struct {
 }
 
 func (u *UpdateQuery) SetTargetOnly(only bool) {
-	u.Table.SetOnly(only)
+	u.Only = only
+	u.Table.SetOnly(false)
 }
 
 func (u *UpdateQuery) SetTargetTable(table any) {

dialect/psql/dialect/writer.go

@@ -77,7 +77,7 @@ func (w *queryWriter) writeAny(value any) error {
 	case uint64:
 		_, _ = w.w.WriteString(strconv.FormatUint(v, 10))
 	case sql.NamedArg:
-		return fmt.Errorf("named args are not supported by psql dialect")
+		return bob.ErrNoNamedArgs
 	case bob.Expression:
 		return w.writeExpression(v)
 	default:

dialect/psql/immutable_select_test.go

@@ -2,9 +2,12 @@ package psql
 
 import (
 	"context"
+	"database/sql"
+	"errors"
 	"testing"
 
 	"github.com/stephenafamo/bob"
+	"github.com/stephenafamo/bob/dialect/psql/dialect"
 	"github.com/stephenafamo/bob/dialect/psql/sm"
 	"github.com/stephenafamo/bob/expr"
 )
@@ -74,6 +77,14 @@ func TestImmutableViewQueryWithDoesNotMutateOriginal(t *testing.T) {
 	}
 }
 
+func TestViewQueryWithNilReceiver(t *testing.T) {
+	var q *ViewQuery[*someStruct, []*someStruct]
+
+	if got := q.With(sm.Where(Quote("id").EQ(Arg(1)))); got != nil {
+		t.Fatalf("expected nil view query, got %#v", got)
+	}
+}
+
 func TestImmutableSelectQueryApplyDoesNotMutateOriginal(t *testing.T) {
 	base := Select(
 		sm.Columns("id"),
@@ -243,8 +254,12 @@ func TestViewSelectQueryHooksUseImmutableSelectQuery(t *testing.T) {
 	view := NewView[*someStruct, bob.Expression]("public", "some_struct", expr.ColsForStruct[someStruct]("some_struct"))
 
 	var hookSQL string
-	view.SelectQueryHooks.AppendHooks(func(ctx context.Context, exec bob.Executor, q *SelectQuery) (context.Context, error) {
-		sql, _, err := q.Build(ctx)
+	view.SelectQueryHooks.AppendHooks(func(ctx context.Context, exec bob.Executor, q *dialect.SelectQuery) (context.Context, error) {
+		sql, _, err := bob.BaseQuery[*dialect.SelectQuery]{
+			Expression: q,
+			Dialect:    dialect.Dialect,
+			QueryType:  bob.QueryTypeSelect,
+		}.Build(ctx)
 		if err != nil {
 			return ctx, err
 		}
@@ -272,6 +287,19 @@ func TestViewSelectQueryHooksUseImmutableSelectQuery(t *testing.T) {
 	}
 }
 
+func TestImmutableSelectQueryBuildReturnsErrNoNamedArgs(t *testing.T) {
+	_, _, err := Select(
+		sm.Columns(sql.Named("id", 1)),
+		sm.From("users"),
+	).Build(t.Context())
+	if err == nil {
+		t.Fatal("expected named arg error")
+	}
+	if !errors.Is(err, bob.ErrNoNamedArgs) {
+		t.Fatalf("expected ErrNoNamedArgs, got %v", err)
+	}
+}
+
 func TestImmutableSelectQueryApplySupportsCommonDerivedMods(t *testing.T) {
 	base := Select(
 		sm.Columns("users.id", "users.name"),

dialect/psql/immutable_write_test.go

@@ -147,6 +147,41 @@ func TestDeleteApplyDoesNotMutateOriginal(t *testing.T) {
 	}
 }
 
+func TestUpdateApplyDoesNotDuplicateOnly(t *testing.T) {
+	base := Update(
+		um.Table("films"),
+		um.SetCol("kind").ToArg("Drama"),
+		um.Only(),
+	)
+
+	derived := base.Apply(um.Only())
+
+	sql, _, err := derived.Build(t.Context())
+	if err != nil {
+		t.Fatal(err)
+	}
+	if sql != "UPDATE ONLY films SET\n\"kind\" = $1" {
+		t.Fatalf("unexpected update SQL: %#v", sql)
+	}
+}
+
+func TestDeleteApplyDoesNotDuplicateOnly(t *testing.T) {
+	base := Delete(
+		dm.From("films"),
+		dm.Only(),
+	)
+
+	derived := base.Apply(dm.Only())
+
+	sql, _, err := derived.Build(t.Context())
+	if err != nil {
+		t.Fatal(err)
+	}
+	if sql != "DELETE FROM ONLY films" {
+		t.Fatalf("unexpected delete SQL: %#v", sql)
+	}
+}
+
 func TestInsertApplyDoesNotMutateOriginal(t *testing.T) {
 	base := Insert(
 		im.Into("films"),

dialect/psql/view.go

@@ -57,7 +57,7 @@ type View[T any, Tslice ~[]T, C bob.Expression] struct {
 	Columns C
 
 	AfterSelectHooks bob.Hooks[Tslice, bob.SkipModelHooksKey]
-	SelectQueryHooks bob.Hooks[*SelectQuery, bob.SkipQueryHooksKey]
+	SelectQueryHooks bob.Hooks[*dialect.SelectQuery, bob.SkipQueryHooksKey]
 }
 
 func (v *View[T, Tslice, C]) Name() Expression {
@@ -100,10 +100,14 @@ func (v *View[T, Tslice, C]) Query(queryMods ...bob.Mod[*dialect.SelectQuery]) *
 type ViewQuery[T any, Ts ~[]T] struct {
 	SelectQuery
 	Scanner scan.Mapper[T]
-	Hooks   *bob.Hooks[*SelectQuery, bob.SkipQueryHooksKey]
+	Hooks   *bob.Hooks[*dialect.SelectQuery, bob.SkipQueryHooksKey]
 }
 
 func (q *ViewQuery[T, Ts]) With(queryMods ...bob.Mod[*dialect.SelectQuery]) *ViewQuery[T, Ts] {
+	if q == nil {
+		return nil
+	}
+
 	next := *q
 	next.SelectQuery = next.SelectQuery.Apply(queryMods...)
 	return &next
@@ -158,5 +162,5 @@ func (q *ViewQuery[T, Ts]) RunHooks(ctx context.Context, exec bob.Executor) (con
 		return ctx, nil
 	}
 
-	return q.Hooks.RunHooks(ctx, exec, &q.SelectQuery)
+	return q.Hooks.RunHooks(ctx, exec, q.SelectQuery.Expression)
 }

orm/query.go

@@ -28,7 +28,9 @@ func (q *ExecQuery[Q]) With(queryMods ...bob.Mod[Q]) *ExecQuery[Q] {
 	}
 
 	next := q.Clone()
-	next.BaseQuery = next.BaseQuery.Apply(queryMods...)
+	for _, mod := range queryMods {
+		mod.Apply(next.BaseQuery.Expression)
+	}
 	return &next
 }
 
@@ -79,7 +81,9 @@ func (q *Query[Q, T, Ts, Tr]) With(queryMods ...bob.Mod[Q]) *Query[Q, T, Ts, Tr]
 	}
 
 	next := q.Clone()
-	next.BaseQuery = next.BaseQuery.Apply(queryMods...)
+	for _, mod := range queryMods {
+		mod.Apply(next.BaseQuery.Expression)
+	}
 	return &next
 }