Repository navigation
Export the count Preloader's SELECT-list column as orm.CountPreloadColumn - #763
Open
sandonemaki wants to merge 5 commits into
Open
sandonemaki wants to merge 5 commits into
sandonemaki wants to merge 5 commits into
Conversation
…lumn The generated count Preloader (gen/templates/counts, PreloadCount.*) appends its correlated-subquery column to a query's preload columns via bob.Query's AppendPreloadSelect (clause.SelectList.PreloadColumns, a public field), but the appended value's concrete type (aliasedExpr, generated inside bob_counts.bob.go) was unexported: a caller that walks PreloadColumns has no way to recover the relationship name a given column belongs to, or recognize a count-preload column as such, without parsing rendered SQL text. Introduce orm.CountPreloadColumn (Name, Alias, Expr — Alias is always "__count_" + Name) as the typed, exported replacement for aliasedExpr. WriteSQL renders byte-identical output to the previous unexported type. Update the generator template to construct this type instead, and drop the template's own "io" import along with the aliasedExpr definition it was for (removing the last WriteSQL user of io.StringWriter directly in this file; the interface itself is still referenced via bob.Expression). No behavior change: the rendered SQL is identical, and every existing caller of the generated PreloadCount.* API is unaffected.
golangci-lint (installed as 'latest' in CI) moved from 2.12.2 to 2.13.1 and its bundled gofumpt now reformats the ExpressionFunc arguments in the three wm/qm.go files. No code change; formatting only.
Same formatting-only change as the previous commit, applied to every other file that gofumpt (as bundled in golangci-lint 2.13.1) reports. golangci-lint only surfaces three of them per run (max-same-issues), so this applies the full list from 'gofumpt -extra -l .' at once.
This was referenced Oct 11, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Context: the UNION ALL loading strategy — and your question about needed APIs
You asked, in the discussion around the UNION ALL loading work (#738),
whether that work would need any public APIs from bob. This PR is the
first concrete answer: one small, self-contained export.
Quick recap, since the code hasn't been shared yet: we are building a
third loading strategy alongside Preload (LEFT JOIN) and ThenLoad (one
query per relation) — a generated-code companion that fetches a parent
query and several of its relations in a single round-trip by
composing one UNION ALL statement, then stitches the rows back into the
generated models with ThenLoad-equivalent results. The full
implementation (runtime + bobgen-psql templates, PostgreSQL-only,
opt-in) will be proposed as a separate, larger PR; nothing in the
present PR depends on it, and this PR is useful on its own to any tool
that inspects a built query.
To support
PreloadCountunder that strategy, the tool must carry thecount column that bob's generated
CountPreloaderdeclared on theparent query through its own composed SELECT, scan it, and hand the
value to the generated model's existing
PreloadCount(name, count)method. That requires recognizing — on the query value itself — which
SELECT-list entries are count-preload declarations, and which
relationship each one belongs to. Exporting that information is the
single bob-core capability this PR adds.
Background
gen/templates/countsgenerates a per-relationCountPreloader(
PreloadCount.<Table>.<Rel>()) that, when applied to a query, appends acorrelated-subquery column to that query's own SELECT list via
bob.Query'sAppendPreloadSelect— landing inclause.SelectList.PreloadColumns, a public field. The element type ofthat slice, however, was
aliasedExpr, an unexported type definedinside the generated
bob_counts.bob.gofile itself, with unexportedexpr/aliasfields.This makes it impossible for a caller that walks
PreloadColumns(e.g.a tool composing its own SQL around a generated model, without going
through bob's own
scan.Mappermachinery) to recognize acount-preload column, or recover which relationship name
(
PreloadCount(name, count)in the generated model's own switch) itcorresponds to — short of parsing the column's rendered SQL text and
matching against its trailing
AS "<alias>", which is fragile andeasy to get subtly wrong (quoting, whitespace, args placeholders).
What changed
orm/countpreload.go:orm.CountPreloadColumn{Name, Alias, Expr}, an exported replacement for the generatedaliasedExpr.Aliasis always"__count_" + Name(unchanged from before).WriteSQLrenders byte-identical output to the previous type.gen/templates/counts/bob_counts.bob.go.tpl:applyCountnowconstructs
orm.CountPreloadColumninstead of the template-localaliasedExpr(which is removed along with itsWriteSQLmethod).The template's own
"io"import is dropped — it was only used byaliasedExpr.WriteSQL'sio.StringWriterparameter — since bob'sgenerator only runs the output through
gofumpt'sformat.Source(not goimports), so an unused import is not auto-removed and would
otherwise fail
go buildon the generated package.Why this scope, why not more
ormalready sits on thedialect/psql → ormside of the existingdependency direction (
psql.Preloader = orm.Preloader[*dialect.SelectQuery],dialect/psql/load.go), and no non-test file inorm/*.goimportsdialecttoday, so adding this type introduces no import cycle. Thegenerated template already imports
orm(used elsewhere in the samefile, e.g.
orm.Loadable), so this is a like-for-like type swap withzero new imports required by generated code.
This PR is scoped to exactly the type export needed to let an external
tool recognize a declared count-preload column from
PreloadColumnswithout parsing SQL. It does not change
PreloadCount's own runtimebehavior, its generated
switch-based dispatch, or any other part ofthe count-preload machinery.
Compatibility
CountPreloadColumn.WriteSQLproducesthe exact same
<expr> AS "<alias>"textaliasedExpr.WriteSQLdid.PreloadCount.*call sites — this onlychanges an unexported generated type into an exported one with the
same fields (renamed
expr→Expr,alias→Alias, plus a newNamefield callers can now read directly instead of parsing itback out of
Alias).go buildon the regenerated package to pick up the droppedioimport (this is caught by a build, not by the generator itself,
since bob's generator does not run goimports).
Testing
This PR itself does not add new bob-repo tests (the change is a
generated-template + supporting-type swap with no new logic). It was
developed and verified against the UNION ALL loading tool described
above, which reads
PreloadColumnsat runtime via the exact mechanismthis PR enables and carries the declared count columns through its own
composed SELECT. That codebase is not public yet (it will arrive as the
larger PR mentioned in the context section); until this export merges,
it runs on a temporary shim that renders each
PreloadColumnsentryand parses the trailing
AS "<alias>"back out — exactly the fragileworkaround this PR removes — and the shim will be swapped for a direct
type assertion against
orm.CountPreloadColumnonce this merges.Future work (not part of this PR)
The downstream consumer above accepts a query carrying a
bob.MapperModonly when the MapperMod count matches the count of
CountPreloadColumndeclarations it read off the same query's
PreloadColumns(plus, as afurther defense, the total
PreloadColumnscount). This is acount-based proxy, not a proof of provenance: current bob APIs give no
way to verify that a given
bob.MapperModactually originates from aspecific
CountPreloadColumndeclaration, so a caller who deliberatelyreplaces the MapperMod slice via
orm.Load.SetMapperMods()with anunrelated MapperMod — while the matching count happens to stay the
same — cannot be detected by counting alone (identified during a
downstream code review, 2026-08-20; recorded here as a residual risk
rather than a bug in this PR's scope).
A residual-risk-closing extension identified during that review: give
each MapperMod public metadata (kind, target relationship name, and an
opaque identifier shared with its originating
CountPreloadColumndeclaration) via a new field on
bob.Load(or equivalent). Thegenerated count preloader would register both the column declaration
and its MapperMod under the same identifier;
SetMapperMods()wouldinvalidate (mark "unknown provenance") any MapperMod it did not
originate; and a downstream consumer could then require every
MapperMod to be both count-kind AND identifier-matched before treating
it as accounted for — closing the same-count substitution, unrelated
addition, and reordering cases all at once. This is out of scope for
the present PR and is not implemented here.