Skip to content

Relation auto-loading abandons the caller's DataMapper and shares one thread-local SqlStatement #619

Description

@Yaraslaut

Correction (please read first). This issue originally reported a spurious
Invalid argument count and attributed it to the shared _stmt described below. That
attribution was wrong. Tracing the emitted SQL showed the real cause to be an unrelated
defect — copying a query builder leaves inputBindings pointing at the source's
_boundInputs — now filed separately as #620. Nothing in this issue throws that
exception.

What remains here is still a real problem, just a different one: lazy relation loaders
abandon the caller's DataMapper and run on a thread-local one's shared SqlStatement.

The original bisect against #576 is left at the bottom because it documents a behaviour change
that is worth knowing about, not because it causes a crash.

Problem

Every lazy relation loader installed by ConfigureRelationAutoLoading resolves through
DataMapper::AcquireThreadLocal() rather than the mapper that produced the record, and runs its
query on that mapper's single shared _stmt:

field.SetAutoLoader(typename FieldType::Loader {
    .loadReference = [value = field.Value()]() -> std::optional<typename FieldType::ReferencedRecord> {
        DataMapper& dm = DataMapper::AcquireThreadLocal();
        return dm.LoadBelongsTo<FieldType>(value);
    },
});

Two consequences follow, and neither is visible at the call site:

1. The connection you chose is silently discarded. Take a mapper from a pool, query with it,
then touch a relation on a returned record: the follow-up query does not run on that mapper. It
runs on a thread-local mapper built from the default connection string. Callers that carefully
give each concurrent task its own pooled connection still end up funnelling all relation traffic
onto one shared statement per thread — which SqlPreparedStatementCache.hpp states is
unsupported:

Not thread-safe, mirroring SqlConnection: one connection is used by one thread at a time.

We honour that contract for our own mappers. What we cannot honour, because nothing at the call
site suggests it, is that dereferencing a relation reroutes the query elsewhere.

2. It is an N+1 by construction. RunRelationPreloaders is a no-op unless the query asked
for .With<>() (or eagerLoadDepth > 0), so with plain loadRelations = true every row issues
its own SELECT on that shared statement.

This overlaps #584, which documents the same thread-local bypass and its consequences for
multi-database setups, transactions, pool accounting and per-connection tuning. This issue adds
the shared-_stmt angle: it is not only the connection that is wrong, but a single statement
handle being reused across logically independent queries.

Shape

// Pooled mapper: one connection per task, several tasks in parallel.
auto query = dm.Query<RecordA, DataMapperOptions { .loadRelations = true }>();
std::ignore = query.WhereIn(
    SqlQualifiedTableColumnName { .tableName = RecordA::TableName, .columnName = "ColumnY" }, keys);

for (auto const& row: query.All())
    if (auto const related = row.someRelation.Record())   // -> AcquireThreadLocal()._stmt
        use(related->get().someField);

Suggested direction

  1. Resolve a relation on the mapper that loaded the record, so the caller's connection choice is
    honoured. The loaders deliberately capture values rather than the mapper because a record
    may outlive the mapper that produced it (see the NRVO note in
    ConfigureRelationAutoLoading), so this needs an owning handle or a pool reference rather
    than a raw DataMapper& — see Relation auto-loading bypasses the connection pool: BelongsTo & friends resolve through a thread-local DataMapper #584, which discusses that constraint.
  2. Failing that, give each lazy load its own SqlStatement on the chosen connection, as All()
    already does, rather than sharing the mapper's one.
  3. At minimum, document on loadRelations / Record() that touching a relation reroutes onto a
    thread-local mapper, so callers using a pool can reason about which connection and statement
    are actually in play.

Workaround

Resolve relations eagerly on the caller's own mapper — .With<&Entity::relation>(), or an
explicit batch query — and keep loadRelations = false. RunRelationPreloaders invokes the
preloader with the caller's mapper and PreloadBelongsTo deduplicates the keys into one query,
so nothing escapes to the thread-local mapper and the N+1 disappears.

Appendix: the SQLNumParams() skip added by #576

Recorded because it is a real behaviour change between releases, not because it causes the
above. We moved v0.20260625.0 -> v0.20260921.0. In the old pin, SqlStatement.cpp:320 read:

RequireSuccess(SQLNumParams(m_hStmt, &m_expectedParameterCount));

unconditionally on every Prepare() — no reuse check, and no m_preparedParameterCount field.
#576 added a reuse fast path that skips SQLNumParams() and restores the count from
m_preparedParameterCount:

bool const reusePreparedQuery = !m_preparedQuery.empty() && m_preparedQuery == query;
...
bool const skipReprepare = acquiredFromCache || reusePreparedQuery;
...
if (!skipReprepare)
    // ... SQLNumParams()
// The own-handle fast path skipped SQLNumParams(), so restore what it would have reported.

Worth noting: reusePreparedQuery is plain text equality against the statement's own last
prepared query, so this path is taken with the prepared-statement cache switched off
(PreparedStatementCacheCapacityDefault is 0). The restored count is therefore load-bearing for
correctness on a statement whose prepared query changes between uses, not merely a cache
optimisation.

Environment

  • Lightweight v0.20260921.0 (vcpkg)
  • MS SQL Server via ODBC Driver 18, Windows, MSVC
  • Prepared-statement cache disabled (default capacity 0)
  • Concurrent reads over a mapper pool, with relation auto-loading on one of the queries

Activity

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions