Skip to content

FlatTableProjection: honor enum storage + value types + nullable (#4290, #4291)#4292

Merged
jeremydmiller merged 1 commit intomasterfrom
fix-4290-4291-flat-table-enum-nullable
Apr 26, 2026
Merged

FlatTableProjection: honor enum storage + value types + nullable (#4290, #4291)#4292
jeremydmiller merged 1 commit intomasterfrom
fix-4290-4291-flat-table-enum-nullable

Conversation

@jeremydmiller
Copy link
Copy Markdown
Member

Summary

FlatTableProjection.Map / Increment / Decrement built ParameterSetters eagerly via PostgresqlProvider.ToParameterType, which left three bug paths:

This PR defers ParameterSetter construction from registration time to Compile, where EventGraph (and therefore StoreOptions) is in scope. At Compile the leaf member's storage is decided as:

  • enumVarchar + .ToString() or Integer + Convert.ToInt32, chosen by DuplicatedFieldEnumStorage. Nullable enums fall through ParameterSetter's null path and write DBNull.
  • registered value type → inner SimpleType's NpgsqlDbType + ValueProperty.GetValue extraction. Nullable value types share the null path.
  • everything else → existing PostgresqlProvider lookup, no transform.

ParameterSetter<TSource,TValue> gains a (member, dbType, transform) constructor so the leaf can bind to a DB type the wrapper type wouldn't infer; the existing 1-arg constructor stays untouched. BuildPrimaryKeySetter<T> / BuildSetterForMembers<T> now require StoreOptions; EventDeleter and StatementMap defer their setter builds to Compile to match.

Documentation: docs/events/projections/flat.md gets a new Enums, Nullable Values, and Registered Value Types section that calls out the DuplicatedFieldEnumStorage dependency and the column-type expectation.

Fixes #4290.
Fixes #4291.

Test plan

  • New regression file Bug_4290_4291_flat_table_enum_and_value_types.cs (7 facts) — all fail on master, all pass with this fix:
    • enum stored as text under EnumStorage.AsString
    • enum stored as int under EnumStorage.AsInteger
    • nullable enum with value present
    • nullable enum with null (writes DBNull)
    • registered value type auto-unwrapped to inner primitive
    • nullable registered value type with value present
    • nullable registered value type with null (writes DBNull)
  • All 40 existing Flattened / FlatTable tests still pass
  • All 164 Projections event-sourcing tests still pass

🤖 Generated with Claude Code

FlatTableProjection.Map / Increment / Decrement built ParameterSetters
eagerly off PostgresqlProvider.ToParameterType, so:

- enum columns ignored Advanced.DuplicatedFieldEnumStorage and always
  bound the parameter as Integer, breaking string-typed enum columns
  (#4291)
- registered value-type properties (e.g. Vogen wrappers) and nullable
  variants threw "Can't infer NpgsqlDbType for type <Wrapper>" (#4290)
- nullable enums were never special-cased

Defer setter construction from registration time to Compile (which has
EventGraph and therefore StoreOptions in scope). At Compile, the leaf
member's storage is decided based on:

  - enum -> Varchar + .ToString() OR Integer + Convert.ToInt32, picked
    from DuplicatedFieldEnumStorage; Nullable<TEnum> works because
    ParameterSetter now writes DBNull on null and only invokes the
    transform on a non-null value
  - registered value type -> inner SimpleType's NpgsqlDbType + ValueProperty
    extraction; Nullable<TWrapper> handled the same way as nullable enum
  - everything else -> existing PostgresqlProvider lookup, no transform

ParameterSetter<TSource,TValue> gets an additional ctor that takes
(member, dbType, transform) so the leaf can bind to a db type the
.NET wrapper type wouldn't infer.

EventDeleter and StatementMap were updated to defer setter construction
through the new BuildPrimaryKeySetter<T>(members, storeOptions) /
BuildSetterForMembers<T>(members, storeOptions) signatures. The static
methods are still internal but now require StoreOptions.

Documentation in docs/events/projections/flat.md gets a new
"Enums, Nullable Values, and Registered Value Types" section that calls
out the DuplicatedFieldEnumStorage dependency and the matching column-type
expectation.

Fixes #4290.
Fixes #4291.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
@jeremydmiller jeremydmiller merged commit 63da002 into master Apr 26, 2026
6 of 9 checks passed
@jeremydmiller jeremydmiller deleted the fix-4290-4291-flat-table-enum-nullable branch April 26, 2026 16:50
jeremydmiller added a commit that referenced this pull request Apr 28, 2026
Three independent root causes feed the test-flake pattern that's been
hitting recent PRs (#4279, #4281, #4292, #4295, #4296, #4302). All three
are localized; this PR addresses each.

## Root cause 1: shared static random sequence

Target.cs defines `private static readonly Random _random = new Random(67)`
that is consumed by ~80 tests across LinqTests + DocumentDbTests. Each
Target.Random() / GenerateRandomData() call advances the shared sequence,
so a test's effective random data depends on which sibling tests ran
before it. xUnit discovery order is mostly stable but NOT guaranteed
identical run-to-run, especially across CI workers with different load,
.NET TFM combinations, etc. A small order shift consumes a different
slice of the sequence and produces different test data — silently flipping
assertions that depend on exact counts or distributions.

Fix: introduce `Target.ResetRandomSeed(int seed = 67)` so a test that
genuinely depends on specific random data can pin the sequence at the
start. Remove the readonly modifier on _random to allow rebinding.

Updated tests to call ResetRandomSeed():
- Bug_605_unary_expressions_in_where_clause_of_compiled_query (3 facts)
- Bug_3337_select_page.try_it_out
- query_against_child_collections.buildUpTargetData (covers
  can_query_on_enum_properties and many more)

Also tightened Bug_605's assertion: it was hardcoded to `.ShouldBe(15)`
but the real point of the test is "compiled query == inline LINQ query for
the same expression"; the page size of 15 is incidental. Compare against
expected.Count instead so the test is robust to data variance.

## Root cause 2: DateTimeOffset.UtcNow inside a shared LINQ expression

`child_collection_queries.cs:67` was registering this where-clause for the
acceptance suite:

    @where(x => x.Children.Any(c => c.NullableDateOffset <= DateTimeOffset.UtcNow));

That expression runs in BOTH the in-memory LINQ-to-objects "expected"
provider AND the LINQ-to-SQL "actual" provider. Each provider evaluates
DateTimeOffset.UtcNow at its own moment. Target.NullableDateOffset values
are ±60 seconds of "now" from random data; values within microseconds of
either provider's "now" can land on opposite sides of <= and disagree.

Fix: capture a fixed `asOf = DateTimeOffset.UtcNow.AddDays(1)` in the
static ctor and use that as the boundary. The expression now embeds a
constant timestamp that both providers see identically. AddDays(1) puts
it well beyond the test data range so the predicate is meaningfully true
for matching rows.

## Root cause 3: ordering assumptions on server-generated Guids

Bug_4282 asserted `ids.ShouldHaveTheSameElementsAs(doc1.Id, doc3.Id)`
after `OrderBy(x => x.Id)`. The IDs are server-generated Guids; their
sort order does not in general match declaration order (Marten uses
sequential Guids in many configs but not always, depending on the
StoreOptions in scope and the underlying provider). Switched to
set-membership: `Count == 2` plus ShouldContain for each expected id.

## Root cause 4 (defensive): ShouldBeEqualWithDbPrecision tolerance

The helper used to round both sides to 100µs with truncation (`Ticks /
1000 * 1000`) and then ShouldBe. The math works in the common case, but
the assertion was fragile under loaded-runner clock-comparison edge
cases. Switched to a 1ms tolerance check; widely above the worst-case
PostgreSQL truncation (9 ticks ≈ 0.9µs) but still tight enough to catch
real semantic differences. Also produces a clearer failure message when
it does fire.

## Verification

Stress-ran the previously-flaky suites locally: 5x consecutive runs of
all 178 LinqTests.Bugs tests, no failures. All 123 tests across Bug_605,
Bug_4282, Bug_3337, query_against_child_collections, and
child_collection_queries pass. Bug_2283 in DocumentDbTests passes.

Closes #4310.

Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
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.

FlatTableProjection mapping enums as strings throws FlatTableProjection throws when mapping value object or nullable value object

1 participant