efcore / EFCore.NamingConventions

Entity Framework Core plugin to apply naming conventions to table and column names (e.g. snake_case)
Apache License 2.0
717 stars 73 forks source link

Consider naming conventions for Cosmos DB or other non-relational databases #122

Open TarasKovalyk opened 2 years ago

TarasKovalyk commented 2 years ago

Hello I am using EFCore.NamingConventions (5.0.2) and Microsoft.EntirtyFrameworkCore.Cosmos (5.0.13) (Besides it doesn't work for Microsoft.EntirtyFrameworkCore.Cosmos (6.0.1) and EFCore.NamingConventions (6.0.0))

In my startup I have added the following registration image and DbContext image

However all fields within TestEntity are still in PascalCase. Could you help me what is wrong with the usage of this library? Or it won't work with non relational db?

ajcvickers commented 2 years ago

@TarasKovalyk This package targets relational-specific naming (e.g. tables) in relational databases.

TarasKovalyk commented 2 years ago

@ajcvickers thanks for your response. Actually now we are using as workaround our own approach for this purpose.

image However there was a challenge to configure 'camel casing' for properties which were navigations. For that we also use reflection and some kind of extension for OwnedNavigationBuilder.

Once this possibility is added to EFCore.NamingConvention lib we will use it.

roji commented 2 years ago

@TarasKovalyk for now, as @ajcvickers wrote above, EFCore.NamingConventions is a relational-only package, and there are no real plans to change that at this point (but if lots of users request it, that may change). It's possible to create a separate package, eg. EFCore.NamingConventions.Cosmos which would do the same, but EFCore.NamingConventions wouldn't support Cosmos, since that would require taking a reference on the Cosmos provider package, which is something users of EFCore.NamingConventions don't generally want.

To understand why things really are separate here, consider that different database types have different concepts of "columns" and what gets named in the database (i.e. what this library needs to affect). For example, in relational database there's typically a schema (which needs to be rewritten), but Cosmos does not have this concept.

julealgon commented 2 years ago

EFCore.NamingConventions is a relational-only package, and there are no real plans to change that at this point

@roji I believe this statement is currently misleading on the readme then: https://github.com/efcore/EFCore.NamingConventions/blob/d85cb49f52b2fc5d86576e1749f7ba3c371eef90/README.md?plain=1#L56

If there are no plans to make this work with the CosmosDB provider, it would be nice to make that point a bit more clear in the documentation, stating this will only work with relational database providers.

To understand why things really are separate here, consider that different database types have different concepts of "columns" and what gets named in the database (i.e. what this library needs to affect). For example, in relational database there's typically a schema (which needs to be rewritten), but Cosmos does not have this concept.

This confuses me a bit. Isn't the model created for the dbcontext already some sort of abstraction that is leveraged to obtain the final result? Why can't the Cosmos provider build its serialization model using the same properties that are used for columns on a relational provider? From a distance, this all feels like some sort of design smell to me: if we are manipulating the provider-agnostic abstraction, then it should ideally reflect properly on any provider, including Cosmos.

Perhaps @ajcvickers could provide some insight here as well?

roji commented 2 years ago

@roji I believe this statement is currently misleading on the readme then:

You're right, I added the word "relational" in there.

This confuses me a bit. Isn't the model created for the dbcontext already some sort of abstraction that is leveraged to obtain the final result? Why can't the Cosmos provider build its serialization model using the same properties that are used for columns on a relational provider? From a distance, this all feels like some sort of design smell to me: if we are manipulating the provider-agnostic abstraction, then it should ideally reflect properly on any provider, including Cosmos.

EF Core doesn't attempt to provide a uniform abstraction over all the databases in the world, since that simply isn't doable. In most relational databases, a table is identified not just by a name, but also by a schema; Cosmos does not have this concept. Cosmos, on the other hand, has its own special concepts, such as partition keys, which make no sense on relational databases. So each provider type has its own APIs for working with these database-specific concepts, and we design those APIs to match the database in the most idiomatic way; for example, we talk about "columns" in relational databases, but about properties in Cosmos's JSON documents. That's just the reality of the very different database types that exist in the world.

julealgon commented 2 years ago

In most relational databases, a table is identified not just by a name, but also by a schema; Cosmos does not have this concept.

Sure, but we are not talking about tables here though. I understand you are just providing an example, but I don't think it's that useful here.

Cosmos, on the other hand, has its own special concepts, such as partition keys, which make no sense on relational databases.

Again, that's fair. It's something that has no parallels in relational databases, and thus why it needs those special extensions to configure. When a concept is completely new, I'm fine with having a completely new way of configuring it.

So each provider type has its own APIs for working with these database-specific concepts, and we design those APIs to match the database in the most idiomatic way; for example, we talk about "columns" in relational databases, but about properties in Cosmos's JSON documents.

Now, about this, isn't it just semantics though? Since Cosmos doesn't have "columns", why not use the underlying value used to name columns in relational databases to name the properties?

I understand that some concepts don't carry over and have to be implemented in completely different ways, but names for each of the properties seems like something that should be covered by the main abstraction, since it is so basic/general. It feels to me that EF should be able to abstract these names in a general-purpose manner, that can then be leveraged by relational providers for column names, and for document databases for property names.

Obviously, I'm saying all this from the perspective of someone who is not familiar with the EF codebase. It just seems like there is a gap somewhere if we are currently unable to abstract a simple thing such as the name used for mapping in both situations.

roji commented 2 years ago

@julealgon let's suppose for a moment we did have a universal concept of a data-store property name, which would be used for relational columns names, Cosmos JSON property names, etc. That would indeed enable this plugin to rewrite that specific thing; but the job of this plugin is also to rewrite other things: the (relational) table/schema names, primary key names, etc. In the same way, Cosmos users would also expect this plugin to rewrite e.g. the Cosmos container name, and whatever else has a name in Cosmos. So at the end of the day, you really do need two plugins, one which can rewrite all the relational stuff, and another which can rewrite all the Cosmos stuff. I hope that makes sense.

wbuck commented 1 year ago

I was also hoping this would work for Cosmos DB. @TarasKovalyk would you be willing to post your full work around for this?

TarasKovalyk commented 1 year ago

@wbuck please find an archive with the needed configurations for the usage of CosmosDB with EF and fix the issue mentioned above BuildingBlocks.Data.EFCosmosDb.zip TaskManagement.Data.zip