freevryheid / duckdb

fortran bindings to duckdb c api
MIT License
9 stars 2 forks source link

chunk functions #13

Closed freevryheid closed 1 year ago

freevryheid commented 1 year ago

Still having issues wrapping: duckdb_result_chunk_count

In contrast to the other result functions this expects the result to be passed by value and not a pointer.

! DUCKDB_API·idx_t·duckdb_result_chunk_count(duckdb_result·result); ! result is value vs DUCKDB_API·bool·duckdb_value_boolean(duckdb_result·*result,·idx_t·col,·idx_t·row); ! result is pointer

Tried passing c_loc(result) from the interface but still getting seg fault.

I'll explore further.

freevryheid commented 1 year ago

There is a note in the API: https://duckdb.org/docs/api/c/api#duckdb_result_get_chunk

stating:

If this function is used, none of the other result functions can be used and vice versa (i.e. this function cannot be mixed with the legacy result functions).

Not sure if this is related.

ludnic commented 1 year ago

I had the same doubt initially, but since that test comes from here: https://github.com/duckdb/duckdb/blob/master/test/api/capi/test_capi_data_chunk.cpp#L26-L28 I'm assuming that this is supposed to pass.

freevryheid commented 1 year ago

We're mixing with legacy functions in our tests. The test_capi_data_chunk.cpp file doesn't.

When I commented out the initializer on the duckdb result as shown below, the program segfaults on a function defined earlier.

│····subroutine·test_data_chunk(error)¬
 445│¬
 446│······type(error_type),·allocatable,·intent(out)·::·error¬
 447│······type(duckdb_database)·::·db¬
 448│······type(duckdb_connection)·::·con¬
 449│······!·type(duckdb_result)·::·result·=·duckdb_result()¬
 450│······type(duckdb_result)·::·result·!=·duckdb_result()¬
 451│······type(duckdb_data_chunk)·::·chunk¬
 452│······type(duckdb_vector)·::·vector¬
 453│······integer·::·col_count,·col_idx¬
 454│······integer·::·chunk_count,·chunk_idx¬
 455│······integer·::·row_count,·row_idx¬

I'll try splitting it out.

freevryheid commented 1 year ago

Splitting it out didn't solve the issue. There may be some C++ initialization of classes in the header we're missing:

https://github.com/duckdb/duckdb/blob/master/test/include/capi_tester.hpp

freevryheid commented 1 year ago

The valgrind output indicates a query error as shown below. I'm guessing it cant write this error to the duckdb_result type as defined in fortran.

The query looks good to me and tests OK in the cli app.

==1653924== Invalid read of size 1
==1653924==    at 0x5D27930: duckdb::BaseQueryResult::HasError() const (in /usr/lib/libduckdb.so)
==1653924==    by 0x5D3E251: duckdb::MaterializedQueryResult::Collection() (in /usr/lib/libduckdb.so)
==1653924==    by 0x5CD0E3B: duckdb_result_chunk_count (in /usr/lib/libduckdb.so)
==1653924==    by 0x112231: __duckdb_MOD_duckdb_result_chunk_count (duckdb.f90:1277)
==1653924==    by 0x110E36: __test_chunk_api_MOD_test_data_chunk (test_chunk_api.f90:64)
==1653924==    by 0x11E29E: __testdrive_MOD_run_unittest (testdrive.F90:400)
==1653924==    by 0x11EE52: __testdrive_MOD_run_testsuite (testdrive.F90:344)
==1653924==    by 0x10AE25: MAIN__ (main.f90:22)
==1653924==    by 0x10AF53: main (main.f90:3)
==1653924==  Address 0x8d48fffffd688611 is not stack'd, malloc'd or (recently) free'd
==1653924==
ludnic commented 1 year ago

while I was trying to reproduce your error above I came across a fix that removes the error (see #14 ) not sure if the result is correct as it seems to show 0 chunks.

lnicotina-rms commented 1 year ago

Of course the new result is empty as expected. But then the problem is not with the chunk count itself but with result. I guess that's the same conclusion you can came to.

freevryheid commented 1 year ago

Latest commit fixes the error outlined above but type(c_ptr) declarations were used for the "deprecated" pointer variables in the duckdb_column and duckdb_result derived types - consequently helper functions will be needed to cast these pointers back to logical and character strings, etc. Let's leave this issue open for the time being until these have been addressed.

It is encouraging to see that the following declaration in the interface holds up for functions expecting results that are not pointers:
type(duckdb_result),·value·::·res

freevryheid commented 1 year ago

Also - what's the correct terminology: deprecated or depreciated. I see the former used in the c header files. I'm guessing they disapprove of the use of depreciated functions :)

ludnic commented 1 year ago

Cool, I see the fix now. that was a tricky one to spot!!

I heard deprecated before .. but either one will not use it. Will open now a separate issue for one the next thing that I'm not sure how to do!!