feat(c,format,python): implement error details

[lidavidm]

No description provided.

[lidavidm]

@zeroshade @kou @ywc88 this is the alternative to #946 using Kou's idea. It's very clearly unfinished, but it's much more straightforward to implement IMO.

If this seems roughly reasonable, and we prefer this to the other PR, I'll add all the tests/compatibility tests, finish up the implementation here, fix up the generated Go code, and copy over the bits of the other PR that are still useful.

[kou]

+1

[kou]

It seems that we can use details->count instead of 2 * details->count here.

[lidavidm]

ah, good catch - at one point I had encoded both the keys and values in the same array

[lidavidm]

Hmm, the main weakness I see here is there's no way for us to augment an ArrowArrayStream with error details.

We could add something like this?

struct AdbcError* AdbcErrorFromArrayStream(struct ArrowArrayStream* stream);

that only works for an ArrowArrayStream produced by an ADBC driver. Thoughts?

(For this, the driver manager would have to wrap the ArrowArrayStream itself, but that's probably OK.)

[lidavidm]

Minus the question above, I believe this is ready. If we think this is good, I'll merge this, then follow up and delete the error_details stuff/fix up and make the Go driver work, and also pull any improvements that are still applicable from the old PR.

[kou]

Hmm, the main weakness I see here is there's no way for us to augment an ArrowArrayStream with error details.

When do we need this? When the last ArrowArrayStream::get_next() is failed? (ArrowArrayStream::get_last_error() can't return enough error information for the case, right?)

[lidavidm]

@kou yes, that's the case I'm thinking about. (For instance, for Flight SQL, if we get an error in the middle of the stream.) So in that case, AdbcErrorFromArrayStream would let us get the rest of the details.

It would be annoying to implement, but that's OK. The driver manager would wrap all ArrowArrayStream results with its own implementation, and return to the client its own implementation. The wrapper stream will store the original stream and struct AdbcDriver* private_driver in the ArrowArrayStream's private_data. When the caller calls AdbcErrorFromArrayStream, which is implemented by the driver manager, the driver manager can then forward the call to the driver's AdbcErrorFromArrayStream implementation.

[kou]

Thanks but I'm not sure when/how to use AdbcErrorFromArrayStream() by caller yet.
Could you provide a code snippet for caller?

[lidavidm]

struct ArrowArrayStream stream = ...;

while (true) {
  struct ArrowArray batch;
  if (int code = stream.get_next(&stream, &batch); code != 0) {
    struct AdbcError* error = AdbcErrorFromArrayStream(&stream);
    if (error != nullptr) {
      std::cout << error->message << " SQLSTATE: " << error->sqlstate << std::endl;
      // Also use AdbcErrorGetDetailCount, etc.
    } else {
      std::cout << stream.get_last_error(&stream) << std::endl;
    }
  } else if (batch.release == nullptr) {
    break;
  }
  // Process batch
}

[lidavidm]

Since it looks like pitrou/zeroshade are unavailable and I don't want to be blocked, I'm going to merge this for now. If AdbcErrorFromArrayStream seems reasonable, I'll demo it in a follow-up PR.

[kou]

Thanks.
I think that I can understand what you want to do. It seems reasonable.

[lidavidm]

Thanks for the help! I'll follow up with another PR and I hope that'll be the last in the ADBC 1.1.0 drafts...