refactor(psql): simplify immutable query internals by jay-babu · Pull Request #657 · stephenafamo/bob

jay-babu · 2026-04-21T16:10:30Z

Summary

Relative to upstream/main, this PR moves Bob further toward immutable-by-default query building and rewrites the hot psql query path so that immutable query derivation stays competitive with or ahead of the current mutable Apply path.

What this PR changes:

makes bob.BaseQuery immutable-by-default: Apply(...) now returns a derived query instead of mutating in place
updates orm.ExecQuery and orm.Query to follow the same immutable semantics
updates mysql/sqlite table and view wrappers to work with immutable Apply(...)
refactors psql to use a single query representation instead of a parallel immutable wrapper model
adds Clone() support and immutable derivation support for the psql dialect query structs
specializes the psql writer path to keep immutable derivation fast enough to beat the current upstream baselines on the main benchmark shapes
simplifies some of the supporting psql wrapper and table/view logic after the core rewrite

Client migration

The main breaking change is semantic:

Apply(...) no longer mutates queries in place
Apply(...) now behaves like With(...): it returns a derived query

So code like this on upstream/main:

q := psql.Select(sm.From("users"))
q.Apply(sm.Where(psql.Quote("id").EQ(psql.Arg(1))))

must become:

q := psql.Select(sm.From("users"))
q = q.Apply(sm.Where(psql.Quote("id").EQ(psql.Arg(1))))

The same applies to pointer-returning wrappers such as orm.Query, orm.ExecQuery, table queries, and view queries:

q := userTable.Update(...)
q = q.Apply(um.Where(...))

If the existing code was already written as:

q = q.Apply(...), or
q2 := q.With(...)

then no behavior change is required.

Practical migration notes by layer:

bob.BaseQuery: always capture the return value of Apply(...)
orm.ExecQuery / orm.Query: always capture the return value of Apply(...)
dialect wrappers like psql.SelectQuery, psql.UpdateQuery, psql.DeleteQuery, psql.InsertQuery: always capture the return value of Apply(...)
table/view helpers in psql, mysql, and sqlite: same rule, always capture the return value of Apply(...)

What does not change:

With(...) still returns a derived query
constructors such as psql.Select(...), psql.Update(...), psql.Delete(...), psql.Insert(...), view.Query(...), and table helpers still exist
SQL shape and result behavior are intended to remain the same apart from immutable query semantics

Main areas changed

Core immutable semantics:

query.go
orm/query.go
query_immutable_test.go

psql query core and writers:

dialect/psql/dialect/clone.go
dialect/psql/dialect/derive.go
dialect/psql/dialect/writer.go
dialect/psql/dialect/select.go
dialect/psql/dialect/update.go
dialect/psql/dialect/delete.go
dialect/psql/dialect/insert.go

psql wrappers and table/view behavior:

dialect/psql/select.go
dialect/psql/update.go
dialect/psql/delete.go
dialect/psql/insert.go
dialect/psql/view.go
dialect/psql/table.go
dialect/psql/{sm,um,dm,im}/qm.go

Compatibility fallout for other dialect wrappers:

dialect/mysql/{view,table}.go
dialect/sqlite/{view,table}.go

Verification

Ran:

go test ./dialect/psql ./dialect/psql/dialect ./orm ./dialect/mysql ./dialect/sqlite
go test ./dialect/psql -run '^$' -bench '^Benchmark(BaseQuery|ViewQueryCountThenPaginate|UpdateQuery|DeleteQuery|InsertQuery)(ApplyMain|ImmutableNativeHotPath)$' -benchmem

Latest immutable benchmark snapshot on this branch:

BenchmarkBaseQueryImmutableNativeHotPath: 2931 ns/op, 3149 B/op, 35 allocs/op
BenchmarkViewQueryCountThenPaginateImmutableNativeHotPath: 5941 ns/op, 6316 B/op, 62 allocs/op
BenchmarkUpdateQueryImmutableNativeHotPath: 2601 ns/op, 2575 B/op, 43 allocs/op
BenchmarkDeleteQueryImmutableNativeHotPath: 1962 ns/op, 2156 B/op, 27 allocs/op
BenchmarkInsertQueryImmutableNativeHotPath: 1507 ns/op, 1720 B/op, 18 allocs/op

Upstream/main baselines used for comparison:

BenchmarkBaseQueryApplyMain: 3039 ns/op, 2865 B/op, 48 allocs/op
BenchmarkViewQueryCountThenPaginateApplyMain: 10070 ns/op, 7424 B/op, 130 allocs/op
BenchmarkUpdateQueryApplyMain: 3038 ns/op, 2319 B/op, 47 allocs/op
BenchmarkDeleteQueryApplyMain: 1926 ns/op, 1956 B/op, 33 allocs/op
BenchmarkInsertQueryApplyMain: 1473 ns/op, 1576 B/op, 24 allocs/op

Notes

This PR still adds complexity in the psql specialization layer, mainly in:

dialect/psql/dialect/writer.go
dialect/psql/dialect/derive.go

That is the current cost of keeping the immutable path at or above the upstream performance bar on the benchmarked query shapes.

jay-babu · 2026-04-21T16:16:25Z

One migration decision worth calling out separately: we now have two viable paths for client-facing semantics.

Remove Apply(...) and force all clients onto With(...).
This is the cleaner API because there is only one immutable composition path, and the migration is mechanically understandable: find places that currently call q.Apply(...) and switch them to q = q.With(...) (or equivalent reassignment). That makes the touched call sites explicit and avoids clients having to remember that Apply(...) changed meaning.
Keep Apply(...) and let clients absorb the semantic change.
This minimizes immediate source edits in some codebases, but it pushes the cognitive cost onto clients because Apply(...) no longer behaves the way it did on upstream/main. In practice, clients then need to audit every place that previously relied on in-place mutation and make sure the returned query is captured.

My bias is that if we are willing to take a breaking change anyway, removing Apply(...) is the cleaner long-term API. If we keep it, we should treat the migration burden as a real client cost and document the exact call sites to audit: psql.Select, view.Query, table queries, orm.Query, and orm.ExecQuery derivation chains.

jay-babu · 2026-04-21T16:16:56Z

@stephenafamo Here is the PR migrating everything to be immutable by design

Copilot

Pull request overview

This PR continues the “immutable-by-default” refactor for Bob’s query types, simplifying psql-specific wrappers/derivation while preserving the specialized psql writer path and benchmark performance.

Changes:

Make bob.BaseQuery (and ORM query wrappers) immutable-by-default via With()/Apply() returning derived copies.
Introduce simplified psql query wrappers (SelectQuery/InsertQuery/DeleteQuery/UpdateQuery) and dialect-level derivation/cloning helpers.
Update psql/mysql/sqlite table/view entrypoints and add regression + immutability tests/benchmarks.

Reviewed changes

Copilot reviewed 32 out of 32 changed files in this pull request and generated 7 comments.

Show a summary per file

File	Description
query.go	`BaseQuery` cloning now preserves `QueryType`; adds immutable `With()` and makes `Apply()` return a derived copy.
query_immutable_test.go	Adds core immutability tests for `BaseQuery`.
orm/query.go	Adds immutable `With()`/`Apply()` for ORM `ExecQuery` and `Query` and fixes cloning to preserve `Scanner`.
mods/mods.go	Adds shared mods (`TargetOnly`, `TargetTable`, `Distinct`, etc.) to reduce dialect glue.
dialect/psql/select.go / insert.go / delete.go / update.go	Adds lightweight psql wrapper query types with `With()`/`Apply()` (and derivation where supported).
dialect/psql/dialect/{select,insert,update,delete}.go	Refactors SQL writing to use new internal `queryWriter` and adds setter methods used by shared mods.
dialect/psql/dialect/{derive,clone}.go (+ tests)	Adds derivation fast-paths and deep-clone helpers to support immutability safely.
dialect/psql/view.go (+ tests)	Refactors view query wrapper to embed `SelectQuery`, updates hooks plumbing, and adds Count/scan path.
dialect/psql/*_test.go (multiple)	Adds regression/immutability coverage for With/Apply behavior and default RETURNING behavior.
dialect/mysql/{table,view}.go & dialect/sqlite/{table,view}.go	Updates table/view starters to respect immutable Apply/With semantics; adds MySQL insert query `With()`/`Apply()`.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

stephenafamo · 2026-04-24T13:07:13Z

Is there still any need for q.With()?
I see a lot of additional changes to psql that were not also made to mysql and sqlite. Is it that only psql is now immutable?

jay-babu · 2026-05-03T17:00:57Z

@stephenafamo I can remove With. That was there to introduce it as a nonbreaking change first
The base of code is immutable, but I did not focus on going through and making mysql and sqlite immutable by design

Benchmarks:\n- BenchmarkUpdateQueryApplyMain: 3038 ns/op, 2319 B/op, 47 allocs/op\n- BenchmarkUpdateQueryImmutableNativeHotPath: 1974 ns/op, 1360 B/op, 41 allocs/op\n- BenchmarkDeleteQueryApplyMain: 1926 ns/op, 1956 B/op, 33 allocs/op\n- BenchmarkDeleteQueryImmutableNativeHotPath: 1345 ns/op, 1027 B/op, 26 allocs/op\n- BenchmarkInsertQueryApplyMain: 1473 ns/op, 1576 B/op, 24 allocs/op\n- BenchmarkInsertQueryImmutableNativeHotPath: 1143 ns/op, 920 B/op, 20 allocs/op

Apply now matches With for psql select/view/insert/update/delete wrappers, so derivation no longer mutates the source query. Added regression coverage for immutable Apply semantics and preserved fallback SQL behavior for combined/distinct select shapes. Benchmarks versus stored upstream/main baselines: - BaseQuery immutable: 2404 ns/op vs 3039 ns/op upstream Apply - View count+paginate immutable: 5906 ns/op vs 10070 ns/op upstream Apply - Update immutable: 2147 ns/op vs 3038 ns/op upstream Apply - Delete immutable: 1587 ns/op vs 1926 ns/op upstream Apply - Insert immutable: 1295 ns/op vs 1473 ns/op upstream Apply

Remove the wrapper-level cached mutable BaseQuery compatibility form from psql select/update/delete/insert wrappers. Verification: - go test ./dialect/psql - go test ./dialect/psql -run '^$' -bench '^Benchmark(BaseQuery|ViewQueryCountThenPaginate|UpdateQuery|DeleteQuery|InsertQuery)(ApplyMain|ImmutableNativeHotPath)$' -benchmem Benchmark snapshot after cleanup: - BaseQuery immutable: 2357 ns/op, 1705 B/op, 36 allocs/op - View count+paginate immutable: 5811 ns/op, 4731 B/op, 60 allocs/op - Update immutable: 2111 ns/op, 1360 B/op, 41 allocs/op - Delete immutable: 1555 ns/op, 1027 B/op, 26 allocs/op - Insert immutable: 1253 ns/op, 920 B/op, 20 allocs/op

Remove the unused defaultSelect field from ViewQuery, keep only the one bit of state that matters for overriding the auto-generated select list, and name the mutable writer fallback path explicitly. Verification: - go test ./dialect/psql - go test ./dialect/psql -run '^$' -bench '^Benchmark(BaseQuery|ViewQueryCountThenPaginate|UpdateQuery|DeleteQuery|InsertQuery)(ApplyMain|ImmutableNativeHotPath)$' -benchmem Benchmark snapshot after cleanup: - BaseQuery immutable: 2410 ns/op, 1704 B/op, 36 allocs/op - View count+paginate immutable: 5655 ns/op, 4538 B/op, 60 allocs/op - Update immutable: 2092 ns/op, 1360 B/op, 41 allocs/op - Delete immutable: 1558 ns/op, 1027 B/op, 26 allocs/op - Insert immutable: 1283 ns/op, 920 B/op, 20 allocs/op

Move default view-column handling into immutable select state and route ViewQuery execution directly through bob.One/All/Cursor/Each instead of wrapper-local baseQuery/mutable helpers. Added a regression test to ensure an explicit view Select mod replaces the generated default columns. Verification: - go test ./dialect/psql ./orm ./mods ./clause ./dialect/mysql ./dialect/sqlite - go test ./dialect/psql -run '^$' -bench '^Benchmark(BaseQuery|ViewQueryCountThenPaginate|UpdateQuery|DeleteQuery|InsertQuery)(ApplyMain|ImmutableNativeHotPath)$' -benchmem Benchmark snapshot after cleanup: - BaseQuery immutable: 2468 ns/op, 1705 B/op, 36 allocs/op - View count+paginate immutable: 5982 ns/op, 4674 B/op, 57 allocs/op - Update immutable: 2209 ns/op, 1360 B/op, 41 allocs/op - Delete immutable: 1606 ns/op, 1027 B/op, 26 allocs/op - Insert immutable: 1253 ns/op, 920 B/op, 20 allocs/op

Remove the public wrapper-level baseQuery compatibility methods from psql select/update/delete/insert types and update internal callers to use the immutable internals directly. Verification: - go test ./dialect/psql ./orm ./mods ./clause ./dialect/mysql ./dialect/sqlite - go test ./dialect/psql -run '^$' -bench '^Benchmark(BaseQuery|ViewQueryCountThenPaginate|UpdateQuery|DeleteQuery|InsertQuery)(ApplyMain|ImmutableNativeHotPath)$' -benchmem Benchmark snapshot after cleanup: - BaseQuery immutable: 2369 ns/op, 1705 B/op, 36 allocs/op - View count+paginate immutable: 5711 ns/op, 4674 B/op, 57 allocs/op - Update immutable: 2097 ns/op, 1360 B/op, 41 allocs/op - Delete immutable: 1568 ns/op, 1027 B/op, 26 allocs/op - Insert immutable: 1291 ns/op, 920 B/op, 20 allocs/op

Replace the psql table query contextual mods that injected default RETURNING columns with constructor-time defaults, while preserving explicit RETURNING overrides. Added regression coverage for update/insert/delete table queries to assert both the default RETURNING-all-columns behavior and explicit RETURNING override behavior. Verification: - go test ./dialect/psql - go test ./dialect/psql -run '^$' -bench '^Benchmark(BaseQuery|ViewQueryCountThenPaginate|UpdateQuery|DeleteQuery|InsertQuery)(ApplyMain|ImmutableNativeHotPath)$' -benchmem Benchmark snapshot after cleanup: - BaseQuery immutable: 2329 ns/op, 1705 B/op, 36 allocs/op - View count+paginate immutable: 5759 ns/op, 4674 B/op, 57 allocs/op - Update immutable: 2138 ns/op, 1360 B/op, 41 allocs/op - Delete immutable: 1577 ns/op, 1027 B/op, 26 allocs/op - Insert immutable: 1214 ns/op, 920 B/op, 20 allocs/op

Benchmarks:\n- BenchmarkBaseQueryImmutableNativeHotPath: 2938 ns/op, 3149 B/op, 35 allocs/op\n- BenchmarkViewQueryCountThenPaginateImmutableNativeHotPath: 6092 ns/op, 6316 B/op, 62 allocs/op\n- BenchmarkUpdateQueryImmutableNativeHotPath: 2561 ns/op, 2543 B/op, 41 allocs/op\n- BenchmarkDeleteQueryImmutableNativeHotPath: 1979 ns/op, 2156 B/op, 27 allocs/op\n- BenchmarkInsertQueryImmutableNativeHotPath: 1532 ns/op, 1720 B/op, 18 allocs/op

Benchmarks:\n- BenchmarkBaseQueryImmutableNativeHotPath: 2868 ns/op, 3149 B/op, 35 allocs/op\n- BenchmarkViewQueryCountThenPaginateImmutableNativeHotPath: 5948 ns/op, 6316 B/op, 62 allocs/op\n- BenchmarkUpdateQueryImmutableNativeHotPath: 2509 ns/op, 2543 B/op, 41 allocs/op\n- BenchmarkDeleteQueryImmutableNativeHotPath: 1957 ns/op, 2156 B/op, 27 allocs/op\n- BenchmarkInsertQueryImmutableNativeHotPath: 1487 ns/op, 1720 B/op, 18 allocs/op

Benchmarks:\n- BenchmarkBaseQueryImmutableNativeHotPath: 2857 ns/op, 3149 B/op, 35 allocs/op\n- BenchmarkViewQueryCountThenPaginateImmutableNativeHotPath: 5767 ns/op, 6316 B/op, 62 allocs/op\n- BenchmarkUpdateQueryImmutableNativeHotPath: 2532 ns/op, 2576 B/op, 43 allocs/op\n- BenchmarkDeleteQueryImmutableNativeHotPath: 1872 ns/op, 2156 B/op, 27 allocs/op\n- BenchmarkInsertQueryImmutableNativeHotPath: 1422 ns/op, 1720 B/op, 18 allocs/op

Benchmarks:\n- BenchmarkBaseQueryImmutableNativeHotPath: 2726 ns/op, 3149 B/op, 35 allocs/op\n- BenchmarkViewQueryCountThenPaginateImmutableNativeHotPath: 5769 ns/op, 6315 B/op, 62 allocs/op\n- BenchmarkUpdateQueryImmutableNativeHotPath: 2477 ns/op, 2576 B/op, 43 allocs/op\n- BenchmarkDeleteQueryImmutableNativeHotPath: 1856 ns/op, 2156 B/op, 27 allocs/op\n- BenchmarkInsertQueryImmutableNativeHotPath: 1375 ns/op, 1720 B/op, 18 allocs/op

Benchmarks:\n- BenchmarkBaseQueryImmutableNativeHotPath: 2931 ns/op, 3149 B/op, 35 allocs/op\n- BenchmarkViewQueryCountThenPaginateImmutableNativeHotPath: 5941 ns/op, 6316 B/op, 62 allocs/op\n- BenchmarkUpdateQueryImmutableNativeHotPath: 2601 ns/op, 2575 B/op, 43 allocs/op\n- BenchmarkDeleteQueryImmutableNativeHotPath: 1962 ns/op, 2156 B/op, 27 allocs/op\n- BenchmarkInsertQueryImmutableNativeHotPath: 1507 ns/op, 1720 B/op, 18 allocs/op

jay-babu · 2026-05-28T15:01:12Z

@stephenafamo what will it take to get this merged?

stephenafamo · 2026-05-29T12:19:29Z

Sorry I ignored this for a while... I'm just conflicted because of how big of a breaking change this is.

I'm considering having a side-by-side implementation, i.e. psql.Select and psql.Select2 along with a flag for the code generation to switch between them to allow for less painful migration.

Copilot AI review requested due to automatic review settings April 21, 2026 16:10

Copilot started reviewing on behalf of jay-babu April 21, 2026 16:10 View session

Copilot AI reviewed Apr 21, 2026

View reviewed changes

jay-babu added 23 commits May 3, 2026 17:03

feat(psql): make derived select queries immutable

ea9d2cb

refactor(psql): route select with through immutable queries

bb27f9f

refactor(psql): hide immutable query internals

2eb36df

test(psql): add with regression coverage

230c162

test(psql): use test contexts in query tests

835cb44

feat(psql): support native immutable combined selects

5345cd6

refactor(orm): make table queries immutable by default

9c7f391

refactor(psql): run view hooks on immutable queries

f69f122

refactor(psql): drop dead immutable contextual state

cb9440e

refactor(psql): remove dead immutable select fallback

29f2602

refactor(psql): support common immutable select mods

d16c25a

refactor(psql): support common immutable write mods

0a91ca3

refactor(psql): build table queries directly

faf981f

refactor(psql): localize table returning behavior

8f279d9

perf(psql): build common queries immutably

5e0b358

refactor(query): make base queries immutable by default

831ca50

jay-babu added 10 commits May 3, 2026 17:04

refactor(psql): add clone support for dialect queries

aa059fe

refactor(psql): use a single immutable query core

2710d2b

refactor(psql): simplify table returning defaults

a64a500

fix(psql): address immutable query review feedback

46ec3ae

refactor: remove query With methods

d6e79df

jay-babu force-pushed the immutable-psql-single-core branch from 434568a to d6e79df Compare May 3, 2026 17:23

stephenafamo mentioned this pull request Jun 10, 2026

Auto-inject LIMIT 1 in bob.One #673

Open

snowyukitty mentioned this pull request Jul 11, 2026

perf(psql): use EXISTS for view checks #735

Open

Navigation Menu

Uh oh!

refactor(psql): simplify immutable query internals - #657

refactor(psql): simplify immutable query internals#657
jay-babu wants to merge 33 commits into
stephenafamo:mainfrom
jay-babu:immutable-psql-single-core

jay-babu commented Apr 21, 2026 •

edited

Loading

Uh oh!

jay-babu commented Apr 21, 2026

Uh oh!

jay-babu commented Apr 21, 2026

Uh oh!

Copilot AI left a comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

stephenafamo commented Apr 24, 2026

Uh oh!

jay-babu commented May 3, 2026

Uh oh!

jay-babu commented May 28, 2026

Uh oh!

stephenafamo commented May 29, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Uh oh!

Conversation

jay-babu commented Apr 21, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

Client migration

Main areas changed

Verification

Notes

Uh oh!

jay-babu commented Apr 21, 2026

Uh oh!

jay-babu commented Apr 21, 2026

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

stephenafamo commented Apr 24, 2026

Uh oh!

jay-babu commented May 3, 2026

Uh oh!

jay-babu commented May 28, 2026

Uh oh!

stephenafamo commented May 29, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

jay-babu commented Apr 21, 2026 •

edited

Loading