feat(c/driver/postgresql,python): implement error_details spec

[lidavidm]

Fixes #939.
Fixes #942.

[lidavidm]

I'm not too happy with how error_details works in C++. As you can see here, it's rather messy/stateful, and it's easy to forget to clear the state. However, it seems hard to do it any other way, unless we break ABI for AdbcError.

I thought we could try something like changing

/// \brief A detailed error message for an operation.
struct ADBC_EXPORT AdbcError {
  /// \brief The error message.
  char* message;

  /// \brief A vendor-specific error code, if applicable.
  int32_t vendor_code;

  /// \brief A SQLSTATE error code, if provided, as defined by the
  ///   SQL:2003 standard.  If not set, it should be set to
  ///   "\0\0\0\0\0".
  char sqlstate[5];

  /// \brief Release the contained error.
  ///
  /// Unlike other structures, this is an embedded callback to make it
  /// easier for the driver manager and driver to cooperate.
  void (*release)(struct AdbcError* error);
};

to this:

/// \brief A detailed error message for an operation.
struct ADBC_EXPORT AdbcError {
  /// \brief The error message.
  char* message;

  /// \brief Driver-specific state.
  void* private_data;

  /// \brief A SQLSTATE error code, if provided, as defined by the
  ///   SQL:2003 standard.  If not set, it should be set to
  ///   "\0\0\0\0\0".
  char sqlstate[5];

  /// \brief Release the contained error.
  ///
  /// Unlike other structures, this is an embedded callback to make it
  /// easier for the driver manager and driver to cooperate.
  void (*release)(struct AdbcError* error);
};

const char* AdbcErrorGetDetail(size_t index);
// ...

Having a private_data would allow for future extensibility by adding getter functions.

On a 64-bit platform, AdbcError is already 32 bytes, but sqlstate's offset is 12, not 16 like I had hoped... Or if we're entertaining things like this, maybe we should accept the mistake and break ABI, despite our earlier goals; but then we would have to update all drivers at once and probably couldn't maintain any compatibility at all.

We could perhaps recycle sqlstate, replacing it with void* private_data. Older clients would perhaps see gunk in the 5th byte of sqlstate. Older drivers wouldn't implement the new getters, so newer clients wouldn't be able to call them/would call the no-op stub provided by the driver manager.

Or we can continue as-is, and accept the API wart.

CC @pitrou @zeroshade @ywc88 for opinions here.

[lidavidm]

Oh, and @kou I'm not sure if you've been following this too closely, but if you have ideas here I'd appreciate them!

[kou]

How about adding a new member instead of recycling vendor_code?

struct ADBC_EXPORT AdbcError {
  /// \brief The error message.
  char* message;

  /// \brief A vendor-specific error code, if applicable.
  int32_t vendor_code;

  /// \brief A SQLSTATE error code, if provided, as defined by the
  ///   SQL:2003 standard.  If not set, it should be set to
  ///   "\0\0\0\0\0".
  char sqlstate[5];

  /// \brief Release the contained error.
  ///
  /// Unlike other structures, this is an embedded callback to make it
  /// easier for the driver manager and driver to cooperate.
  void (*release)(struct AdbcError* error);

  /// \brief Driver-specific state.
  void* private_data;
};

We use a special vendor_code value such as #define ADBC_ERROR_VENDOR_CODE_USE_PRIVATE_DATA INT32_MIN to detect whether the AdbcError uses private_data or not. It has a small API (not ABI) incompatibility (users can't use the special vendor_code value) but it'll not be a real problem by choosing a good special value.

[lidavidm]

Hmm, so the caller would have to initialize the struct with the special vendor code to tell the driver/driver manager the size of the struct - effectively the same strategy we're doing with AdbcDriver. It's a little inconvenient as a C/C++ user (I should really go tackle #598!) but would be consistent with what we are already doing. I don't dislike it :)

If there's no objections I'll go sketch out how that looks next week.

[lidavidm]

It's a little unfortunate that we have this amount of API baggage already (I do regret making this 1.0.0 instead of 0.1.0...), but I suppose we can let things prove out for a while longer and then roll up a 2.0.0 (maybe once we have async APIs that we're happy with as well?)

[kou]

It's a little unfortunate that we have this amount of API baggage already

Ah...

We may want to provide an initialization macro for convenient but it may not reduce the migration cost so much...:

#define ARROW_ADBC_ERROR_INIT {NULL, ADBC_ERROR_VENDOR_CODE_USE_PRIVATE_DATA, "\0\0\0\0\0", NULL}

// struct AdbcError error = {0};
struct AdbcError error = ARROW_ADBC_ERROR_INIT;