Giorgi / DuckDB.NET

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

All integers, decimal & guid #58

Closed unconverged closed 2 years ago

unconverged commented 2 years ago

This pull request brings support for all of integral types, decimal (through string) and GUID (through string).

Fix #55 Fix #56

coveralls commented 2 years ago

Pull Request Test Coverage Report for Build 2885165571


Changes Missing Coverage Covered Lines Changed/Added Lines %
DuckDB.NET.Data/DuckDBDataReader.cs 10 20 50.0%
DuckDB.NET.Data/DuckDBParameter.cs 12 24 50.0%
<!-- Total: 33 55 60.0% -->
Files with Coverage Reduction New Missed Lines %
DuckDB.NET.Data/DuckDBDataReader.cs 1 57.41%
<!-- Total: 1 -->
Totals Coverage Status
Change from base Build 2728373780: 2.0%
Covered Lines: 463
Relevant Lines: 597

💛 - Coveralls
coveralls commented 2 years ago

Pull Request Test Coverage Report for Build 2885165571


Changes Missing Coverage Covered Lines Changed/Added Lines %
DuckDB.NET.Data/DuckDBDataReader.cs 10 20 50.0%
DuckDB.NET.Data/DuckDBParameter.cs 12 24 50.0%
<!-- Total: 33 55 60.0% -->
Files with Coverage Reduction New Missed Lines %
DuckDB.NET.Data/DuckDBDataReader.cs 1 57.41%
<!-- Total: 1 -->
Totals Coverage Status
Change from base Build 2728373780: 2.0%
Covered Lines: 463
Relevant Lines: 597

💛 - Coveralls
Giorgi commented 2 years ago

@IvaYan Thanks for the PR. We should probably treat DUCKDB_TYPE_UUID as Guid too.

Giorgi commented 2 years ago

Also, decimal should use duckdb_value_decimal to get value instead of parsing from string.

unconverged commented 2 years ago

Also, decimal should use duckdb_value_decimal to get value instead of parsing from string.

This does not seem possible. DuckDB uses the following structure to represent a decimal value:

typedef struct {
    uint8_t width;
    uint8_t scale;

    duckdb_hugeint value;
} duckdb_decimal;

and duckdb_hugeint is defined as follows:

typedef struct {
    uint64_t lower;
    int64_t upper;
} duckdb_hugeint;

It does not seem possible to reconstruct C#'s decimal given this data. Although we can do something like this:

var d = (duckdb_decimal.value.upper * 2m ^ 64 + duckdb_decimal.value.lower) / 10m ^ duckdb_decimal.scale;

this approach would not work. First of all, we could not apply the operator ^ to decimals (C# does not support this). Secondly, I'm not sure division can safely preserve the position of the float point. And thirdly, I'm not sure that this approach is going to be faster than to simply parse.

Giorgi commented 2 years ago

Ok, let's keep decimal as is.

unconverged commented 2 years ago

Thanks for the PR. We should probably treat DUCKDB_TYPE_UUID as Guid too.

Good idea, working on it. However, there are some strange things. I have a value of UUID type. DuckDB identifies the value as UUID. The docs indicate that UUID is actually a duckdb_hudeint (128 bit integer), but when I try getting this value as a HugeInt, I get 0. When I try getting the same value as a string, I get an empty string. Huh, something wrong, apparently.

Giorgi commented 2 years ago

Try asking at Discord or GitHub discussions.