dotnet / efcore

EF Core is a modern object-database mapper for .NET. It supports LINQ queries, change tracking, updates, and schema migrations.
https://docs.microsoft.com/ef/
MIT License
13.73k stars 3.18k forks source link

Allow precision on type mappings with scale #11439

Closed roji closed 6 years ago

roji commented 6 years ago

11419 adds better facet management on relation type mappings (to address #11167).

It's great to see this, but it seems that the options are currently Size or PrecisionAndScale. A precision-only option exists for date/time types in both MySQL and PostgreSQL. PostgreSQL also allows decimal/numeric with a precision but without scale (in which case scale defaults to 0).

ajcvickers commented 6 years ago

@roji Because I was considering whether just precision and just scale were really needed, or whether the more general "size" could be used for all, and also because we don't really fully support "precision" and "scale" yet--I have been adding bits and pieces of support as needed to get other things working.

I assume from a Postgres perspective you are saying that just precision and just scale are need?

roji commented 6 years ago

I'm not aware of a case where I can specify scale but not precision, so at least for PostgreSQL just precision should be enough. After all, these are "unnamed positional arguments" on the data type, and it would be impossible to know whether a single number in parentheses means precision or scale... But it may be worth looking at what Oracle/MySQL/whatever do.

ajcvickers commented 6 years ago

"These are 'unnamed positional arguments' on the data type." This is why I was wondering about just using "size" but I think the problem is knowing where that "size" comes from in the type mapping, and consequentially how it interacts with what is set on the parameter. I think the best approach will likely be to allow the time mapping to have size, precision, and scale set independently, and then to orthogonality allow the type mapping to choose whether to append the size, the precision, or the precision and scale, and then just for completeness scale on it's own. (There also seems to be some ambiguity between size and precision for numerics on Oracle.)

roji commented 6 years ago

That sounds like a good way forward to me... Another way to approach this is to allow the mapping to render it's complete data type (including size/precision/scale), like it renders literals. This would allow, for example, the PostgreSQL numeric mapping to omit the scale when it's zero (the default). This goes with the general direction in the new mapper of not trying to do too much but rather leaving it up to the provider etc.

It would also have to parse the same, possibly with the help of some basic support from you.

ajcvickers commented 6 years ago

@roji I think a type mapping should be able to override StoreType and do a complete rendering if needed. I don't think EF now does any manipulation of the store type name unless opted into using the new flags.

We already parse out the base name and either a single or pair of postfixed values and put the results in the TypeMappingInfo. However, a single value is always parsed into "size" and two values are always parsed into "precision and scale". These flags could likely be extended to influence this parsing as well, so if the type expects either precision and scale or just precision, then a single value will be parsed into precision instead of size. I'll look into that, but it may be post 2.1.

roji commented 6 years ago

I understand. I do hope you'll be able to do this for 2.1, as there have already been so many changes to the type mapping system and some stability in this area going forward would be nice...

roji commented 6 years ago

Great, thanks guys... I'll start working on preview3 very soon and will report.