feat(orm)!: preload child structs with a generated NULL-safe typed mapper - #725
Merged
Merged
Conversation
Generated <Parent>Slice.Load<Rel> loaders scanned related rows with scan.StructMapper (per-row reflection). Generate a per-table reflection-light <table>ScanMapper and use it instead, scheduling each result column directly into the struct field by index. .All() merely delegates to bob.Allx with the view scanner, so hooks, nested Preload/ThenLoad and behaviour are unchanged.
…pper Generate a per-table NULL-tolerant typed mapper (<table>ScanMapperNullable) for every table that is the target of a to-one relationship, and pass it to orm.Preload so the joined child columns are scanned without reflection while preserving the LEFT JOIN semantics of the previous scan.StructMapper path: an all-NULL row still yields no child object, and NULL values scanned into non-nullable fields still leave the zero value. Builds on stephenafamo#715 and benefits from stephenafamo/scan#8. BREAKING CHANGE: orm.Preload (and the psql/mysql/sqlite Preload wrappers) gain a new PreloadMapper[T] parameter. Only hand-written callers of orm.Preload are affected; generated code is regenerated as part of this change. Pass nil to keep the previous reflection-based behaviour.
3 tasks
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.
Summary
This is the follow-up left "out of scope" in #715:
Preloaded child columns were still scanned bythe prefix-aware, reflection-based
scan.StructMapperinside the preload mapper mod. This PRgenerates a second, NULL-tolerant typed mapper per table (
<table>ScanMapperNullable) and passesit to
orm.Preload, so the joined child columns are also scanned without reflection.Breaking change
orm.Preload(and thepsql/mysql/sqlitePreloadwrappers) gain a parameter to accept atyped, NULL-safe mapper for the child struct. This lets the JOIN-based preload path skip the
reflection-based
scan.StructMapper(+NullTypeConverter+ row validator) while preserving itsexact LEFT JOIN semantics: rows where every prefixed column is NULL still produce no child object,
and NULLs scanned into non-nullable fields still leave the zero value instead of erroring.
Who is affected: only hand-written callers of
orm.Preload/<dialect>.Preload. Generatedcode (the vast majority of users) is regenerated as part of this change and requires no manual
updates.
Migration: pass
nilfor the new parameter to keep the current reflection-based behaviorunchanged. Passing a generated
<table>ScanMapperNullableopts into the typed path.I considered an alternative, fully backward-compatible design (injecting the mapper via a new
PreloadOptioninstead of a signature change), but went with the explicit parameter for a simplerbuildPreloaderimplementation, and for symmetry with the constructor change in #715. Happy toswitch to the option-based approach if you'd prefer to avoid the breaking change — let me know and
I'll rework this PR.
Why the slice-loader mapper from #715 can't be reused as-is
The preload path needs three behaviours the fast mapper doesn't have (all currently provided at
runtime by
StructMapperoptions inbuildPreloader):<runtime alias>.<column>(the alias isgenerated per query, e.g.
users_5.id).struct fields can't hold NULL.
What is generated
For every table that is the target of a to-one relationship (the only tables
Preloadcan load —this guard also keeps the
unusedlinter happy):nulltypes the models already use — no new wrapper types):<table>ScanMapperNullable(prefix string) scan.Mapper[*Table]that resolvescolumn names once per query (prefix check + name
switchin the(ctx, cols)closure); theper-row path is one buffer allocation plus
ScheduleScanByIndexcalls. Theafterfunc returnsnil, nilwhen every buffer field is invalid — the same contract as the current row validator(zero value, no error) — and otherwise copies the buffer into the struct, guarding non-nullable
fields with a validity check so a NULL leaves the zero value exactly like the reflection mapper.
Because unselected columns are never scheduled and stay invalid, the static validity check over
all generated fields is equivalent to the runtime check over the selected subset, so
PreloadOnly/PreloadExceptbehave identically.Nested preloads compose unchanged:
buildPreloaderwraps the typed mapper withscan.Mod(mapper(prefix), mapperMods...)instead ofWithMapperMods(semantically equivalent).Behaviour preservation
ormtest (TestPreloadMapperParity) runs the same mock rows through the reflectionmapper and a typed mapper and asserts identical results for: matched child, all-NULL child
(LEFT JOIN miss → nil), partial NULL, and a selected-column subset.
children, NULL values in nullable child columns, nested preload (posts → users → countries,
where the middle table has a NULL FK), and
PreloadOnly— results identical to the currentbehaviour, including runtime-generated alias prefixes.
(e.g.
PreloadOnlyover nullable columns only) is dropped — same as today, since the currentrow validator also only sees the selected columns.
Performance
Preload scan-path micro-benchmark (
BenchmarkPreloadMapper, committed), mocksql.Rows→scan.AllFromRows, parent (2 cols) + preloaded child (2 cols),-benchmem -count=3(medians).Numbers include the parent's scan cost, so the child-mapper delta itself is larger:
Testing
go test ./orm/... ./gen ./dialect/psql/... ./dialect/mysql/... ./dialect/sqlite/...: all pass.go test ./gen/bobgen-sqlite/driver(generate → build → run generated test suites): modernc,ncruces and libsql pass. (mattn fails in my environment because CGO is unavailable — unrelated
to this change.)
go test ./gen/bobgen-psql/driver(testcontainers): pass.golangci-lint runon the touched packages: 0 issues;gofumpt -l: clean.Out of scope
ThenLoadrelations (separate query, never sees join NULLs — already covered by perf(gen): generate typed scan mappers and inject them via the model constructors #715).o.LoadRel()) — also already covered by perf(gen): generate typed scan mappers and inject them via the model constructors #715 via the base scanner.