CLR enum underlying type mapping#6581
Open
NinoFloris wants to merge 8 commits into
Open
Conversation
There was a problem hiding this comment.
Pull request overview
This PR introduces default CLR enum support by mapping enums to PostgreSQL integer types based on the enum’s underlying type (including array support), while also refactoring/moving PostgreSQL enum (label) mapping tests into a dedicated suite.
Changes:
- Add enum-underlying type mapping via a new
EnumUnderlyingConverter<T>and resolver support inAdoTypeInfoResolverFactory. - Extend converter dispatch and array conversion to support enum↔underlying “reduced-type equivalence” with trimming/NativeAOT considerations.
- Restructure and expand tests: move PG-enum label mapping tests into
PgEnumTestsand add enum-underlying tests inEnumTests.
Reviewed changes
Copilot reviewed 13 out of 13 changed files in this pull request and generated 5 comments.
Show a summary per file
| File | Description |
|---|---|
| test/Npgsql.Tests/Types/PgEnumTests.cs | New test suite covering PostgreSQL enum (label) mapping/unmapping and unmapped PG enum behavior. |
| test/Npgsql.Tests/Types/EnumTests.cs | Replaced prior PG-enum tests with tests for enum-underlying integer mappings. |
| test/Npgsql.Tests/GlobalTypeMapperTests.cs | Updated expectations around enum mapping behavior (DbType/type inference assertions). |
| test/Npgsql.Tests/BugTests.cs | Adjusted composite test to avoid accidental enum fallback mapping by changing a PG field type. |
| src/Npgsql/TypeMapping/UserTypeMapper.cs | Switched mapped PG enum converter from EnumConverter<> to PgEnumConverter<>. |
| src/Npgsql/Internal/ResolverFactories/UnmappedTypeInfoResolverFactory.cs | Renamed resolver classes and updated converter activation to PgEnumConverter<>. |
| src/Npgsql/Internal/ResolverFactories/AdoTypeInfoResolverFactory.cs | Added CLR enum underlying-type typeinfo resolution and enum-array support. |
| src/Npgsql/Internal/PgTypeInfo.cs | Allowed enum requested types over underlying converters and adjusted SupportsRead/Write defaults for this case. |
| src/Npgsql/Internal/PgConverter.cs | Added enum-underlying dispatch paths for Read/Write/Bind/IsDbNull gated by RuntimeFeature.IsDynamicCodeSupported. |
| src/Npgsql/Internal/PgBufferedConverter.cs | Unsealed ReadAsObject/WriteAsObject overrides to allow specialized behavior in derived converters. |
| src/Npgsql/Internal/Converters/PgEnumConverter.cs | Renamed enum label converter type from EnumConverter<> to PgEnumConverter<>. |
| src/Npgsql/Internal/Converters/EnumUnderlyingConverter.cs | New converter intended to read/write enums via their underlying integer PG wire formats. |
| src/Npgsql/Internal/Converters/ArrayConverter.cs | Added “effective array type” creation support to enable enum-array materialization over underlying element converters. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
vonzshik
reviewed
May 19, 2026
Contributor
vonzshik
left a comment
There was a problem hiding this comment.
Looks good, just one moment
Arrays can't immediately be supported
1f3121c to
5ba7640
Compare
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.
Solves half of #3303 and fixes #5591.
This brings NativeAOT compatible *unmapped* CLR enum support for reads and writes. Once merged we begin to map (by default, inferred) to the closest pg type based on the enum's underlying type:
We don't support any primitive conversions like we have for raw primitives. We can leave those until people actually ask for them. This means if users want to map an int enum into an int8 (and so on) that we do not yet support this.
Arrays are supported but not fully, see: dotnet/runtime#127249. We could consider adding these mappings on CoreCLR only. So far I've kept the functionality symmetrical, as we try to do with most mappings.
A lot of the recent work was required to land this feature:
This PR just really threads the needle through CLR and NativeAOT quirks. The enum <-> underlying "reduced-type equivalence" is one big, weird, exception in the type system: array covariance carries it through, though generic instantiation stays invariant. Given our infra is generic it requires full manual mapping to faithfully resurface the relationship, rather than simply relying on the (allocating) boxing/unboxing helpers to do it for us.
Once past the IL stage NativeAOT brings its own problems. Its aggressive trimming does not carve out any real exceptions for these historical equivalences beyond the absolute simplest cases (nevertheless working with enums through their underlying type is the recommended way). Then just as I was rounding off the work I noticed a classic bloat issue, one that has surfaced a few times before in our NativeAOT journey. In short, there are fundamental limitations in the current IL scanner, and we are still waiting for a more capable one (dotnet/runtime#83021).
More concretely: we cannot make unmapped enum reading and writing *allocation free* on NativeAOT without incurring massive code bloat. The essential issue is that the scanner cannot clean up code blocks that were found dead or under vacuous conditionals after scanning completed. This applies to typeof(T).IsX branches, isinst branches over types that didn't get instantiated, and so forth. All of that knowledge requires the whole program view the scanner itself is building (and potentially the generic specialization view that codegen has) which all happens after it has decided the methods and types must be kept.
If we do not fortify our typeof(T).IsEnum check with
RuntimeFeature.IsDynamicCodeSupportedwe would root all that enum dispatching logic for every T that flows through, and as they are dispatching stubs, that's basically everything. We can't use any of our usual tricks here - like putting the code behind virtual dispatch - because there are no actual rooted instantations over the user's TEnum. The entire point was that we didn't need to know about those types in Npgsql. Finally, on the more exotic end there is no 'unsafe but guaranteed' equivalence of virtual table layout between T = enum and its underlying T = int either. Which means we cannot 'safely' doUnsafe.As<PgConverter<IntEnum>(PgConverter<int>)and be guaranteed their virtual slots line up such that we end up in the virtual method we intended to call. As a result NativeAOT support is a bit worse than I would have wanted, but it sits on the edge of what's reasonably possible.Suffice to say this ended up being an even more hilariously painful feature to land than I initially assumed, but it's type safe, trim safe, and complete with as much compositional support as the runtime provides guarantees for.