feat: async and multi-result set APIs WIP

[zeroshade]

A draft for a possible update to the ADBC API to add several driver methods:

• AdbcStatementExecuteQueryAsync
• AdbcStatementNextResultSet
• AdbcStatementNextResultSetAsync
• AdbcConnectionReadPartitionAsync

This would also be a 1.2.0 revision of the ADBC API

Actually implementing these functions would be done in a separate PR once we've reached consensus on this addition to the API

[CurtHagenlocher]

It would be nice if this also worked for "schema-only" evaluation. (I started down this road last year and my work-in-progress was at main...CurtHagenlocher:arrow-adbc:MoreResults.)

[zeroshade]

We have AdbcStatementExecuteSchema for schema-only evaluation, so you're suggesting a NextResultSet function for that one too?

[lidavidm]

Do we want async variants of things like GetObjects, Prepare, etc.?

[CurtHagenlocher]

From a tactical perspective, should 1.2-related work all happen on a separate branch (at least until we feel the proposed changes have been proven out)?

On an unrelated note, are there Arrow interop tests yet for the device APIs? I seem to recall that there weren't any a year ago.

[lidavidm]

From a tactical perspective, should 1.2-related work all happen on a separate branch (at least until we feel the proposed changes have been proven out)?

Yeah, I think that'll have to be what happens due to the header changes.

[lidavidm]

I think we need to comment on the lifetime of serialized_partition too - does it need to live until the call returns or until the callback finishes?

[lidavidm]

When would rows_affected be populated? Maybe it should be included in schema metadata or something instead...

[lidavidm]

I guess it could go in ArrowAsyncProducer's additional_metadata. Or, provide an AdbcAsyncProducerGetMetadata(ArrowAsyncProducer*) like how we have an AdbcErrorFromArrayStream. (Speaking of which, we need an equivalent of that for the async stream.)

[zeroshade]

Hmm, that's a good point. Since it's an asynchronous execution it wouldn't be populated before returning. I guess we can define a canonical metadata key to indicate total rows?

[lidavidm]

Not to expand the scope too much, but looking at #1514 I wonder if we should add this parameter to Execute. (It might be cleaner to just add a separate call for it, though.)

[zeroshade]

Do we want async variants of things like GetObjects, Prepare, etc.?

I'm not sure if an async variant of Prepare makes that much sense, but it would make sense to add async variants for anything that currently returns an ArrowArrayStream such as GetObjects etc.

Would an async variant for Prepare be something like AdbcStatementPrepare(struct AdbcStatement*, void (*on_prepared)(struct AdbcStatement*, struct AdbcError*)) where it returns immediately and calls on_prepared when it finishes (with the error being non-nil if there was an error, and being nil if successful)?

[zeroshade]

Added more functions based on the comments and back-and-forth here. The current collection of new functions is:

• ConnectionGetInfoAsync
• ConnectionGetObjectsAsync
• ConnectionGetTableSchemaAsync
• ConnectionGetTableTypesAsync
• ConnectionGetStatisticsAsync
• ConnectionGetStatisticNamesAsync
• ConnectionReadPartitionAsync
• StatementExecuteSchemaAsync
• StatementNextResultSetSchema
• StatementNextResultSetSchemaAsync
• StatementExecutePartitionsAsync
• StatementNextResultSet
• StatementExecuteQueryAsync
• StatementNextResultSetAsync

Not to expand the scope too much, but looking at #1514 I wonder if we should add this parameter to Execute. (It might be cleaner to just add a separate call for it, though.)

personally, I'd prefer a separate call for this and do it in a follow-up PR rather than in this one. But I'm open to it here if there's consensus.

[lidavidm]

Prepare may have to do I/O, hence it should have an async variant.

[paleolimbot]

Awesome! Probably an async wrapper around a sync array stream in nanoarrow would help fill some of these in.

It's a bummer we have to repeat so much documentation but I suppose the existing documentation has mostly stayed the same.

Not to expand the scope too much, but looking at #1514 I wonder if we should add this parameter to Execute. (It might be cleaner to just add a separate call for it, though.)

I would love this (although I think a separate function makes sense). I can write that up as separate PR if that would be helpful.

[zeroshade]

Awesome! Probably an async wrapper around a sync array stream in nanoarrow would help fill some of these in.

Wouldn't it make more sense to just have nanoarrow implement the Async stream handler from the C Data interface?

I can write that up as separate PR if that would be helpful.

It would! Thanks!

[zeroshade]

Prepare may have to do I/O, hence it should have an async variant.

Added StatementPrepareAsync

[paleolimbot]

I can write that up as separate PR if that would be helpful.

It would! Thanks!

#3623

[lidavidm]

Just a shower thought (that can be split into a separate discussion) but I wonder, instead of adding async and non-async versions of APIs that don't return data, we could just add a set of AdbcStatementWait, AdbcConnectionWait, AdbcDatabaseWait that set the callback for whatever operations were just performed (probably this is too complex vs just adding duplicate functions though)

[zeroshade]

instead of adding async and non-async versions of APIs that don't return data, we could just add a set of AdbcStatementWait, AdbcConnectionWait, AdbcDatabaseWait that set the callback for whatever operations were just performed (probably this is too complex vs just adding duplicate functions though)

I like the idea that lets us avoid needing to have async and non-async versions of the functions, but I think this might be too complex. @CurtHagenlocher @paleolimbot thoughts?

[lidavidm]

I don't like overloading the return code. I think it'd be more consistent to return a schema with no release callback.

[zeroshade]

I can update this to do that, then I'll attempt to implement these APIs and see how it goes