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

Reverse Engineer - Custom Database Column Type Mapping for Postgres #1137

Closed
jinweijie opened this issue Oct 12, 2021 · 19 comments
Closed

Reverse Engineer - Custom Database Column Type Mapping for Postgres #1137

jinweijie opened this issue Oct 12, 2021 · 19 comments

Comments

@jinweijie
Copy link

Hello @ErikEJ ,

First, thanks for the great tool. We are trying to use EF Power Tools to replace the Entity Developer which we are currently using.

We are using Postgres database and for date time column, we use "timestamp with time zone". With Entity Developer, we are able to generate entity property type DateTimeOffset, which makes other parts of the application easily dealing with time zone. (We know the default mapping of NpgSql for "timestamp with time zone" is DateTime because in actual storage in Postgres is UTC date time).

I tried to customize the handlebar template, but didn't figure out how to get the actual database type property in template (also seems no clear documentation listing all the possible properties for handlebar template).

Also for efpt.config.json, I didn't see any configuration for this purpose.

So is there some possible easy way to customize the data table column type mapping?

Thanks a lot in advance.

Steps to reproduce

  1. Create a table in Postgres with some column using "timestamp with time zone".
  2. Reverse engineer to C# entity class using EF Power Tools
  3. The property type is DateTime.
  4. Would like to have a way to customize it to DateTimeOffset.

Further technical details

EF Core Power Tools version: 2.5.774

Database engine: Postgres

Visual Studio version: Visual Studio 2019 16.11.4

@ErikEJ
Copy link
Owner

ErikEJ commented Oct 12, 2021

Maybe you can use the column exclusion feature? #572 (comment)

@jinweijie
Copy link
Author

jinweijie commented Oct 13, 2021

Hi Erik,

Thanks for the reply.

The column exclusion feature seems not to meet my need. So I did a quick dirty test to modify the source code and it works:

in ColumnRemovingScaffoldingModelFactory, add:

        protected override TypeScaffoldingInfo GetTypeScaffoldingInfo(DatabaseColumn column)
        {
            if (column.StoreType == "timestamp with time zone")
                return new TypeScaffoldingInfo(typeof(DateTimeOffset), false, null, null, null);

            return base.GetTypeScaffoldingInfo(column);
        }

I am thinking to make this a feature to customize the column type mapping and create RP, how do you think? Thanks.

@ErikEJ
Copy link
Owner

ErikEJ commented Oct 13, 2021

Not sure if this is something I would like to maintain
@roji - any ideas?

@roji
Copy link

roji commented Oct 13, 2021

First, note that some significant changes to timestamp mapping are coming in Npgsql 6.0 - see this blog post and/or the release notes. The most relevant bit here is that the EF Core provider will start mapping DateTime to timestamp with time zone by default, and will only ever read and write DateTimes with Kind=Utc for that type.

I believe that UTC DateTime is a good way to represent a UTC timestamp in .NET (and to map to PostgreSQL timestamp with time zone), so once you make the switch to 6.0, having timestamp with time zone reverse-engineer to DateTime is actually OK. DateTimeOffset is also still supported (as long as the offset is 0), but I'm not sure there's an advantage for using that over UTC DateTime.

Aside from that, you're basically asking for a way to override the default CLR type (DateTime) for a given database type (DateTimeOffset). This kind of mapping change can generally be done via a type mapping plugin - see this for Npgsql's NodaTime plugin as an example. You'd basically override FindMapping to return the DateTimeOffset mapping for timestamp with time zone - @ErikEJ's Power Tools should pick up this change automatically.

@ErikEJ
Copy link
Owner

ErikEJ commented Oct 13, 2021

@roji Actually, it will not, EF Core Power Tools never attempts to load user code, it simply generates files

@roji
Copy link

roji commented Oct 13, 2021

@ErikEJ are you saying that EF Core plugins aren't taken into account? For example, if I use the SQL Server NetTopologySuite plugin, I will not be able to reverse-engineer a database with geometry/geography columns?

@jinweijie
Copy link
Author

jinweijie commented Oct 13, 2021

Hi @ErikEJ and @roji , thanks for your reply.

I created a commit (jinweijie@208ea38) for this feature. It works nicely for my use case. If you please have a check? If you think it might help, I can create a PR, otherwise, I might just use a custom build, also not a big problem :).

The configuration looks like this:

image

@roji ,for the DateTime or DateTimeOffset topic, my problem to use DateTime is that before saving to database, I need an additional step to convert to UTC time. Also we have an existing application which use DateTimeOffset everywhere. Or do you have a better way to handle it? Thanks.

image

Thanks again.

@ErikEJ
Copy link
Owner

ErikEJ commented Oct 13, 2021

@roji I support a fixed number of "known" plugins... (I build a serviceprovider based on the user options selected in a console app)

@roji
Copy link

roji commented Oct 13, 2021

If you think it might help, I can create a PR, otherwise, I might just use a custom build, also not a big problem :).

I don't think this kind of specific customization belongs inside EF Power Tools... If we can somehow make the Power Tools take plugins into account, it would be possible to take care of it that way, so:

@roji I support a fixed number of "known" plugins... (I build a serviceprovider based on the user options selected in a console app)

I see... then yeah, it's a problem. The EF reverse-engineering CLI builds its service provider based on the user's configuration (as you know...) and so takes plugins into accounts. Is this something you think is doable at some point in the Power Tools? It would remove the need for you to know about any specific plugin etc...

for the DateTime or DateTimeOffset topic, my problem to use DateTime is that before saving to database, I need an additional step to convert to UTC time.

Ideally, the DateTime instances in your application would already be with Kind=UTC - just like your DateTimeOffsets have to be with offset 0 (it really is the same thing); I would advise checking where you actually create the instances of DateTime or DateTimeOffset, and ensuring that they're created as UTC there ("UTC everywhere"). If you do this, then using DateTimeOffset makes little sense, since it always has the same offset anyway (but of course, if your application is already working in a certain way, that's OK).

@ErikEJ
Copy link
Owner

ErikEJ commented Oct 13, 2021

Is this something you think is doable at some point in the Power Tools? It would remove the need for you to know about any specific plugin etc...

I specifically want to avoid this, otherwise I get into the migrations support nightmare, where I actually need to load the users project.

@roji
Copy link

roji commented Oct 13, 2021

OK, that's understandable... In that case, you could think of some feature to allow users to override default mappings, i.e. specify that for a given database type, some arbitrary .NET CLR type should be used instead - that would take care of this. But I can't imagine there being lots of users needing this...

@ErikEJ
Copy link
Owner

ErikEJ commented Oct 13, 2021

@roji there has been few users asking.. i will review the code changes made above.

@ErikEJ
Copy link
Owner

ErikEJ commented Oct 14, 2021

I think the proposed solution is too narrow in scope by only knowing about existing CLR types (and not user defined types) , and maybe to wide is scope (not targeting a particular column)

see related issues #16 #815 #1082

@roji
Copy link

roji commented Oct 14, 2021

Could be... I think that asking to override type mappings on a per-column basis is a bit much when reverse-engineering, but maybe this is something you want to support. Mapping to user-defined types is certainly a bit niche, at least the moment; that may change when we introduce JSON support, but once again, it seems a bit much to allow users to specify which user CLR type they want to reverse engineer per column...

@ErikEJ
Copy link
Owner

ErikEJ commented Oct 14, 2021

Appreciate the feedback, @roji -

@jinweijie Please create a PR!

@ErikEJ
Copy link
Owner

ErikEJ commented Oct 31, 2021

Given that this can be achieved in other ways and the very low number of requests for this, using for example a partial class for the entity, this is not something that I plan to add and support.

I will revisit if demand arises.

@ErikEJ ErikEJ closed this as completed Oct 31, 2021
@jinweijie
Copy link
Author

Ok, thanks anyway. Just leave a link here in case if somebody also looking for this feature. jinweijie@208ea38

Thanks.

@clintsinger
Copy link

Just to add to this. I was looking for a way to map my code-first POCO classes to a json column and it appears the only way to do this is to exclude the json column during reverse engineering and then add it again via a partial class of the entity itself and corresponding updates to the DbContext.

I'd would have loved it if I could have defined in the config the mapping so that each time I run the reverse engineering I get a nice and clean entity object and the appropriate initialization is handled already in the DbContext with out me having to hand write it and track it separately.

@ErikEJ
Copy link
Owner

ErikEJ commented Oct 27, 2023

@clintsinger this issue is closed. If you have any feature requests please create a new issue with sufficient details for me to understand what you are asking for!

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

No branches or pull requests

4 participants