npgsql / efcore.pg

Entity Framework Core provider for PostgreSQL
PostgreSQL License
1.52k stars 223 forks source link

UseNodaTime EF+Dapper #1806

Closed suhrab closed 2 years ago

suhrab commented 3 years ago

We use both EF and Dapper in our application to work with postgresql database. We follow recommendation to use NodaTime for type mapping https://www.npgsql.org/doc/types/nodatime.html Extension method that enables NodaTime for EFCore is calling next method each time DbContext is created NpgsqlConnection.GlobalTypeMapper.UseNodaTime(); https://github.com/npgsql/efcore.pg/blob/6b0b62c503cfd3737d4150960212f8c627666745/src/EFCore.PG.NodaTime/Extensions/NpgsqlNodaTimeDbContextOptionsBuilderExtensions.cs#L26

Issues:

  1. NpgsqlConnection.GlobalTypeMapper.UseNodaTime(); is called for every DbContext instance. I didnt measured the overhead of this. Suggestion: Put NpgsqlConnection.GlobalTypeMapper.UseNodaTime(); into assembly Initializer for Npgsql.EntityFrameworkCore.PostgreSQL.NodaTime.dll. Not sure if it is a good idea
  2. I get exceptions with Dapper something like: cannot cast DateTime to NodaTime.LocalDate, until the first instance of DbContext is ever created. To fix this I put NpgsqlConnection.GlobalTypeMapper.UseNodaTime(); on the application startup
roji commented 3 years ago

That's a good point. Module initializers have only been introduced in C# 9, so they won't work on older version of .NET which are still supported. We could simply put this in the static constructor of the extension class for UseNodaTime; it wouldn't fix your Dapper issue, but it seems reasonable to require users to manually set up the Npgsql ADO NodaTime plugin in that scenario.

suhrab commented 3 years ago

Module initializers are first time implemented in C# version 9. But they do exist in .net long time ago. Before C#9 we could use Fody Weavers https://github.com/Fody/ModuleInit You can safely use module initializer and target different runtimes.

static constructor vs module initializer

One particular pain point of current static constructor approaches is that the runtime must do additional checks on usage of a type with a static constructor, in order to decide whether the static constructor needs to be run or not. This adds measurable overhead.

roji commented 3 years ago

Would you like to submit a PR for this?

suhrab commented 3 years ago

https://github.com/npgsql/efcore.pg/pull/1829

roji commented 2 years ago

@suhrab sorry I've left this (and #1829) unanswered for so long.

I have a plan for 7.0 which would hopefully altogether obviate global use of plugins (which isn't a great thing in general).

In a nutshell, the idea would be to introduce a new user-facnig NpgsqlDataSource abstraction, which would roughly correspond to a connection pool inside Npgsql. One of the advantages of this, is that we'd allow the user enable plugins (and other settings) at that level, and the plugin would automatically apply for all connections managed by that pool (or data source).

If we do this, then the EF Core provider would get its connections from such an NpgsqlDataSource, and calling UseNodaTime would simply set up the plugin on that same NpgsqlDataSource. This would mean that the plugin's effects would no longer be global, etc.

As a result, I'm going to go ahead and close this issue (and #1829) - thanks for doing the work (and conversation) and apologies it didn't end up getting merged.