lidavidm commented on issue #61: URL: https://github.com/apache/arrow-adbc/issues/61#issuecomment-1214419966
> Just to be sure we're on the same page: > > * a "query" is a single SQL string that can return a result set but doesn't have to > > * a "statement" is the result of preparing an SQL string with placeholders, so that parameters can be added > > * we're debating whether the caller should declare up front if a result is expected > > > In R DBI we're using the term "query" to indicate something that returns a result set (with or without parameters), and a "statement" doesn't return a result set but we can query the number of rows affected. That sounds reasonable. (`adbc.h` should define some terminology up front!) Though I'm still leaning towards having a separate `Statement` struct even for things that aren't necessarily prepared statements, just because that makes it easier to add options (as suggested) without potentially breaking ABI. > The more information we can share with the driver up front, the better. Do we really need two methods, though -- would a single `Query(struct AdbcConnection*, const char*, struct ArrowArrayStream*, struct AdbcError*)` with an optional third argument (can be `NULL`) also work? Or perhaps an option argument that indicates if we expect a result set, and return metadata (rows affected etc.) via the arrow stream? A single method sounds reasonable. The `Statement` struct can keep the fine-grained APIs. I'd possibly argue that if you want more metadata, you should resort to the lower level APIs. Maybe we can just add the row count parameter as well, for convenience? ```c Query(struct AdbcConnection*, const char*, struct ArrowArrayStream*, size_t*, struct AdbcError*) ``` If the query returns a result set, the ArrowArrayStream contains the result set and the size_t is the number of rows (if known, else 0). If not, the ArrowArrayStream is not set (or: contains last inserted IDs, if supported and configured) and the size_t contains rows affected. > > What should happen to queries that indicate that no result set is expected but where a result set is available? Is a warning useful or annoying? I think we can just ignore the result set in that case. This also makes it easy to support "Retrieve the last inserted id for inserts into an auto-increment table" that @zeroshade mentioned in #55: the caller can set an option to return the inserted IDs, and they'll come back via the result set. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: issues-unsubscr...@arrow.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
