Giorgi / DuckDB.NET

Bindings and ADO.NET Provider for DuckDB
https://duckdb.net
MIT License
338 stars 61 forks source link

Unrecognised type TimestampNs #168

Closed bouvierr closed 4 months ago

bouvierr commented 4 months ago

I am using DuckDB.NET.Data.Full 0.9.2.

When I call DuckDB.NET.Data.DuckDBDataReader.GetSchemaTable, I get the following exception if any column is of type TIMESTAMP_NS:

System.ArgumentException: Unrecognised type TimestampNs (22) for column TargetDate
   at DuckDB.NET.Data.Internal.Reader.VectorDataReaderBase.GetColumnType()
   at DuckDB.NET.Data.Internal.Reader.VectorDataReaderBase.get_ClrType()
   at DuckDB.NET.Data.DuckDBDataReader.GetFieldType(Int32 ordinal)
   at DuckDB.NET.Data.DuckDBDataReader.GetSchemaTable()

If I change the type to its alias with ALTER TABLE mytable ALTER TargetDate TYPE TIMESTAMP, then GetSchemaTable works fine.

I believe the issue is simply that the code currently supports only the type alias TIMESTAMP and not the official type TIMESTAMP_NS.

Giorgi commented 4 months ago

It's implemented in the Reader Type Overhaul and will be released after DuckDB 0.10 is released.

bouvierr commented 4 months ago

Glad to hear this will be fixed in the next release!

I tried out Reader Type Overhaul.

In TimestampTests.QueryScalarTest, if I replace TIMESTAMP by TIMESTAMP_NS in the Command.CommandText, the test fails because we get a DuckDBTimestamp instead of a DateTime. I was wondering if this is the expected behavior? The Date and Time values are all good though.

Thanks.

Giorgi commented 4 months ago

I made some changes to the returned types in that branch. I will check the behaviour you described.

Giorgi commented 4 months ago

@bouvierr Can you clarify which test you mean?

bouvierr commented 4 months ago

this test: https://github.com/Giorgi/DuckDB.NET/blob/86017db53ff55fbe4c794124e4dd3bfbe74f69cb/DuckDB.NET.Test/Parameters/TimestampTests.cs#L21

Giorgi commented 4 months ago

It should be fixed now.

bouvierr commented 4 months ago

Awesome. Thanks!

Giorgi commented 4 months ago

This is now merged into develop and will be included in the next release.