Giorgi / DuckDB.NET

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

No results are received with any query #31

Closed unconverged closed 2 years ago

unconverged commented 2 years ago

Hi. I'm experimenting with DuckDb through C# and it appears that I'm not able to get results of any query.

I receive no syntax or execution error, but no result is returned. I've tried several queries and all of the queries that I run with ExecuteScalar return 0 as a value and all of the queries that are expected to return multiple columns return zero columns (i.e. db reader does not iterate over rows). The same queries produce the correct result with DuckDb CLI.

The simplest code is:

await using (var conn = new DuckDBConnection("Data Source=:memory:"))
{
    conn.Open();

    var command = conn.CreateCommand();

    command.CommandText = "SELECT 42;";
    var executeScalar = await command.ExecuteScalarAsync();
    Console.WriteLine(executeScalar);
}

We expect this query to return 42, but it returns 0.

My setup:

Giorgi commented 2 years ago

Can you try the same project on Windows or Linux? Or can you check if non-async versions work?

unconverged commented 2 years ago

I've just tested this code on Windows (I do not have a Linux machine at hand), the result is the same (0 instead of 42). Same for the non-async version, also.

Giorgi commented 2 years ago

Can you upload a simple repro app?

unconverged commented 2 years ago

Here it is. I've also included duckdb.dll, just in case.

Duckdbtest.zip

Giorgi commented 2 years ago

Can you try release 0.4.0?

unconverged commented 2 years ago

No luck, unfortunately. Simple queries like SELECT 42 do work fine now, however, something more complex still returns invalid result. I have attached a small app that tries different queries that work wrong.

Based on the result, it appears that it tries to convert a value to an integer instead of retrieving it.

duckdbtest.zip

Giorgi commented 2 years ago

There is already an issue for that: #37

Giorgi commented 2 years ago

@IvaYan The queries work as expected. ExecuteScalar should be used with queries like this: SELECT count(*) from users

To get the data from the table use ExecuteReader and use the returned reader to get query values.

unconverged commented 2 years ago

I disagree, unfortunately. Why do SELECT 42; returns 42, but SELECT 'test'; returns 0 and SELECT sin(10) returns -1 although System.Math.Sin(10) return -0.5440211108893699?

It appears that the current implementation of ExecuteScalar assumes that the result value is always an integer, which is not correct. MSDN says that ExecuteScalar

returns the first column of the first row in the first returned result set. All other columns, rows and result sets are ignored.

Which means (to me) that ExecuteScalar should return the first value regardless of its type and the total number of values in the result set.

unconverged commented 2 years ago

And I was right. The current implementation simply converts the result value to an integer:

public override object ExecuteScalar()
{
    return ExecuteScalarOrNonQuery();
}

private int ExecuteScalarOrNonQuery()
{
    using var unmanagedString = CommandText.ToUnmanagedString();
    var queryResult = new DuckDBResult();
    var result = NativeMethods.Query.DuckDBQuery(connection.NativeConnection, unmanagedString, queryResult);

    if (!result.IsSuccess())
    {
        var errorMessage = NativeMethods.Query.DuckDBResultError(queryResult).ToManagedString(false);

        NativeMethods.Query.DuckDBDestroyResult(queryResult);
        throw new DuckDBException(string.IsNullOrEmpty(errorMessage) ? "DuckDBQuery failed" : errorMessage, result);
    }

    var rowCount = NativeMethods.Query.DuckDBRowCount(queryResult);
    var columnCount = NativeMethods.Query.DuckDBColumnCount(queryResult);
    if (columnCount > 0 && rowCount > 0)
    {
        return NativeMethods.Types.DuckDBValueInt32(queryResult, 0, 0);
    }

    return 0;
}

If we look into the source code of System.Data.Sqlite we will see entirely different logic.

Giorgi commented 2 years ago

@IvaYan Want to send a PR?

unconverged commented 2 years ago

I can. Will you accept it?

unconverged commented 2 years ago

Simply assign this issue to me (I can't do this myself) and I'll take care of it.

Giorgi commented 2 years ago

@IvaYan Thanks for finding and fixing the issue.