From 245912a905ed7d3552b52c4ae4b232d7c5e2f8f0 Mon Sep 17 00:00:00 2001 From: Akash Kumar <116457960+akashchamp@users.noreply.github.com> Date: Thu, 8 Oct 2026 01:03:01 +0530 Subject: [PATCH] fix(psql): include closing END in CASE expression span The walker computed the span of a CASE expression from its child nodes only, so it stopped at the ELSE result and left out the closing END. For a query such as UPDATE ... SET col = CASE ... END WHERE ..., the generated AppendSet range dropped the END and the built query was invalid SQL. Extend the span of a CaseExpr to its matching END token. Fixes #767 --- CHANGELOG.md | 2 + .../driver/parser/case_expr_test.go | 108 ++++++++++++++++++ gen/bobgen-psql/driver/parser/walker.go | 28 +++++ 3 files changed, 138 insertions(+) create mode 100644 gen/bobgen-psql/driver/parser/case_expr_test.go diff --git a/CHANGELOG.md b/CHANGELOG.md index bb5be7d4..c2d9c0b8 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -18,6 +18,8 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ### Fixed +- Fixed `bobgen-psql` cutting off the closing `END` of a `CASE` expression when building the mods for a query, so a query like `UPDATE ... SET col = CASE ... END WHERE ...` produced invalid SQL. See [#767](https://github.com/stephenafamo/bob/issues/767). + - Fixed the generated `Slice.copyMatchingRows` (used by `UpdateAll`, `ReloadAll`, and the `UpdateMod`/`DeleteMod`/`MergeMod` loaders) dropping relationship and plugin caches when refreshing a slice in place. It used to swap each slice element for a freshly scanned model after copying over only `.R`, silently losing `.C` and any other non-column field. It now copies just the column fields onto the existing model instead, so the slice keeps its original pointers and all its cached fields. See [#759](https://github.com/stephenafamo/bob/pull/759) for the full rationale. Note: code relying on pointer identity changing across `UpdateAll`/`ReloadAll` will now observe the same pointer, and models returned by a caller-driven `.All()` call are no longer mutated with `.R`. - **BREAKING:** Fixed the generated single-model `(*Foo).Update` and `(*Foo).Reload` silently clearing relationship and plugin caches on every call. Both methods used to overwrite the receiver wholesale (`*o = *v`) with the freshly scanned row, which zeroed out every non-column field — the relationship cache `.R` (including `R.Loaded`), the counts plugin's `.C`, and anything a plugin adds via `model/fields/additional`. They now copy only the column fields instead, so those caches survive. See [#758](https://github.com/stephenafamo/bob/pull/758) for the full rationale. diff --git a/gen/bobgen-psql/driver/parser/case_expr_test.go b/gen/bobgen-psql/driver/parser/case_expr_test.go new file mode 100644 index 00000000..c9572796 --- /dev/null +++ b/gen/bobgen-psql/driver/parser/case_expr_test.go @@ -0,0 +1,108 @@ +package parser + +import ( + "regexp" + "strconv" + "strings" + "sync/atomic" + "testing" + + pg "github.com/pganalyze/pg_query_go/v6" + pgparse "github.com/wasilibs/go-pgquery" +) + +var subExprRe = regexp.MustCompile(`q\.(\w+)\(EXPR\.subExpr\((\d+), (\d+)\)\)`) + +func TestCaseExprEnd(t *testing.T) { + t.Parallel() + + tests := []struct { + name string + input string + want map[string]string + }{ + { + name: "update set case", + input: `UPDATE item i +SET + name = CASE + WHEN $2 THEN $3 + ELSE i.name + END +WHERE i.id = $1`, + want: map[string]string{ + "AppendSet": `name = CASE + WHEN $1 THEN $2 + ELSE i.name + END`, + "AppendWhere": `i.id = $3`, + }, + }, + { + name: "update set nested case", + input: `UPDATE item SET name = CASE WHEN $1 THEN CASE WHEN $2 THEN 'a' ELSE 'b' END END WHERE id = $3`, + want: map[string]string{ + "AppendSet": `name = CASE WHEN $1 THEN CASE WHEN $2 THEN 'a' ELSE 'b' END END`, + "AppendWhere": `id = $3`, + }, + }, + { + name: "update where case", + input: `UPDATE item SET price = 1 WHERE CASE WHEN $1 THEN id = $2 ELSE false END`, + want: map[string]string{ + "AppendSet": `price = 1`, + "AppendWhere": `CASE WHEN $1 THEN id = $2 ELSE false END`, + }, + }, + } + + for _, tc := range tests { + t.Run(tc.name, func(t *testing.T) { + t.Parallel() + + scanResult, err := pgparse.Scan(tc.input) + if err != nil { + t.Fatalf("scan: %v", err) + } + + parseResult, err := pgparse.Parse(tc.input) + if err != nil { + t.Fatalf("parse: %v", err) + } + + w := walker{ + input: tc.input, + tokens: scanResult.GetTokens(), + mods: &strings.Builder{}, + names: make(map[position]string), + nullability: make(map[position]nullable), + groups: make(map[argPos]struct{}), + multiple: make(map[[2]int]struct{}), + atom: &atomic.Int64{}, + paramIdxMap: make(map[int64]int64), + } + + stmt := parseResult.Stmts[0].Stmt + info := w.walk(stmt) + w.modUpdateStatement(stmt.Node.(*pg.Node_UpdateStmt), info.children["UpdateStmt"]) + + formatted, err := w.formattedQuery() + if err != nil { + t.Fatalf("formattedQuery: %v", err) + } + + got := make(map[string]string) + for _, m := range subExprRe.FindAllStringSubmatch(w.mods.String(), -1) { + start, _ := strconv.Atoi(m[2]) + end, _ := strconv.Atoi(m[3]) + got[m[1]] = formatted[start:end] + } + + for method, want := range tc.want { + if got[method] != want { + t.Errorf("%s: got %q, want %q", method, got[method], want) + } + } + }) + } +} diff --git a/gen/bobgen-psql/driver/parser/walker.go b/gen/bobgen-psql/driver/parser/walker.go index 5609dee5..796c32af 100644 --- a/gen/bobgen-psql/driver/parser/walker.go +++ b/gen/bobgen-psql/driver/parser/walker.go @@ -183,6 +183,9 @@ func (w *walker) walk(a any) nodeInfo { w.maybeSetName(info.position(), w.input[info.start:info.end]) } + case *pg.CaseExpr: + info = w.walkCaseExpr(a) + case *pg.CoalesceExpr: info = w.reflectWalk(reflect.ValueOf(a)) for _, argInfo := range info.children["Args"].children { @@ -338,6 +341,31 @@ func (w *walker) walkAConst(a *pg.A_Const) nodeInfo { return info } +// walkCaseExpr extends the span of a CASE expression to its closing END +// keyword, which is not covered by any of its child nodes. +func (w *walker) walkCaseExpr(a *pg.CaseExpr) nodeInfo { + info := w.reflectWalk(reflect.ValueOf(a)) + if !info.isValid() { + return info + } + + depth := 0 + endInfo := w.findTokenAfterFunc(info.start, func(_ int, t *pg.ScanToken) bool { + switch t.Token { + case pg.Token_CASE: + depth++ + case pg.Token_END_P: + depth-- + } + return depth == 0 + }) + if endInfo.isValid() && endInfo.end > info.end { + info.end = endInfo.end + } + + return info +} + func (w *walker) walkSelectStmt(a *pg.SelectStmt) nodeInfo { if a == nil { return newNodeInfo()