Skip to content

This issue was moved to a discussion.

You can continue the conversation there. Go to discussion →

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

DuckDBType.Date representation as DateTime/DateOnly #232

Closed
VitaliyMF opened this issue Nov 21, 2024 · 5 comments
Closed

DuckDBType.Date representation as DateTime/DateOnly #232

VitaliyMF opened this issue Nov 21, 2024 · 5 comments

Comments

@VitaliyMF
Copy link
Contributor

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
Copy link
Owner

Giorgi commented Nov 21, 2024

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

@VitaliyMF
Copy link
Contributor Author

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
Copy link
Owner

Giorgi commented Nov 21, 2024

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
Copy link
Contributor Author

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
Copy link
Owner

Giorgi commented Nov 21, 2024

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:

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.

Repository owner locked and limited conversation to collaborators Nov 22, 2024
@Giorgi Giorgi converted this issue into discussion #233 Nov 22, 2024

This issue was moved to a discussion.

You can continue the conversation there. Go to discussion →

Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants