-
Notifications
You must be signed in to change notification settings - Fork 228
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
Global option to always use PostgreSQL native enums? #2435
Comments
There's not much risk here; if you forget to add it, it simply would throw an exception when you try to use it. At that point you'd be reminded to add it. Stepping back... As you write, it's not appropriate to automatically map all auto-discovered enums to the database, since many enums aren't meant to be mapped; this is why a user gesture is needed to indicate that a specific enum should be mapped. In addition, auto-discovery of types in an assembly isn't friendly to trimming, and we'd also somehow have to know which assembly (or assemblies) to scan. That's nothing something I'd want Npgsql to do. I do agree that having to specify the enum twice is sub-par; #1026 tracks improving that. However, in practice this is very rarely a problem, and nobody has actually complained about it as far as I can remember. So this isn't a very high-priority issue. |
Closing as no response was provided. |
@roji Oh sorry, I missed this one!
Well, wouldn't the enum property result in an integer column in the database in that case? What exception?!
But why would you want to do an assembly scan? Why not just look at the properties of entity classes that have an enum as their type and map those? That's what I had in mind personally, not an assembly scan. Is that not feasible? |
You're right, I had the scenario in mind where you do call MapEnum, but forget to have EF create it via HasPostgresEnum - that would cause an exception. But you're right that if you forget both, you'll just get the default int mapping instead. Thinking about this again, I do think you're right and there's value in having a global opt-in to native PostgreSQL enums. The provider could have an (optional) convention that goes over the model, and for every enum CLR type it finds there, configures it as a PostgreSQL enum. However, there's a technical stumbling block - the MapEnum call. The below shouldn't really interest you - it's mostly implementation notes for myself. I'm reopening to think more about this.
In short, I agree there's something to be done here, but it isn't going to be simple. |
Hello Roji, has there been any traction on this? I've just started looking into migrating to Npgsql + Entity Framework 7 and already got the warning that using For now, it's just a warning, the GlobalTypeMapper is still supported, but at some point this will be removed I assume. |
@carlosrfernandez GlobalTypeMapper.MapEnum definitely won't be removed until an alternative mapping solution is provided for EF, it's definitely OK to continue using that. Note that this issue is about a global option to always use native enums, which is different from a mechanism map a single enum that doesn't depend on GlobalTypeMapper. I have some thoughts on how to do things better here, but it depends on some EF-side changes (dotnet/efcore#29489). |
This would be really convenient. I'm experiencing this exact issue. It's a bit tedious to have to map each and every single enum manually in two places when you just want all the CLR enums in the model to be mapped to Postgres ones. A simple opt-in switch would be perfect. |
Hi there. Thank you for all your hard work.
Currently, if you want all your enums to be mapped to corresponding Postgres enums, you'll have to register each one of them explicitly twice in two different places, like so:
This indeed works, but it's easy to see how it could become tedious and error-prone over time; you could add a new enum to your schema and very easily forget to come add it here.
One workaround is to use reflection to search in the assembly and discover all the enums and map them, but this can become tricky in cases where the assembly contains other non-relevant enums but you only want to map those that are actually used in your entity classes. And even if you manage to pull that off cleanly, you'll still have to have two methods and call them in two different places (i.e.
OnModelCreating
and the staticDbContext
contructor).I think it's fair to say this provides a sub-par developer experience.
Ideally, all you would have to do to achieve this, would be to turn on some sort of a "switch", as it were, to let Npgsql know that you want your enums to be mapped to Postgres enums, as opposed to
integer
columns which is the default behavior in EF Core:I would assume this was at some point considered but for some reason eventually discarded, if so I'm curious what that reason was and whether it still applies. I think having a feature like this would certainly help improve the developer experience, this is definitely one of those things that makes you scratch your head and go "Wait, why isn't there a simple option for this?".
Thank you in advance.
The text was updated successfully, but these errors were encountered: