fix(DataMapper): resolve the inverse BelongsTo of HasMany by type, not by member index - #528
Open
Yaraslaut wants to merge 12 commits into
Open
fix(DataMapper): resolve the inverse BelongsTo of HasMany by type, not by member index#528Yaraslaut wants to merge 12 commits into
Yaraslaut wants to merge 12 commits into
Conversation
github-actions
Bot
added
documentation
Codecov Report❌ Patch coverage is
📢 Thoughts on this report? Let us know! |
…t by member index HasMany<Other> located its back-referencing BelongsTo positionally: it reused the owner's HasMany member index to name a column in the *child* record. When the two relation members were not declared at the same index, the generated WHERE clause filtered on an unrelated column and the relationship silently returned wrong or empty results, with no error or diagnostic. Introduce InverseBelongsToIndexOf<OwnerRecord, ChildRecord> and its companion InverseBelongsToFieldNameOf<> in DataMapper/Record.hpp: a constexpr fold over the child's members that selects the unique BelongsTo whose ReferencedRecord is the owner. Missing or ambiguous inverses are now hard compile errors instead of silent wrong answers. The helper builds on FoldRecordMembers, so it works unchanged in both the reflection and the non-reflection build. CallOnHasMany, BuildHasManySelectQuery and LoadHasMany drop their FieldIndex template parameter and take the owner record type instead. HasManyThrough and HasOneThrough were already resolving their BelongsTo by type and are untouched. Add regression coverage with deliberately misaligned relation members, and drop the now-obsolete index-alignment note (and the artificial alignment it excused) from docs/sql-to-lightweight.md and its test-backed snippet source. Fixes #515 Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Two code-review findings, both in the new test helper. The recorder was held by `auto const`, but the logger callbacks mutate it through the non-const `SqlLogger&` it installs in its own constructor. Modifying a const object during its lifetime is undefined behaviour ([dcl.type.cv]/4) regardless of how benign the member happens to be. Neither ASan nor UBSan diagnoses it, so it would have sat there. The NOLINTNEXTLINE suppressed cppcoreguidelines-special-member-functions instead of answering it. AGENT.md is explicit that clang-tidy findings are fixed at the source and never silenced, and the honest answer here is that copying or moving the recorder would leave two objects believing they own the logger slot -- so all four are deleted. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
A record may legitimately hold more than one foreign key into the same table -
a meeting referencing the person table as both its organizer and its attendee,
or a join record whose two foreign keys both point at the table it joins to
itself. Lightweight could not express any of it, and in the through-relations it
silently produced wrong results.
Relationship selectors
----------------------
HasMany, HasManyThrough and HasOneThrough gain optional selector template
parameters naming the foreign key *column* they belong to:
HasMany<Meeting, SqlRealName { "organizer_id" }> organizedMeetings;
HasMany<Meeting, SqlRealName { "attendee_id" }> attendedMeetings;
HasManyThrough<Person, Friendship,
SqlRealName { "a_id" }, // this person
SqlRealName { "b_id" }> // the friend
friends;
The selector is a column name rather than a pointer-to-member because the two
records of a relationship reference each other: neither type is complete where
the other is declared, so &Meeting::organizer cannot be named from Human. The
BelongsTo side is unchanged - it already names its own column. Selectors default
to std::nullopt (AutoDetectRelation), so every existing record compiles and
resolves exactly as before.
One resolver for every relationship
-----------------------------------
InverseBelongsToIndexOf<Owner, Child, Selector> is now the single way any
relationship locates a foreign key, and it fails to compile unless it matches
exactly one BelongsTo. Naming a column that matches none is a hard error too, so
a typo cannot pass silently.
That closes three silent failures in the through-relations, which located their
foreign keys by enumerating *every* BelongsTo of a record with no type filter at
all, invoking their callback once per combination and keeping whatever the last
one produced:
- LoadHasOneThrough / LoadHasOneThroughByPK ran one query per (any FK in the
join record) x (any FK in the referenced record).
- CallOnHasManyThrough / CallOnHasManyThroughByPK ran one query per matching
FK. For a self-referential many-to-many the surviving query joined the join
record's second FK to itself and returned the record itself as its own
relation.
Two positional lookups are fixed with them: LoadHasOneThrough named a column of
ThroughRecord using *Record's* primary key index and qualified it with Record's
table, and CallOnHasManyThrough joined on Record's primary key index applied to
ReferencedRecord. Both agreed only while every record happened to declare an "id"
primary key at index 0 - which every record in the test suite did.
The four loaders now share BuildHasOneThroughSelectQuery and
BuildHasManyThroughSelectQuery. The HasOneThrough query drops its redundant join
back to the owning table (filtering the join record's foreign key is equivalent,
since the caller already holds that primary key) - which also removes an
unaliased duplicate-table join when the owning and referenced records are the
same table.
ddl2cpp
-------
Self-referencing foreign keys were dropped to plain fields, losing the relation:
Chinook's Employee.ReportsTo is the checked-in example. They now become a
BelongsTo, provided the primary key is declared before them - a pointer-to-member
may only name an already-declared member - and fall back to a plain field
otherwise. A self-reference emits no include of its own.
Tests
-----
- Human/Meeting with two foreign keys into the same table: counts, loading,
LoadRelations and the emitted SQL per relation.
- Self-referential many-to-many through a join record with two foreign keys into
the same table.
- A through-relation whose three records have differently named primary keys at
differing member indices - this fails against the previous loaders.
- ddl2cpp: self-reference becomes a BelongsTo; self-reference preceding its
primary key stays a plain field.
Databases tested: sqlite3, mssql2022 (Docker), postgres (Docker 16.4).
Also names three unused std::index_sequence parameters that a newer clang-tidy
rejects; unrelated to this change but it blocks the clang-debug build.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
The feature had a reference description but no example anyone could copy. Add a schema that carries both shapes at once - two direct roles on the meeting itself plus any number of attendees through a join table - and walk it end to end. Humans id, name Meetings id, topic, organizer_id, minute_taker_id Attendances id, meeting_id, human_id -- many attendees per meeting Only the two ambiguous relations carry a selector; the attendance many-to-many resolves on its own, which shows where a selector is needed and where it is not. The example creates three people, a meeting with three attendees and a second one with two, then reads both directions back: organizer through a mandatory BelongsTo, minute taker through a nullable one, attendees through HasManyThrough, and the same relations from the person's side. The documented output is the output the test actually prints. A closing section covers the self-referential case, where both foreign keys of the join record point at the same table and HasManyThrough needs both column names. All of it is snippet-backed per docs/sql-to-lightweight.md convention: the code lives in src/tests/DocExampleTests.cpp between //! [id] markers, is executed against every database, and scripts/check-doc-snippets.py fails on drift. Also aligns the HasMany Doxygen example with the documented schema and points at HasManyThrough for the many-attendees case. Databases tested: sqlite3, mssql2022 (Docker), postgres (Docker 16.4). Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Four defects surfaced by reviewing this branch: - DocExampleTests: the new Doc.Meetings test dereferenced optionals without narrowing, which bugprone-unchecked-optional-access rejects under the clang-debug preset (-Werror), breaking the build. Both the documented snippet and the assertion block now narrow with `if`, matching the pattern already used by the doc-relationships example. The doc snippet in sql-to-lightweight.md is updated in lockstep. - sql-to-lightweight.md claimed HasOneThrough takes the same selector pair as HasManyThrough. It does not: HasManyThrough names two columns on the join record, while HasOneThrough's second selector names a column on the referenced record. Transliterating the self-referential example was a compile error. - CxxModelPrinter emitted a BelongsTo for self-references that cannot compile: a foreign key targeting a UNIQUE NOT NULL non-primary-key column trips BelongsTo's static_assert, and the member pointer was built from the raw column name rather than the identifier the primary key member was actually emitted under (which SanitizeName or de-duplication may change). Both are now guarded, with regression tests. - LoadBelongsTo logged "Loading BelongsTo failed" for a NULL foreign key, which is an empty relation rather than a failure. It now returns early, avoiding both the warning and a pointless query. Databases tested: sqlite3, mssql2022 (Docker), postgres (Docker 16.4). The 4 postgres failures in SqlVariantDbTests/SqlBackup are pre-existing and untouched by this branch. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Doxygen 1.9.8, which CI runs, cannot resolve a \ref to a concept, so the "Check documentation coverage" job failed with four unresolved references. Newer doxygen (1.17 locally) resolves them, which is why this went unnoticed when the references were introduced. Plain autolinking degrades silently on old doxygen instead of erroring, and still produces a link on versions that index concepts. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Yaraslaut
force-pushed
the
fix/hasmany-inverse-by-type
branch
from
August 2, 2026 11:37
a509c31 to
16b403a
Compare
Adds RelationShapeTests.cpp, covering the *shape of the schema graph* -
self-reference, depth and nullability - where RelationTests.cpp already
covers how a relation finds its inverse. The shapes are the ones a mature
ORM suite treats as highest-risk (SQLAlchemy's test_cycles.py,
test_relationships.py, test_eager_relations.py), narrowed to what this
relation model can express: HasMany is a read-only view, so the cascade,
orphan-detection and backref-fixup families do not apply, and neither
composite foreign keys nor foreign keys onto a non-primary-key column are
representable.
Passing coverage:
- nullable foreign key: a NULL-parent row is attributed to no parent and
reads back as an empty optional rather than as id 0
- clearing a nullable foreign key removes the child from the collection
- a four-table chain A->B->C->D, with a sibling branch at every level so
a hop resolving against the wrong record pair shows up as a doubled
count
- LoadRelations() on a record with no BelongsTo, which is the boundary of
the second defect below
Two defects are recorded as [!shouldfail] tests rather than as prose, so a
fix has an executable definition of done:
1. A record whose BelongsTo names its own enclosing type overflows the
stack on construction. Reduced: sizeof() alone is fine (64 bytes), so it
is not the layout; the crash happens during static initialisation, before
a test body's first statement; and the same record with its BelongsTo
pointing at a *different* type is fine. Every pre-existing test uses the
cross-type form, so the self-referential one was never instantiated -
even though ddl2cpp emits exactly this shape and CxxModelPrinterTests
asserts it does. The shape's tests are written but compiled out: a stack
overflow in static init aborts the whole binary, which [!mayfail] cannot
contain.
2. LoadRelations() does not compile for any record holding a BelongsTo,
nullable or not: LoadBelongsTo() returns std::optional<ReferencedRecord>
and BelongsTo has no operator= accepting one. It instantiates only for
records that are all Fields plus HasMany-family relations. Latent because
all three pre-existing call sites pass such a record; the nullable
BelongsTo in that fixture sits on the child, which is never enumerated.
LoadHasMany() is private, so there is no other public entry point.
Full suite: 1292 passed, 1 skipped, 2 failed as expected (sqlite3, clang-cl).
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01TVec3YeQg2FZnToiPQwcQH
LoadRelations() was ill-formed for every record holding a BelongsTo:
field = LoadBelongsTo<FieldType>(field.Value());
// error: no viable overloaded '='
LoadBelongsTo() returns std::optional<ReferencedRecord>, while BelongsTo
declares operator= only for SqlNullType, for ReferencedRecord&, and for
another BelongsTo - an optional converts to none of them. So the call
instantiated only for records whose members are all plain Fields plus
HasMany-family relations; adding a single BelongsTo, nullable or not, broke
the build at the call site. LoadHasMany() is private, so affected records had
no public way to load their relations at all.
It stayed latent because all three pre-existing call sites pass a record with
no BelongsTo (MisalignedDepartment, Human, Suppliers). The nullable BelongsTo
in that fixture sits on the child record, which LoadRelations never
enumerates.
Adds BelongsTo::AdoptFetchedRecord() as the sink for an already-fetched
record and routes both LoadRelations() branches - reflection and
non-reflection - through it. Adopting differs from operator=(ReferencedRecord&)
on purpose:
- the foreign key is left untouched, because the value already came from the
row being loaded; operator= *establishes* a relationship and so rewrites it
- the field stays unmodified, since adopting a fetched value is not a pending
change to be written back on the next Update()
- an empty optional leaves the relation unloaded rather than clearing the
key, so a mandatory relation with a missing target row surfaces through the
accessor instead of silently becoming NULL
Both LoadRelations() branches are edited because they diverge and a guard added
to one is not present in the other.
Tests: replaces the [!shouldfail] placeholder with real coverage for the
non-nullable case (ChainB, which carries both a BelongsTo and a HasMany, so one
call exercises both enumeration branches) and the nullable case (OptionalChild,
covering both a set and an unset foreign key). Asserts the referenced record is
reachable and that the field comes back unmodified.
Full suite: 1294 passed, 1 skipped, 1 failed as expected (sqlite3, clang-cl).
The remaining expected failure is the unrelated self-referential-BelongsTo
defect, still open.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01TVec3YeQg2FZnToiPQwcQH
…defect
Replaces the speculative note on the self-referential BelongsTo defect with
what is actually measured, now that cdb was available to inspect the fault.
The earlier note guessed at "some member instantiated over the cycle has no
base case", implying runaway recursion. The debugger says otherwise:
- the fault is Stack overflow (c00000fd) on a stack only EIGHT frames deep,
one frame of which eats ~570 KB of the 1 MB stack - so nothing is
recursing at run time
- clang-cl reports a frame size of ~1.8e16 bytes (~2^54, an overflowed
computation) for any function that holds such an object
- the fault happens on function entry, before the first statement, which is
why no printf ever appeared regardless of placement
- sizeof() is a well-behaved 64 bytes, and a function that only mentions the
type compiles and runs fine; heap-allocating rather than stack-allocating
does not help, because it is the enclosing frame that is mis-sized
So this is a compile-time blow-up that the code generator turns into an
impossible stack allocation, not a runtime cycle.
Also records the hypotheses ruled out by isolated reproduction, so the next
attempt does not repeat them: BelongsTo's variadic converting constructor, the
ConfigureRelationAutoLoading -> LoadBelongsTo -> QuerySingle cycle,
std::function<optional<Record>()> held by value inside the record it returns,
and reflection-cpp's CountMembers probe against a constructor absorbing
AnyType. Each compiles and runs correctly alone.
Pinning the exact interaction needs a debug build with symbols; the release
build emits no PDB and D: had no free space for a debug configure.
No behaviour change - comments and the placeholder message only.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01TVec3YeQg2FZnToiPQwcQH
…piles it
The self-referential BelongsTo shape is not a Lightweight defect. Under MSVC cl
the tests pass - all of them, including the Each() traversal that was expected
to be the hardest case - and sizeof(TreeNode) is a well-behaved 120 bytes. What
breaks is clang-cl 22.1.3 code generation, so the block is now guarded on that
toolchain instead of being unconditionally compiled out.
Diagnosis, with cdb against a symbolised debug build (the earlier notes guessed
at runtime recursion; that was wrong):
- The fault is Stack overflow (c00000fd) in __chkstk+0x37, reached from
CATCH2_INTERNAL_TEST_0+0x10 - the stack probe in the function *prologue*,
before the body runs. Hence no output ever appeared wherever the printf went.
- The stack is eight frames deep, not thousands. Nothing recurses at run time.
- The prologue carries the frame size as a baked-in immediate with no
relocation: movabsq $0x404b56408001e0 / callq __chkstk / subq %rax, %rsp.
- Every other movabsq in that function shares the high half (0x404b5640_80..)
and differs only in the low bytes - 0x1e0, 0x158, 0x4a, 0x140, 0x148, 0x1e8 -
which are plausible frame offsets. The high half also changes between builds
(0x404b5640.., 0x3ef21fe0.., 0x3ef26540..). A small correct offset is being
OR-ed with garbage: a miscompile, not a computed size.
- sizeof() alone is fine, and heap-allocating rather than stack-allocating does
not help, because it is the enclosing frame that is mis-sized.
Ruled out as sufficient causes, each by an isolated reproduction that compiles
and runs correctly: BelongsTo's variadic converting constructor; the
ConfigureRelationAutoLoading -> LoadBelongsTo -> QuerySingle cycle;
std::function<optional<Record>()> held by value inside the record it returns; and
reflection-cpp's CountMembers probe against a constructor absorbing AnyType.
So the [!shouldfail] placeholder is now emitted only under clang-cl, where it
stands in for the skipped coverage; under MSVC the real tests run and there is no
expected failure at all. Next step is reducing this to a standalone case for an
upstream LLVM report - tracked in the comment, not in code.
Verified:
MSVC cl-release : 1298 passed, 1 skipped, 0 failed (self-ref tests ACTIVE)
clang-cl release : 1294 passed, 1 skipped, 1 failed as expected (guarded off)
Targeted run under MSVC: 92 assertions in 12 cases, all green.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01TVec3YeQg2FZnToiPQwcQH
The HasMany and HasManyThrough `Each()` auto-loaders bind one record instance and rebind it for each row without resetting it. A fetch does not necessarily overwrite the whole of a variable-width buffer, so a shorter value leaves the tail of the previous row in place and a string column comes back as a blend of two rows. Caught by the new self-referential HasMany coverage on this branch, where a tree of "childA"/"childB" read back as "childA"/"child". Only PostgreSQL surfaced it - every other backend passed - and the loop is byte-identical on master, so this is a pre-existing defect the new test exposed rather than a regression. Fixed in both loaders: assign a fresh record before rebinding, which clears every field's buffer and indicator. The HasManyThrough loader had the same bug with no test covering it. Also replaces the `REQUIRE(x.has_value()); x->member` pattern throughout RelationShapeTests.cpp with a ValueOf() helper that keeps the check and the dereference in one expression. clang-tidy cannot follow the check across statements and flagged 42 sites under bugprone-unchecked-optional-access; the helper fixes them at the source rather than suppressing each one with NOLINT. Verified against the failing configuration - PostgreSQL 16.0.4, MSVC: 1297 passed, 2 skipped, 0 failed (CI had 1 failed) Relation tests alone: 345 assertions in 29 cases No regression on SQLite: 1298 passed under MSVC, 1294 passed plus the one toolchain-guarded expected failure under clang-cl. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01TVec3YeQg2FZnToiPQwcQH
The previous commit routed the tests' optional accesses through ValueOf(), which cut bugprone-unchecked-optional-access from 42 findings to 6. The remaining ones were the helper itself, plus four `QuerySingle(...).value()` temporaries that pre-date this branch. Catch2's REQUIRE expands to a loop the analyser cannot prove exits, so it does not count as a guard - which is why `REQUIRE(v.has_value()); return *v;` was still flagged, and why switching to `.value()` did not help either. ValueOf now throws before dereferencing, so the access is unconditionally reachable only when a value is present. Catch2 reports an escaped exception as a test failure, so the diagnostic a caller sees is equivalent, and the safety is real rather than a NOLINT hiding the question. The four temporaries are routed through the same helper. Verified locally this time, with clang-tidy driven from the compile database rather than hand-assembled flags - the earlier attempt aborted on unrelated STL mismatches before reaching the file, which is why the previous push only fixed part of it: clang-tidy --checks='-*,bugprone-unchecked-optional-access' -> exit 0 PostgreSQL 16.0.4 under MSVC: 1297 passed, 2 skipped, 0 failed. Relation tests: 351 assertions in 29 cases. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01TVec3YeQg2FZnToiPQwcQH
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
1 participant
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.
Root cause
HasMany<Other>never searched for its inverseBelongsTo. It assumed the back-referencingBelongsToin the child record sits at the same member index as theHasManymember in theowner record:
ConfigureRelationAutoLoadingcaptured the owner's member indexFieldIndexand forwarded it toBuildHasManySelectQuery<FieldIndex, ReferencedRecord>().BuildHasManySelectQuerythen emitted.Where(FieldNameAt<FieldIndex, OtherRecord>, SqlWildcard)— taking the FK column name from the child record at the owner's index.
CallOnHasMany(and thereforeLoadHasMany/LoadRelations) had the same assumption.When the indices did not line up, the
WHEREclause filtered on an unrelated column. For thereporter's
Department/Employeepair (HasManyat index 2,BelongsToat index 5) the generatedSQL was
WHERE "lastName" = <department.id>anddepartment.employees.Count()returned 0 — noerror, no warning, no diagnostic. The forward direction (
BelongsTo) resolved correctly by type, soonly the reverse direction was broken for the very same pair of records.
Existing tests missed it because the only direct
HasManycoverage (User/Email) happens todeclare both relation members at index 2.
New resolution mechanism
src/Lightweight/DataMapper/Record.hppgains a reusable,constexprtype-based lookup that sitsnext to the other record-reflection helpers (
RecordPrimaryKeyIndex,RecordStorageFieldCount, …):It folds over
ChildRecord's members via the existingFoldRecordMembersand keeps the firstBelongsTowhoseReferencedRecordisOwnerRecord, plus a match count. Because it is layered onFoldRecordMembers/EnumerateRecordMembers, it works unchanged in both the default build andthe
LIGHTWEIGHT_CXX26_REFLECTIONbuild — there is no#ifin the new code, exactly asCallOnBelongsToandCallOnPrimaryKeyalready do it.CallOnHasMany,BuildHasManySelectQueryandLoadHasManydrop theirsize_t FieldIndextemplateparameter and take the owner record type instead; all four call sites were updated. This also fixes
the reflection build's
LoadHasMany<el>(...), which passed astd::meta::infointo asize_ttemplate parameter.
static_assertbehaviourInstantiating the lookup is a hard compile error in the two cases the
HasManycontract forbids.Verified against clang 21:
No inverse found
More than one inverse (ambiguous)
C++23 requires a string literal in
static_assert, so both record types are named by theinstantiation backtrace rather than interpolated into the message; the resolver is a class template
parameterised on exactly those two types so the backtrace always spells them out. The resolved index
is clamped on failure so these two asserts are the only diagnostics emitted — no cascade of
follow-up errors from
FieldNameAt.Mutually recursive records (the reproducer's shape:
Departmentrefers to a forward-declaredEmployee, which refers back) still compile: the lookup is only instantiated insideDataMappermember templates, by which point both record types are complete.
HasManyThrough/HasOneThroughNot affected — they already resolve by type.
CallOnHasManyThroughandCallOnHasManyThroughByPKwalkCallOnBelongsTo<ThroughRecord>and gate each candidate onstd::is_same_v<...::ReferencedRecord, Record>/...ReferencedRecord. No positional assumption,no change made.
One adjacent observation, deliberately left out of scope:
LoadHasOneThrough/LoadHasOneThroughByPKenumerateCallOnBelongsTo<ThroughRecord>andCallOnBelongsTo<ReferencedRecord>without the
is_same_vgate, so a through-record with severalBelongsTomembers runs the bodyonce per candidate and the last one wins. That is a different bug (under-constrained selection, not
a positional shortcut) and is better handled separately.
Documentation
docs/sql-to-lightweight.md+src/tests/DocExampleTests.cpp: removed the inline note claiming"a HasMany and its inverse BelongsTo are matched by field position" and the artificial member
ordering that existed only to work around this bug.
Employee::departmentnow sits at its naturalposition (index 5, the reproducer's layout) — so the documented schema is itself regression
coverage.
python3 scripts/check-doc-snippets.py→OK: 38 doc snippet(s) match their tested source regions.HasMany's doc comment now states that the inverse is matched by relationship type, that themembers may be declared at any index, and that zero or multiple inverses are compile errors.
InverseBelongsToIndexOf/InverseBelongsToFieldNameOfcarry Doxygen and are exported fromLightweight.cppm.Tests added
src/tests/DataMapper/RelationTests.cpp— newMisalignedDepartment/MisalignedEmployeepairusing the reporter's exact layout (
HasManyat index 2,BelongsToat index 5):FieldNameAt<2, MisalignedEmployee> == "lastName"(documents what the buggypositional lookup used to pick),
InverseBelongsToIndexOf<...> == 5,InverseBelongsToFieldNameOf<...> == "department_id", plus the same two assertions for theindex-aligned
User/Emailpair to prove no regression there.TEST_CASE_METHOD(SqlTestFixture, ...)with sections forCount()/IsEmpty(),All(),Each(), range-basedfor,LoadRelations, and aScopedSqlQueryRecorder(a scopedSqlLoggerthat captures prepared statements) asserting the emittedCOUNTquery contains"department_id"and does not contain"lastName".All connections come from the
--test-env-wired Catch2 fixture; no hard-coded connection strings.Databases tested
export ODBCSYSINI=$HOME/.local/share/lightweight-odbc, built withcmake --preset clang-debug(clang-tidy + ASan/UBSan +
-Werror, zero findings, noNOLINTadded)../out/build/clang-debug/src/tests/LightweightTest --test-env=sqlite3… --test-env=mssql2022… --test-env=postgresSkips are all pre-existing and unrelated to this change (
SqlWideString read from VARCHAR column,RowArrayCursor throws on a value truncated to the bound buffer, MSSQLSQLStatisticsindexverification).
One flake was observed on PostgreSQL during an early run:
SqlBackup: Complex Typesfailed withnull value in column "id" of relation "Person". It is a plainTEST_CASE(noSqlTestFixture,therefore no table cleanup) that backs up and restores the whole database, so it trips over
whatever table a preceding randomly-ordered test left behind. It passes standalone and passed on six
subsequent full runs; it involves no relations at all. Pre-existing and out of scope.
src/tests/test_dbtool.pywas not re-run: this change is confined toDataMapperrelationshiptemplates and does not touch
dbtool.Risk assessment
HasMany. TheWHEREcolumn is now derived from the child'sBelongsTo, not from a member index. For records that were accidentally aligned (likeUser/Email) the emitted SQL is byte-for-byte identical — covered by static assertions and bythe unchanged
HasManytest suite. For records that were misaligned the SQL changes from wrong tocorrect, which is the point of the fix.
HasManychild has noBelongsToback to the owner, or two of them, used to compile and silently produce a queryagainst an arbitrary column; it now fails to build. This is intentional — that code was already
returning wrong answers — but it is the one source-compatibility risk in this PR. The message
names the contract and the backtrace names both record types.
LoadHasMany,CallOnHasManyandBuildHasManySelectQueryareprivate members of
DataMapperand header-only templates; their signature change is invisibleoutside the library. Two new
constexprvariable templates are added to the public surface.into the existing query builder. Verified on all three engines.
#if, and the change removes a pre-existing bug inthe
LIGHTWEIGHT_CXX26_REFLECTIONpath ofLoadRelations(astd::meta::infowas being passedas a
size_ttemplate argument). That path is not built in this environment, so it is verified byinspection only.
Performance impact
None measurable. The lookup is a compile-time fold that replaces a compile-time index; the emitted
SQL has the same shape and the same number of round trips. There is a marginal extra template
instantiation per (owner, child) record pair, amortised alongside the reflection work already done
for those records.
Fixes #515
🤖 Generated with Claude Code