Skip to content

Export the count Preloader's SELECT-list column as orm.CountPreloadColumn - #763

Open
sandonemaki wants to merge 5 commits into
stephenafamo:mainfrom
sandonemaki:unionall-countpreload-column
Open

sandonemaki wants to merge 5 commits into
stephenafamo:mainfrom
sandonemaki:unionall-countpreload-column

Conversation

@sandonemaki

Copy link
Copy Markdown
Contributor

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 PreloadCount under that strategy, the tool must carry the
count column that bob's generated CountPreloader declared on the
parent 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/counts generates a per-relation CountPreloader
(PreloadCount.<Table>.<Rel>()) that, when applied to a query, appends a
correlated-subquery column to that query's own SELECT list via
bob.Query's AppendPreloadSelect — landing in
clause.SelectList.PreloadColumns, a public field. The element type of
that slice, however, was aliasedExpr, an unexported type defined
inside the generated bob_counts.bob.go file itself, with unexported
expr/alias fields.

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.Mapper machinery) to recognize a
count-preload column, or recover which relationship name
(PreloadCount(name, count) in the generated model's own switch) it
corresponds to — short of parsing the column's rendered SQL text and
matching against its trailing AS "<alias>", which is fragile and
easy to get subtly wrong (quoting, whitespace, args placeholders).

What changed

  • New file orm/countpreload.go: orm.CountPreloadColumn{Name, Alias, Expr}, an exported replacement for the generated aliasedExpr.
    Alias is always "__count_" + Name (unchanged from before).
    WriteSQL renders byte-identical output to the previous type.
  • gen/templates/counts/bob_counts.bob.go.tpl: applyCount now
    constructs orm.CountPreloadColumn instead of the template-local
    aliasedExpr (which is removed along with its WriteSQL method).
    The template's own "io" import is dropped — it was only used by
    aliasedExpr.WriteSQL's io.StringWriter parameter — since bob's
    generator only runs the output through gofumpt's format.Source
    (not goimports), so an unused import is not auto-removed and would
    otherwise fail go build on the generated package.

Why this scope, why not more

orm already sits on the dialect/psql → orm side of the existing
dependency direction (psql.Preloader = orm.Preloader[*dialect.SelectQuery],
dialect/psql/load.go), and no non-test file in orm/*.go imports
dialect today, so adding this type introduces no import cycle. The
generated template already imports orm (used elsewhere in the same
file, e.g. orm.Loadable), so this is a like-for-like type swap with
zero 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 PreloadColumns
without parsing SQL. It does not change PreloadCount's own runtime
behavior, its generated switch-based dispatch, or any other part of
the count-preload machinery.

Compatibility

  • Byte-identical generated SQL: CountPreloadColumn.WriteSQL produces
    the exact same <expr> AS "<alias>" text aliasedExpr.WriteSQL did.
  • No public API changes to PreloadCount.* call sites — this only
    changes an unexported generated type into an exported one with the
    same fields (renamed expr→Expr, alias→Alias, plus a new
    Name field callers can now read directly instead of parsing it
    back out of Alias).
  • Regenerating any existing project against this template requires
    go build on the regenerated package to pick up the dropped io
    import (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 PreloadColumns at runtime via the exact mechanism
this 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 PreloadColumns entry
and parses the trailing AS "<alias>" back out — exactly the fragile
workaround this PR removes — and the shim will be swapped for a direct
type assertion against orm.CountPreloadColumn once this merges.

Future work (not part of this PR)

The downstream consumer above accepts a query carrying a bob.MapperMod
only when the MapperMod count matches the count of CountPreloadColumn
declarations it read off the same query's PreloadColumns (plus, as a
further defense, the total PreloadColumns count). This is a
count-based proxy, not a proof of provenance: current bob APIs give no
way to verify that a given bob.MapperMod actually originates from a
specific CountPreloadColumn declaration, so a caller who deliberately
replaces the MapperMod slice via orm.Load.SetMapperMods() with an
unrelated 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 CountPreloadColumn
declaration) via a new field on bob.Load (or equivalent). The
generated count preloader would register both the column declaration
and its MapperMod under the same identifier; SetMapperMods() would
invalidate (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.

…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.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant