Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Stop reading Npgsql enum type mappings in NpgsqlTypeMappingSource #3063

Closed
roji opened this issue Jan 16, 2024 · 0 comments · Fixed by #3167
Closed

Stop reading Npgsql enum type mappings in NpgsqlTypeMappingSource #3063

roji opened this issue Jan 16, 2024 · 0 comments · Fixed by #3167
Assignees
Labels
breaking change Represents a breaking change enhancement New feature or request
Milestone

Comments

@roji
Copy link
Member

roji commented Jan 16, 2024

Currently, NpgsqlTypeMappingSource pulls in enum type mappings from the lower-level Npgsql layer; the nice aspects of this is that it's sufficient to configure enums on your NpgsqlDataSource (at the lower-level), and that makes them automatically work with EF too.

Unfortunately that causes some pretty severe issues:

  • Since NpgsqlTypeMappingSource is singleton, NpgsqlDataSource must also be a singleton.
    • This is especially problematic since we also optionally resolve NpgsqlDataSource from the application service provider; but since NpgsqlOptionsExtension determines when a new internal service provider is needed, but can't access the application service provider, this is currently causing leaks via EF's service provider caching mechanism #2891.
  • It also makes it problematic to call UseNpgsql() with a connection, when that connection comes from a data source; EFCore.PG doesn't see that data source, and therefore doesn't have any enum mappings configured on it. This is the root cause of #3053.

The solution here is simply to have users directly configure enum mappings at the EFCore.PG level; at that point NpgsqlDataSource no longer needs to be a singleton at all, and the mappings will always be there. The disadvantage would be that users need to set up mappings twice - once in EFCore.PG and once in Npgsql.

#2542 would be the mitigation for this, i.e. having EF's UseNpgsql() accept an NpgsqlDataSourceBuilder. This would allow users to continue configure enums once, but at the EF level instead of at the Npgsql level: calling MapEnum() at the EFCore.PG level would automatically also call MapEnum on NpgsqlDataSourceBuilder.

Finally, note the intersection with #1026, which is also about moving enum mappings to be context options, with a convention that propagates them into the model for migrations (removing the need to map twice at that level as well).

/cc @ajcvickers

@roji roji added the enhancement New feature or request label Jan 16, 2024
@roji roji added this to the 9.0.0 milestone Jan 16, 2024
@roji roji self-assigned this Jan 16, 2024
roji added a commit to roji/efcore.pg that referenced this issue May 12, 2024
Fixes npgsql#2891
Fixes npgsql#3063
Fixes npgsql#1026

Co-authored-by: Nino Floris <mail@ninofloris.com>
roji added a commit to roji/efcore.pg that referenced this issue May 13, 2024
Fixes npgsql#2891
Fixes npgsql#3063
Fixes npgsql#1026

Co-authored-by: Nino Floris <mail@ninofloris.com>
roji added a commit to roji/efcore.pg that referenced this issue May 13, 2024
Fixes npgsql#2891
Fixes npgsql#3063
Fixes npgsql#1026

Co-authored-by: Nino Floris <mail@ninofloris.com>
roji added a commit to roji/efcore.pg that referenced this issue May 17, 2024
Fixes npgsql#2891
Fixes npgsql#3063
Fixes npgsql#1026

Co-authored-by: Nino Floris <mail@ninofloris.com>
@roji roji closed this as completed in 6b3ca68 May 18, 2024
@roji roji added the breaking change Represents a breaking change label May 18, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
breaking change Represents a breaking change enhancement New feature or request
Projects
None yet
Development

Successfully merging a pull request may close this issue.

1 participant