Skip to content

EF Core: a value set column reads and writes wrapper elements through the element's Parse, so a rule tightened after the rows were written makes them unreadable #488

Open

Description

@CaffeinatedCoder

Problem

A set over a validated wrapper type (StringSet<TElement>, and the numeric and temporal wrapper arities) keeps each element's invariant text form in an array column. Every trip through the column runs the element's own Parse, and a validated element's Parse applies its rules.

That works until a rule is tightened. Take StringSet<PermissionKey> in a text[] column, where PermissionKey.Parse applies Create (a CodoMetis.TypeKit value object). Forbid a character in Create, and every row holding an older key stops loading. One key makes the whole row unreadable. Reading back what the application itself wrote shouldn't depend on today's rules.

CodoMetis.TypeKit answers this for a single-valued column: IValueObjectMaterializer<TSelf, T>.Materialize(T value) rebuilds the value object without validation, and exists for persistence only. A set column has no equivalent, so the array is the one stored read that still applies the rules.

Where the element's Parse runs today

Read on release/10.x at cbdeca8, for the string-backed path. The file is unchanged since v10.3.0. I did not read main. Three places run the element's Parse on a stored value:

  1. The read. StringBridgedSetTypeDefinition.FromPrimitive calls TSet.ParseValue, which calls ParseElement, which calls TElement.Parse.
  2. The read's round-trip check. WrapperElement.Parsed formats the element it just built and parses that text again with the same parser. A read that skipped the first Parse would still hit this one and throw FormatException.
  3. The write, and a bound element. ToPrimitive proves the text reads back by calling FromPrimitive (Wrapper set arities don't check that an element's text form round-trips #53), and both the set mapping and the element mapping are built from it. So saving the column with an older key in it throws InvalidOperationException, and so does binding one as a query parameter: Contains binds its element through ElementTypeMapping. That includes the query for the rows that still hold a key a rule just made invalid.

Replacing the first call is therefore not enough. All three have to agree on how a stored value is rebuilt. The check in 2 and 3 guards against text that reads back as a different element (#53). It can stay if it uses the same unvalidated rebuild, because it then tests the path the read really takes.

The primitive-backed wrapper arities (Int32Set<TElement> and its siblings) have the same FromPrimitive shape in BridgedSetTypeDefinition and share WrapperElement. I read those, not each arity's ParseElement.

Change

Not decided. Read a wrapper element of a set column through an unvalidated rebuild when the element type offers one. Keep Parse for everything that is input: the array literal, Parse and TryParse on a set, and JSON. A JSON converter can't tell a stored column from a request body, and TypeKit says never to call Materialize on input.

How ValueRanges learns about the rebuild is the open question:

  • Depend on CodoMetis.TypeKit in the EF Core package, and use IValueObjectMaterializer<TSelf, T> when TElement implements it. It adds no public surface here, but it adds a second package to keep in step, and TypeKit elements behave differently from plain wrappers.
  • A seam of ValueRanges' own: a small static abstract interface an element type can implement, which TypeKit's generator or the consumer implements. It needs no dependency, but it is a new public interface for the 10.x line.
  • A registration per element type. It works without either, but every consumer registers every element type, which TypeKit's own EF Core satellite avoids.
  • Document it and stop. Reads re-run Parse, so a wrapper whose rules may tighten has to keep a lenient Parse.

Other open questions:

  • A static abstract call needs the element type constrained to the interface. StringBridgedSetTypeDefinition is constrained to IParsable<TElement> only, so the unvalidated rebuild needs a second definition class, picked when the definition is registered. Check that choice against the AOT claims (Hold the two AOT claims (A02, A03) with a reflection-disabled test project #107).
  • Does a jsonb column count as a stored read? Today it shares ParseValue with request bodies.

Does it change an answer?

A row the element's Parse refuses throws today, and would load. No value that loads today changes. It is a loud failure turning into a value, not a wrong answer. But it applies to every element type that offers the rebuild, without the consumer doing anything, so decide here whether it belongs under ### Changed answers.

Done when

  • the option is decided here
  • a live test on PostgreSQL 16 and 18 stores a set, tightens the element's rule (a probe element type with a switchable rule), and shows that the row loads, that saving it works, and that Contains(storedElement) binds, while the array literal and JSON still refuse the same element
  • a seed that restores TElement.Parse in each of the three places is caught by its own test
  • docs/value-sets.md and docs/efcore.md say which reads validate and which don't
  • the changelog rows are in the root and the affected package changelogs
  • it lands on main first, then is cherry-picked with -x into release/10.x if it ships there
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

    area/value-setsValue set families and wrapper aritiesenhancementNew feature or requestpackage/coreAffects CodoMetis.ValueRangespackage/efcoreAffects CodoMetis.ValueRanges.EFCore.PostgreSQL

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions