Skip to content

CLR enum underlying type mapping#6581

Open
NinoFloris wants to merge 8 commits into
npgsql:mainfrom
NinoFloris:enum-underlying-type-mapping
Open

CLR enum underlying type mapping#6581
NinoFloris wants to merge 8 commits into
npgsql:mainfrom
NinoFloris:enum-underlying-type-mapping

Conversation

@NinoFloris
Copy link
Copy Markdown
Member

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:

  • int, uint => int4
  • long, ulong => int8
  • byte, sbyte, short, ushort => int2

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:

  • Cleaning up the provider/concrete split that proliferated 'AsObject' logic all over the codebase.
  • Getting all non-generic converter dispatching to use the centralized PgConverter.Read (and so on) methods.
  • Rationalizing all PgTypeInfo variance logic to be able to confidently add the enum <-> underlying exemptions.

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.IsDynamicCodeSupported we 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' do Unsafe.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.

Copilot AI review requested due to automatic review settings May 19, 2026 11:28
@NinoFloris NinoFloris requested review from roji and vonzshik as code owners May 19, 2026 11:28
Copy link
Copy Markdown

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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 in AdoTypeInfoResolverFactory.
  • 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 PgEnumTests and add enum-underlying tests in EnumTests.

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.

Comment thread src/Npgsql/Internal/Converters/EnumUnderlyingConverter.cs
Comment thread src/Npgsql/Internal/Converters/EnumUnderlyingConverter.cs
Comment thread src/Npgsql/Internal/ResolverFactories/AdoTypeInfoResolverFactory.cs Outdated
Comment thread test/Npgsql.Tests/Types/EnumTests.cs
Comment thread src/Npgsql/Internal/Converters/ArrayConverter.cs Outdated
Copy link
Copy Markdown
Contributor

@vonzshik vonzshik left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good, just one moment

Comment thread src/Npgsql/Internal/Converters/EnumUnderlyingConverter.cs
Copilot AI review requested due to automatic review settings May 23, 2026 15:48
@NinoFloris NinoFloris force-pushed the enum-underlying-type-mapping branch from 1f3121c to 5ba7640 Compare May 23, 2026 15:48
@NinoFloris NinoFloris requested a review from vonzshik May 23, 2026 15:55
Copy link
Copy Markdown

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 16 out of 16 changed files in this pull request and generated 4 comments.

Comment thread src/Npgsql/Internal/ResolverFactories/AdoTypeInfoResolverFactory.cs
Comment thread src/Npgsql/Internal/Converters/ArrayConverter.cs Outdated
Comment thread src/Npgsql/Internal/ResolverFactories/UnmappedTypeInfoResolverFactory.cs Outdated
Comment thread src/Npgsql/Internal/PgConverter.cs
Copy link
Copy Markdown

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 16 out of 16 changed files in this pull request and generated 3 comments.

Comment thread src/Npgsql/Internal/Converters/EnumUnderlyingConverter.cs
Comment thread src/Npgsql/Internal/Converters/EnumUnderlyingConverter.cs
Comment thread src/Npgsql/Internal/Converters/EnumUnderlyingConverter.cs
Copy link
Copy Markdown

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 16 out of 16 changed files in this pull request and generated 1 comment.

Comment thread src/Npgsql/Internal/Converters/EnumUnderlyingConverter.cs
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.

Lack a default Type Mapping for short array in composite to PostgreSQL smallint[]

3 participants