DuckDBType.Date representation as DateTime/DateOnly

[VitaliyMF]

It seems DuckDB.NET handles date columns differently in netstandard and net6/net8 builds: https://github.com/Giorgi/DuckDB.NET/blob/develop/DuckDB.NET.Data/Internal/Reader/DateTimeVectorDataReader.cs

System.DateOnly is not available in netstandard2.1, and this causes weird situation when DuckDB.NET is referenced from, say, net8 app (so dates are returned in DbDataReader as System.DateOnly) but when datareader is processed in the netstandard2.1 library it simply cannot handle these values because it knows nothing about this type which even cannot be converted to DateTime (via Convert.ToDateTime, for instance).

It would be helpful to have an option to specify which .NET type use for date-only values (for net6+ builds).

[Giorgi]

You can use DuckDBDataReader.GetFieldValue<> to read those columns with a desired type.

[VitaliyMF]

I cannot - this processing is inside the lib, and it simply reads all values via DbDataReader.GetValues(object[]) so it gets values as they are returned by ADO.NET provider implementation. It seems implementations like System.Data.SqlClient, NpgSql, MysqlConnector return DateTime in this case (when no desired type specified) which guarantees 100% backward compatibility.

[Giorgi]

Those libraries return DateTime because when they were started DateOnly didn't exist. DbDataReader.GetValues(object[]) will box all the values read from the database and unbox when casting to specific type, so I would recommend updating that library and prepare for moving to .NET Core

[VitaliyMF]

This makes exactly this particular ADO.NET provider DuckDB.NET incompatible with huge number of libraries that only target netstandard2.0 or netstandard2.1 and if this concrete lib doesn't require any net6/net8/net9 etc specific API, it should not have special builds for these targets.

It is considered that modern NET apps (NET8+) can consume netstandard-compatible libs without problems. That's why it is good idea to return only known types (DateTime instead of DateOnly) for DbDataReader.GetValue and DbDataReader.GetValues methods - and this is how DuckDB.NET netstandard2.1 actually works (it is weird that important API behaviour changes depending on the build, in fact).

In my case, it seems I'll need to wrap DuckDBDataReader with my implementation that will explicitly convert DateOnly to DateTime values.

[Giorgi]

I understand where you are coming from and it would be good to be compatible with those libraries but it would be great if those libraries moved to .NET Core and added support for modern data type as well as migrate to GetFieldValue. This way those libraries will benefit from increased performance too. Also, it would benefit not only DuckDB.NET users but also other ADO.NET provider consumers. Instead of asking me to be compatible with libraries that don't move forward, I think it's better to ask those library authors (maybe you already have done) to support modern .NET (alongside netstandard if they want to) I'm sure those libraries will benefit from other features that modern .NET has. Until then as you have said you can decorate DuckDBDataReader with your custom reader.

As for specific APIs from modern .NET, this library avoids boxing when reading numeric types on .NET 8:

DuckDB.NET/DuckDB.NET.Data/Internal/Reader/NumericVectorDataReader.cs

Line 244 in a241693

private TResult GetUnmanagedTypeValue<TQuery, TResult>(ulong offset) where TQuery : unmanaged

Also, LibraryImport requires .NET 7 and newer and requires lots of #ifdef to support .Net Core and netstandard at the same time.

[VitaliyMF]

No worries, I can live with the current behavior - my BI tool integrates various ADO.NET providers and some of them require wrappers to have expected behavior. For instance, Oracle DP for .NET has a setting that can be set only at OracleCommand level (like SuppressGetDecimalInvalidCastException property) and the wrapper allows to do that.

Wrapper adds some overhead (it needs to check each value for DateOnly) but in terms of performance, it doesn't matter. This is more about engineering feeling about the wrapper, it is required only because DuckDB.NET doesn't have any option/switch to control how dates are mapped to .NET types. I understand your argumentation and in general, I fully agree with you. However, it is good only for an ideal world where all .NET applications/libraries are modern and utilize all new .NET things as quickly as they come.

The reality of products that have 5-8-10 years old code base is rather different (even if they are modernized to be NET8). Many dependencies, some of them are 3rd party libs that are not actively maintained but still do the job and simply don't have drop-in replacements. Even adding support of DateOnly into my own product code can be tricky and time-consuming - for instance, all code that uses TypeCode enum and what adds even more complexity, DateOnly doesn't support implicit conversion to DateTime (I mean Convert.ToDateTime(dateOnlyValue).

As a product owner I want to offer DuckDB connector right now, and I want to be sure that all existing codebase works in the same way as with other ADO.NET providers that currently return DateTime for dates. Updating all codebase to support DateOnly is definitely not a top priority task, as this is just an internal thing, it doesn't add anything from end users perspective (but may add new bugs, huh :)

I agree that in the future dates should be returned as DateOnly values, no doubts here. It was introduced into .NET world very late (all questions to MS). I think it makes sense to have a compatibility option that allows to integrate DuckDB.NET into apps that are not brand new. This is a moderate and balanced policy I use both for my products and open source libs I maintain: even if the breaking change is unavoidable in the new release, I'm trying to offer an option to keep backward compatibility that can be used by old users that are not ready for new behavior yet.

p.s. One more thing I noticed is for-based implementation of DuckDBDataReader.GetOrdinal, which is used in column-name API like

public override object this[string name] => GetValue(GetOrdinal(name));

In SQL Client or NpgSql name is resolved via hashmap. This doesn't affect me in any way as my code access values via ordinal / GetValues, however it is normal when code can use column names to access values, and because of for this can be surprisingly not efficient way to access values (especially if the resultset has many columns).

[Giorgi]

You don't need a wrapper for Oracle, you can set this static property: OracleConfiguration.SuppressGetDecimalInvalidCastException

[VitaliyMF]

You don't need a wrapper for Oracle, you can set this static property: OracleConfiguration.SuppressGetDecimalInvalidCastException

Thanks for noticing! It seems they added it recently -- it seems many users complained about it -- because in 3.21.130 (which is only 11 months old) static property OracleConfiguration.SuppressGetDecimalInvalidCastException wasn't available (so wrapping was the only way to set it).

update: this static property was added in new 23.* version, however another option (UseHourOffsetForUnsupportedTimezone) is still can be set only in OracleConnection. Oracle .NET Core client library is a good example of 'pain in the ass' for users when devs don't care about how ADO.NET providers is used by devs in their real-life projects.

[Giorgi]

Thanks for pointing out about GetOrdinal: 1c96434

[VitaliyMF]

Since NpgSql announced that it will return DateOnly in 10 release, it seems you're right - ADO.NET providers should not care about old apps.
This adds more work to keep nuget references up to date, but who cares 8-)

I'm an old .NET developer who had my first project on .NET Framework 1.1. Dates representation via DateTime wasn't ideal for sure, but somehow we were able to live with that for 20 years. Now they decided to add DateOnly - not bad by itself - but for some reason, it wasn't included in netstandard2.1, and now ADO.NET providers lost their backward compatibility - for no really important reason.