dotnet / runtime

.NET is a cross-platform runtime for cloud, mobile, desktop, and IoT apps.
https://docs.microsoft.com/dotnet/core/
MIT License
14.64k stars 4.57k forks source link

Support for DECFLOAT in System.Data.DbType #41026

Open cincuranet opened 3 years ago

cincuranet commented 3 years ago

Background and Motivation

DECFLOAT datatype is quite common in databases nowadays and would make sense to add the "support" for it into DbType enum.

Proposed API

public enum DbType
{
    AnsiString = 0,
    Binary = 1,
    Byte = 2,
    Boolean = 3,
    Currency = 4,
    Date = 5,
    DateTime = 6,
    Decimal = 7,
    Double = 8,
    Guid = 9,
    Int16 = 10,
    Int32 = 11,
    Int64 = 12,
    Object = 13,
    SByte = 14,
    Single = 15,
    String = 16,
    Time = 17,
    UInt16 = 18,
    UInt32 = 19,
    UInt64 = 20,
    VarNumeric = 21,
    AnsiStringFixedLength = 22,
    StringFixedLength = 23,
    Xml = 25,
    DateTime2 = 26,
    DateTimeOffset = 27,
+    DecFloat16 = 28,
+    DecFloat34 = 29,
}

Risks

I believe this is a low risk change, as it's adding only new values to the end of the enum. The support is purely in the enum and ADO.NET providers can then use the values as they wish/feel usable. For existing ADO.NET world nothing will change.

ghost commented 3 years ago

Tagging subscribers to this area: @roji, @ajcvickers See info in area-owners.md if you want to be subscribed.

Wraith2 commented 3 years ago

DECFLOAT datatype is quite common in databases nowadays

It is? Which databases support it?

cincuranet commented 3 years ago

First of all it's part of SQL:2016 standard. I'm aware of DB2 supporting it. Firebird 4 will/does support it. And I've heard Postgres is going to get the support (@roji surely knows more).

roji commented 3 years ago

Yeah, DECFLOAT seems to have made it into SQL:2016, and PostgreSQL indeed strives to conform as much as possible - but there's definitely no support as of now (here's a relevant discussion).

Given the very limited subset of databases which actually have this right now (DB2, Firebird), the value here seems a bit limited...

Wraith2 commented 3 years ago

The problem with System.Data is that it's part of the framework and because of that it only gets versioned on major releases. The branch for .net 5.0 has been made so any feature requests now can only ship in 6.0 or later. The change will never make it into desktop netfx. If you're the author of an ADO provider that supports desktop netfx then trying to use these new enum members would be difficult and costly in development terms. You're going to end up abstracting away the System defined enum and use your own. In general I think providers are best served by using their own type enum.

I've nothing against DECFLOAT despite never having heard of it. I do think that moving as much functionality as possible out of System.Data.* namespaces is probably a good idea and thus not continuing to use or extend this enum.

cincuranet commented 3 years ago

I wish I could abstract away the usage of this enum, but i.e. DbParameter.DbType is using in public/developer-facing API. FbParameter has it's own FbDbType property, but still some "abstract" (i.e. OR mappers) code uses only what DbParameter offers.

I considered the netfx case, but in this case, for me personally, falling back to DbType.Object on netfx would be reasonable trade-off, given with every passing day the netfx being more legacy.

roji commented 3 years ago

@Wraith2 it's true that it's probably too late to add anything into 5.0.

The change will never make it into desktop netfx. If you're the author of an ADO provider that supports desktop netfx then trying to use these new enum members would be difficult and costly in development terms. You're going to end up abstracting away the System defined enum and use your own. In general I think providers are best served by using their own type enum.

Can you provide more detail? It's quite simple for a provider to have some conditionally-compiled code handling new values of DbType, no? We already do this with methods which are added only in .NET Core, is there any additional complexity for enums that I'm missing?

I do think that the utility of a new DbType value is proportional to how many database providers are actually going to support it (and so DECFLOAT doesn't exactly shine). However, a DbType.Json is a good counter-example (https://github.com/dotnet/runtime/issues/30677) - lots of databases do support it, so why not add it?

cincuranet commented 3 years ago

I do think that the utility of a new DbType value is proportional to how many database providers are actually going to support it (and so DECFLOAT doesn't exactly shine).

On the other hand, adding value to enum seems fairly cheap, isn't it?

However, a DbType.Json is a good counter-example (#30677) - lots of databases do support it, so why not add it?

I absolutely second your opinion here.

am11 commented 3 years ago

Given the excerpt from DbType docs:

image

In the context, where DecFloat16 and DecFloat34 are intended to be used, do the existing Double and Decimal hold any particular (differentiating) meaning? If not, can they be used instead, respectively?

cincuranet commented 3 years ago

The i.e. double is binary floating point, while decfloat is decimal floating point. The range of DECFLOAT(16) is -9.999999999999999 x 10^384 to -1.0 x 10^-383 : 1.0 x 10^-383 to 9.999999999999999 x 10^384. That doesn't quite match. I would say it's a similar story to DbType.DateTime and DbType.DateTime2 (at least how it's related to SqlClient).