This is an automated email from the ASF dual-hosted git repository. lidavidm pushed a commit to branch spec-1.1.0 in repository https://gitbox.apache.org/repos/asf/arrow-adbc.git
commit 7f2f169201dbe73bb54b845497c41566241f5767 Author: David Li <[email protected]> AuthorDate: Thu Aug 3 09:58:41 2023 -0400 feat(c,format,python): implement error details (#954) --- .gitattributes | 2 + adbc.h | 119 ++++++++++++++- c/driver/common/utils.c | 154 ++++++++++++++++++- c/driver/common/utils.h | 13 +- c/driver/common/utils_test.cc | 95 ++++++++++++ c/driver/postgresql/CMakeLists.txt | 1 + c/driver/postgresql/connection.cc | 31 ++-- c/driver/postgresql/database.cc | 10 +- c/driver/postgresql/database.h | 7 + c/driver/postgresql/error.cc | 97 ++++++++++++ c/driver/postgresql/error.h | 42 ++++++ c/driver/postgresql/postgresql.cc | 16 +- c/driver/postgresql/postgresql_test.cc | 61 +++++++- c/driver/postgresql/statement.cc | 104 +++++++------ c/driver_manager/adbc_driver_manager.cc | 78 ++++++++++ c/driver_manager/adbc_version_100.h | 7 + .../adbc_version_100_compatibility_test.cc | 54 ++++--- c/symbols.map | 9 ++ .../recipe/postgresql_create_append_table.py | 2 +- go/adbc/drivermgr/adbc.h | 119 ++++++++++++++- go/adbc/drivermgr/adbc_driver_manager.cc | 78 ++++++++++ .../adbc_driver_manager/_lib.pyi | 49 ++++-- .../adbc_driver_manager/_lib.pyx | 164 ++++++++++++++++----- python/adbc_driver_manager/setup.py | 5 + python/adbc_driver_postgresql/tests/test_dbapi.py | 64 +++++++- 25 files changed, 1227 insertions(+), 154 deletions(-) diff --git a/.gitattributes b/.gitattributes index 7f39f6a3..c4d02d4c 100644 --- a/.gitattributes +++ b/.gitattributes @@ -16,6 +16,8 @@ # under the License. c/vendor/* linguist-vendored +go/adbc/drivermgr/adbc.h linguist-vendored +go/adbc/drivermgr/adbc_driver_manager.cc linguist-vendored python/adbc_driver_flightsql/adbc_driver_flightsql/_static_version.py export-subst python/adbc_driver_manager/adbc_driver_manager/_static_version.py export-subst python/adbc_driver_postgresql/adbc_driver_postgresql/_static_version.py export-subst diff --git a/adbc.h b/adbc.h index 9d9fa923..32eaf8b2 100644 --- a/adbc.h +++ b/adbc.h @@ -248,7 +248,25 @@ typedef uint8_t AdbcStatusCode; /// May indicate a database-side error only. #define ADBC_STATUS_UNAUTHORIZED 14 +/// \brief Inform the driver/driver manager that we are using the extended +/// AdbcError struct from ADBC 1.1.0. +/// +/// See the AdbcError documentation for usage. +/// +/// \since ADBC API revision 1.1.0 +/// \addtogroup adbc-1.1.0 +#define ADBC_ERROR_VENDOR_CODE_PRIVATE_DATA INT32_MIN + /// \brief A detailed error message for an operation. +/// +/// The caller must zero-initialize this struct (clarified in ADBC 1.1.0). +/// +/// The structure was extended in ADBC 1.1.0. Drivers and clients using ADBC +/// 1.0.0 will not have the private_data or private_driver fields. Drivers +/// should read/write these fields if and only if vendor_code is equal to +/// ADBC_ERROR_VENDOR_CODE_PRIVATE_DATA. Clients are required to initialize +/// this struct to avoid the possibility of uninitialized values confusing the +/// driver. struct ADBC_EXPORT AdbcError { /// \brief The error message. char* message; @@ -266,8 +284,103 @@ struct ADBC_EXPORT AdbcError { /// 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 Opaque implementation-defined state. + /// + /// This field may not be used unless vendor_code is + /// ADBC_ERROR_VENDOR_CODE_PRIVATE_DATA. If present, this field is NULLPTR + /// iff the error is unintialized/freed. + /// + /// \since ADBC API revision 1.1.0 + /// \addtogroup adbc-1.1.0 + void* private_data; + + /// \brief The associated driver (used by the driver manager to help + /// track state). + /// + /// This field may not be used unless vendor_code is + /// ADBC_ERROR_VENDOR_CODE_PRIVATE_DATA. + /// + /// \since ADBC API revision 1.1.0 + /// \addtogroup adbc-1.1.0 + struct AdbcDriver* private_driver; +}; + +#ifdef __cplusplus +/// \brief A helper to initialize the full AdbcError structure. +/// +/// \since ADBC API revision 1.1.0 +/// \addtogroup adbc-1.1.0 +#define ADBC_ERROR_INIT \ + (AdbcError{nullptr, \ + ADBC_ERROR_VENDOR_CODE_PRIVATE_DATA, \ + {0, 0, 0, 0, 0}, \ + nullptr, \ + nullptr, \ + nullptr}) +#else +/// \brief A helper to initialize the full AdbcError structure. +/// +/// \since ADBC API revision 1.1.0 +/// \addtogroup adbc-1.1.0 +#define ADBC_ERROR_INIT \ + ((struct AdbcError){ \ + NULL, ADBC_ERROR_VENDOR_CODE_PRIVATE_DATA, {0, 0, 0, 0, 0}, NULL, NULL, NULL}) +#endif + +/// \brief The size of the AdbcError structure in ADBC 1.0.0. +/// +/// Drivers written for ADBC 1.1.0 and later should never touch more than this +/// portion of an AdbcDriver struct when vendor_code is not +/// ADBC_ERROR_VENDOR_CODE_PRIVATE_DATA. +/// +/// \since ADBC API revision 1.1.0 +/// \addtogroup adbc-1.1.0 +#define ADBC_ERROR_1_0_0_SIZE (offsetof(struct AdbcError, private_data)) +/// \brief The size of the AdbcError structure in ADBC 1.1.0. +/// +/// Drivers written for ADBC 1.1.0 and later should never touch more than this +/// portion of an AdbcDriver struct when vendor_code is +/// ADBC_ERROR_VENDOR_CODE_PRIVATE_DATA. +/// +/// \since ADBC API revision 1.1.0 +/// \addtogroup adbc-1.1.0 +#define ADBC_ERROR_1_1_0_SIZE (sizeof(struct AdbcError)) + +/// \brief Extra key-value metadata for an error. +/// +/// The fields here are owned by the driver and should not be freed. The +/// fields here are invalidated when the release callback in AdbcError is +/// called. +/// +/// \since ADBC API revision 1.1.0 +/// \addtogroup adbc-1.1.0 +struct ADBC_EXPORT AdbcErrorDetail { + /// \brief The metadata key. + const char* key; + /// \brief The binary metadata value. + const uint8_t* value; + /// \brief The length of the metadata value. + size_t value_length; }; +/// \brief Get the number of metadata values available in an error. +/// +/// \since ADBC API revision 1.1.0 +/// \addtogroup adbc-1.1.0 +ADBC_EXPORT +int AdbcErrorGetDetailCount(struct AdbcError* error); + +/// \brief Get a metadata value in an error by index. +/// +/// If index is invalid, returns an AdbcErrorDetail initialized with NULL/0 +/// fields. +/// +/// \since ADBC API revision 1.1.0 +/// \addtogroup adbc-1.1.0 +ADBC_EXPORT +struct AdbcErrorDetail AdbcErrorGetDetail(struct AdbcError* error, int index); + /// @} /// \defgroup adbc-constants Constants @@ -284,6 +397,7 @@ struct ADBC_EXPORT AdbcError { /// When passed to an AdbcDriverInitFunc(), the driver parameter must /// point to an AdbcDriver. /// +/// \since ADBC API revision 1.1.0 /// \addtogroup adbc-1.1.0 #define ADBC_VERSION_1_1_0 1001000 @@ -894,6 +1008,9 @@ struct ADBC_EXPORT AdbcDriver { /// /// @{ + int (*ErrorGetDetailCount)(struct AdbcError* error); + struct AdbcErrorDetail (*ErrorGetDetail)(struct AdbcError* error, int index); + AdbcStatusCode (*DatabaseGetOption)(struct AdbcDatabase*, const char*, char*, size_t*, struct AdbcError*); AdbcStatusCode (*DatabaseGetOptionBytes)(struct AdbcDatabase*, const char*, uint8_t*, @@ -959,7 +1076,7 @@ struct ADBC_EXPORT AdbcDriver { /// /// \since ADBC API revision 1.1.0 /// \addtogroup adbc-1.1.0 -#define ADBC_DRIVER_1_0_0_SIZE (offsetof(struct AdbcDriver, DatabaseGetOption)) +#define ADBC_DRIVER_1_0_0_SIZE (offsetof(struct AdbcDriver, ErrorGetDetailCount)) /// \brief The size of the AdbcDriver structure in ADBC 1.1.0. /// Drivers written for ADBC 1.1.0 and later should never touch more diff --git a/c/driver/common/utils.c b/c/driver/common/utils.c index eb01bc18..abd41f8c 100644 --- a/c/driver/common/utils.c +++ b/c/driver/common/utils.c @@ -18,14 +18,46 @@ #include "utils.h" #include <errno.h> -#include <stdarg.h> #include <stdio.h> #include <stdlib.h> #include <string.h> #include <assert.h> +#include "adbc.h" + +static size_t kErrorBufferSize = 1024; + +/// For ADBC 1.1.0, the structure held in private_data. +struct AdbcErrorDetails { + char* message; + + // The metadata keys (may be NULL). + char** keys; + // The metadata values (may be NULL). + uint8_t** values; + // The metadata value lengths (may be NULL). + size_t* lengths; + // The number of initialized metadata. + int count; + // The length of the keys/values/lengths arrays above. + int capacity; +}; + +static void ReleaseErrorWithDetails(struct AdbcError* error) { + struct AdbcErrorDetails* details = (struct AdbcErrorDetails*)error->private_data; + free(details->message); + + for (int i = 0; i < details->count; i++) { + free(details->keys[i]); + free(details->values[i]); + } -static size_t kErrorBufferSize = 256; + free(details->keys); + free(details->values); + free(details->lengths); + free(error->private_data); + *error = ADBC_ERROR_INIT; +} static void ReleaseError(struct AdbcError* error) { free(error->message); @@ -34,20 +66,126 @@ static void ReleaseError(struct AdbcError* error) { } void SetError(struct AdbcError* error, const char* format, ...) { + va_list args; + va_start(args, format); + SetErrorVariadic(error, format, args); + va_end(args); +} + +void SetErrorVariadic(struct AdbcError* error, const char* format, va_list args) { if (!error) return; if (error->release) { // TODO: combine the errors if possible error->release(error); } - error->message = malloc(kErrorBufferSize); - if (!error->message) return; - error->release = &ReleaseError; + if (error->vendor_code == ADBC_ERROR_VENDOR_CODE_PRIVATE_DATA) { + error->private_data = malloc(sizeof(struct AdbcErrorDetails)); + if (!error->private_data) return; + + struct AdbcErrorDetails* details = (struct AdbcErrorDetails*)error->private_data; + + details->message = malloc(kErrorBufferSize); + if (!details->message) { + free(details); + return; + } + details->keys = NULL; + details->values = NULL; + details->lengths = NULL; + details->count = 0; + details->capacity = 0; + + error->message = details->message; + error->release = &ReleaseErrorWithDetails; + } else { + error->message = malloc(kErrorBufferSize); + if (!error->message) return; + + error->release = &ReleaseError; + } - va_list args; - va_start(args, format); vsnprintf(error->message, kErrorBufferSize, format, args); - va_end(args); +} + +void AppendErrorDetail(struct AdbcError* error, const char* key, const uint8_t* detail, + size_t detail_length) { + if (error->release != ReleaseErrorWithDetails) return; + + struct AdbcErrorDetails* details = (struct AdbcErrorDetails*)error->private_data; + if (details->count >= details->capacity) { + int new_capacity = (details->capacity == 0) ? 4 : (2 * details->capacity); + char** new_keys = calloc(new_capacity, sizeof(char*)); + if (!new_keys) { + return; + } + + uint8_t** new_values = calloc(new_capacity, sizeof(uint8_t*)); + if (!new_values) { + free(new_keys); + return; + } + + size_t* new_lengths = calloc(new_capacity, sizeof(size_t*)); + if (!new_lengths) { + free(new_keys); + free(new_values); + return; + } + + memcpy(new_keys, details->keys, sizeof(char*) * details->count); + free(details->keys); + details->keys = new_keys; + + memcpy(new_values, details->values, sizeof(uint8_t*) * details->count); + free(details->values); + details->values = new_values; + + memcpy(new_lengths, details->lengths, sizeof(size_t) * details->count); + free(details->lengths); + details->lengths = new_lengths; + + details->capacity = new_capacity; + } + + char* key_data = strdup(key); + if (!key_data) return; + uint8_t* value_data = malloc(detail_length); + if (!value_data) { + free(key_data); + return; + } + memcpy(value_data, detail, detail_length); + + int index = details->count; + details->keys[index] = key_data; + details->values[index] = value_data; + details->lengths[index] = detail_length; + + details->count++; +} + +int CommonErrorGetDetailCount(struct AdbcError* error) { + if (error->release != ReleaseErrorWithDetails) { + return 0; + } + struct AdbcErrorDetails* details = (struct AdbcErrorDetails*)error->private_data; + return details->count; +} + +struct AdbcErrorDetail CommonErrorGetDetail(struct AdbcError* error, int index) { + if (error->release != ReleaseErrorWithDetails) { + return (struct AdbcErrorDetail){NULL, NULL, 0}; + } + struct AdbcErrorDetails* details = (struct AdbcErrorDetails*)error->private_data; + if (index < 0 || index >= details->count) { + return (struct AdbcErrorDetail){NULL, NULL, 0}; + } + return (struct AdbcErrorDetail){ + .key = details->keys[index], + .value = details->values[index], + .value_length = details->lengths[index], + }; } struct SingleBatchArrayStream { diff --git a/c/driver/common/utils.h b/c/driver/common/utils.h index 381c7b05..88185557 100644 --- a/c/driver/common/utils.h +++ b/c/driver/common/utils.h @@ -17,6 +17,7 @@ #pragma once +#include <stdarg.h> #include <stdbool.h> #include <stdint.h> @@ -35,10 +36,20 @@ extern "C" { #define ADBC_CHECK_PRINTF_ATTRIBUTE #endif -/// Set error details using a format string. +/// Set error message using a format string. void SetError(struct AdbcError* error, const char* format, ...) ADBC_CHECK_PRINTF_ATTRIBUTE; +/// Set error message using a format string. +void SetErrorVariadic(struct AdbcError* error, const char* format, va_list args); + +/// Add an error detail. +void AppendErrorDetail(struct AdbcError* error, const char* key, const uint8_t* detail, + size_t detail_length); + +int CommonErrorGetDetailCount(struct AdbcError* error); +struct AdbcErrorDetail CommonErrorGetDetail(struct AdbcError* error, int index); + struct StringBuilder { char* buffer; // Not including null terminator diff --git a/c/driver/common/utils_test.cc b/c/driver/common/utils_test.cc index 6fa7e254..d5c202bf 100644 --- a/c/driver/common/utils_test.cc +++ b/c/driver/common/utils_test.cc @@ -15,6 +15,12 @@ // specific language governing permissions and limitations // under the License. +#include <cstring> +#include <string> +#include <string_view> +#include <vector> + +#include <gmock/gmock.h> #include <gtest/gtest.h> #include "utils.h" @@ -72,3 +78,92 @@ TEST(TestStringBuilder, TestMultipleAppends) { StringBuilderReset(&str); } + +TEST(ErrorDetails, Adbc100) { + struct AdbcError error; + std::memset(&error, 0, ADBC_ERROR_1_1_0_SIZE); + + SetError(&error, "My message"); + + ASSERT_EQ(nullptr, error.private_data); + ASSERT_EQ(nullptr, error.private_driver); + + { + std::string detail = "detail"; + AppendErrorDetail(&error, "key", reinterpret_cast<const uint8_t*>(detail.data()), + detail.size()); + } + + ASSERT_EQ(0, CommonErrorGetDetailCount(&error)); + struct AdbcErrorDetail detail = CommonErrorGetDetail(&error, 0); + ASSERT_EQ(nullptr, detail.key); + ASSERT_EQ(nullptr, detail.value); + ASSERT_EQ(0, detail.value_length); + + error.release(&error); +} + +TEST(ErrorDetails, Adbc110) { + struct AdbcError error = ADBC_ERROR_INIT; + SetError(&error, "My message"); + + ASSERT_NE(nullptr, error.private_data); + ASSERT_EQ(nullptr, error.private_driver); + + { + std::string detail = "detail"; + AppendErrorDetail(&error, "key", reinterpret_cast<const uint8_t*>(detail.data()), + detail.size()); + } + + ASSERT_EQ(1, CommonErrorGetDetailCount(&error)); + struct AdbcErrorDetail detail = CommonErrorGetDetail(&error, 0); + ASSERT_STREQ("key", detail.key); + ASSERT_EQ("detail", std::string_view(reinterpret_cast<const char*>(detail.value), + detail.value_length)); + + detail = CommonErrorGetDetail(&error, -1); + ASSERT_EQ(nullptr, detail.key); + ASSERT_EQ(nullptr, detail.value); + ASSERT_EQ(0, detail.value_length); + + detail = CommonErrorGetDetail(&error, 2); + ASSERT_EQ(nullptr, detail.key); + ASSERT_EQ(nullptr, detail.value); + ASSERT_EQ(0, detail.value_length); + + error.release(&error); + ASSERT_EQ(nullptr, error.private_data); + ASSERT_EQ(nullptr, error.private_driver); +} + +TEST(ErrorDetails, RoundTripValues) { + struct AdbcError error = ADBC_ERROR_INIT; + SetError(&error, "My message"); + + struct Detail { + std::string key; + std::vector<uint8_t> value; + }; + + std::vector<Detail> details = { + {"x-key-1", {0, 1, 2, 3}}, {"x-key-2", {1, 1}}, {"x-key-3", {128, 129, 200, 0, 1}}, + {"x-key-4", {97, 98, 99}}, {"x-key-5", {42}}, + }; + + for (const auto& detail : details) { + AppendErrorDetail(&error, detail.key.c_str(), detail.value.data(), + detail.value.size()); + } + + ASSERT_EQ(details.size(), CommonErrorGetDetailCount(&error)); + for (int i = 0; i < static_cast<int>(details.size()); i++) { + struct AdbcErrorDetail detail = CommonErrorGetDetail(&error, i); + ASSERT_EQ(details[i].key, detail.key); + ASSERT_EQ(details[i].value.size(), detail.value_length); + ASSERT_THAT(std::vector<uint8_t>(detail.value, detail.value + detail.value_length), + ::testing::ElementsAreArray(details[i].value)); + } + + error.release(&error); +} diff --git a/c/driver/postgresql/CMakeLists.txt b/c/driver/postgresql/CMakeLists.txt index d1697941..9cf595a3 100644 --- a/c/driver/postgresql/CMakeLists.txt +++ b/c/driver/postgresql/CMakeLists.txt @@ -29,6 +29,7 @@ endif() add_arrow_lib(adbc_driver_postgresql SOURCES connection.cc + error.cc database.cc postgresql.cc statement.cc diff --git a/c/driver/postgresql/connection.cc b/c/driver/postgresql/connection.cc index 1c1b0507..df612ea8 100644 --- a/c/driver/postgresql/connection.cc +++ b/c/driver/postgresql/connection.cc @@ -32,7 +32,9 @@ #include "common/utils.h" #include "database.h" +#include "error.h" +namespace adbcpq { namespace { static const uint32_t kSupportedInfoCodes[] = { @@ -52,10 +54,10 @@ struct PqRecord { }; // Used by PqResultHelper to provide index-based access to the records within each -// row of a pg_result +// row of a PGresult class PqResultRow { public: - PqResultRow(pg_result* result, int row_num) : result_(result), row_num_(row_num) { + PqResultRow(PGresult* result, int row_num) : result_(result), row_num_(row_num) { ncols_ = PQnfields(result); } @@ -69,7 +71,7 @@ class PqResultRow { } private: - pg_result* result_ = nullptr; + PGresult* result_ = nullptr; int row_num_; int ncols_; }; @@ -95,10 +97,11 @@ class PqResultHelper { PGresult* result = PQprepare(conn_, /*stmtName=*/"", query_.c_str(), param_values_.size(), NULL); if (PQresultStatus(result) != PGRES_COMMAND_OK) { - SetError(error_, "[libpq] Failed to prepare query: %s\nQuery was:%s", - PQerrorMessage(conn_), query_.c_str()); + AdbcStatusCode code = + SetError(error_, result, "[libpq] Failed to prepare query: %s\nQuery was:%s", + PQerrorMessage(conn_), query_.c_str()); PQclear(result); - return ADBC_STATUS_IO; + return code; } PQclear(result); @@ -117,9 +120,10 @@ class PqResultHelper { ExecStatusType status = PQresultStatus(result_); if (status != PGRES_TUPLES_OK && status != PGRES_COMMAND_OK) { - SetError(error_, "[libpq] Failed to execute query '%s': %s", query_.c_str(), - PQerrorMessage(conn_)); - return ADBC_STATUS_IO; + AdbcStatusCode error = + SetError(error_, result_, "[libpq] Failed to execute query '%s': %s", + query_.c_str(), PQerrorMessage(conn_)); + return error; } return ADBC_STATUS_OK; @@ -167,7 +171,7 @@ class PqResultHelper { iterator end() { return iterator(*this, NumRows()); } private: - pg_result* result_ = nullptr; + PGresult* result_ = nullptr; PGconn* conn_; std::string query_; std::vector<std::string> param_values_; @@ -730,8 +734,6 @@ class PqGetObjectsHelper { } // namespace -namespace adbcpq { - AdbcStatusCode PostgresConnection::Cancel(struct AdbcError* error) { // > errbuf must be a char array of size errbufsize (the recommended size is // > 256 bytes). @@ -754,9 +756,10 @@ AdbcStatusCode PostgresConnection::Commit(struct AdbcError* error) { PGresult* result = PQexec(conn_, "COMMIT"); if (PQresultStatus(result) != PGRES_COMMAND_OK) { - SetError(error, "%s%s", "[libpq] Failed to commit: ", PQerrorMessage(conn_)); + AdbcStatusCode code = SetError(error, result, "%s%s", + "[libpq] Failed to commit: ", PQerrorMessage(conn_)); PQclear(result); - return ADBC_STATUS_IO; + return code; } PQclear(result); return ADBC_STATUS_OK; diff --git a/c/driver/postgresql/database.cc b/c/driver/postgresql/database.cc index 2e18b5bf..5de86280 100644 --- a/c/driver/postgresql/database.cc +++ b/c/driver/postgresql/database.cc @@ -125,10 +125,10 @@ AdbcStatusCode PostgresDatabase::Disconnect(PGconn** conn, struct AdbcError* err // Helpers for building the type resolver from queries static inline int32_t InsertPgAttributeResult( - pg_result* result, const std::shared_ptr<PostgresTypeResolver>& resolver); + PGresult* result, const std::shared_ptr<PostgresTypeResolver>& resolver); static inline int32_t InsertPgTypeResult( - pg_result* result, const std::shared_ptr<PostgresTypeResolver>& resolver); + PGresult* result, const std::shared_ptr<PostgresTypeResolver>& resolver); AdbcStatusCode PostgresDatabase::RebuildTypeResolver(struct AdbcError* error) { PGconn* conn = nullptr; @@ -177,7 +177,7 @@ ORDER BY auto resolver = std::make_shared<PostgresTypeResolver>(); // Insert record type definitions (this includes table schemas) - pg_result* result = PQexec(conn, kColumnsQuery.c_str()); + PGresult* result = PQexec(conn, kColumnsQuery.c_str()); ExecStatusType pq_status = PQresultStatus(result); if (pq_status == PGRES_TUPLES_OK) { InsertPgAttributeResult(result, resolver); @@ -222,7 +222,7 @@ ORDER BY } static inline int32_t InsertPgAttributeResult( - pg_result* result, const std::shared_ptr<PostgresTypeResolver>& resolver) { + PGresult* result, const std::shared_ptr<PostgresTypeResolver>& resolver) { int num_rows = PQntuples(result); std::vector<std::pair<std::string, uint32_t>> columns; uint32_t current_type_oid = 0; @@ -254,7 +254,7 @@ static inline int32_t InsertPgAttributeResult( } static inline int32_t InsertPgTypeResult( - pg_result* result, const std::shared_ptr<PostgresTypeResolver>& resolver) { + PGresult* result, const std::shared_ptr<PostgresTypeResolver>& resolver) { int num_rows = PQntuples(result); PostgresTypeResolver::Item item; int32_t n_added = 0; diff --git a/c/driver/postgresql/database.h b/c/driver/postgresql/database.h index 9269b195..6c3da58d 100644 --- a/c/driver/postgresql/database.h +++ b/c/driver/postgresql/database.h @@ -66,3 +66,10 @@ class PostgresDatabase { std::shared_ptr<PostgresTypeResolver> type_resolver_; }; } // namespace adbcpq + +extern "C" { +/// For applications that want to use the driver struct directly, this gives +/// them access to the Init routine. +ADBC_EXPORT +AdbcStatusCode PostgresqlDriverInit(int, void*, struct AdbcError*); +} diff --git a/c/driver/postgresql/error.cc b/c/driver/postgresql/error.cc new file mode 100644 index 00000000..47e04496 --- /dev/null +++ b/c/driver/postgresql/error.cc @@ -0,0 +1,97 @@ +// Licensed to the Apache Software Foundation (ASF) under one +// or more contributor license agreements. See the NOTICE file +// distributed with this work for additional information +// regarding copyright ownership. The ASF licenses this file +// to you under the Apache License, Version 2.0 (the +// "License"); you may not use this file except in compliance +// with the License. You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, +// software distributed under the License is distributed on an +// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +// KIND, either express or implied. See the License for the +// specific language governing permissions and limitations +// under the License. + +#include "error.h" + +#include <postgres_ext.h> +#include <stdarg.h> +#include <cstring> +#include <string> +#include <vector> + +#include <libpq-fe.h> + +#include "common/utils.h" + +namespace adbcpq { + +namespace { +struct DetailField { + int code; + std::string key; +}; + +static const std::vector<DetailField> kDetailFields = { + {PG_DIAG_COLUMN_NAME, "PG_DIAG_COLUMN_NAME"}, + {PG_DIAG_CONTEXT, "PG_DIAG_CONTEXT"}, + {PG_DIAG_CONSTRAINT_NAME, "PG_DIAG_CONSTRAINT_NAME"}, + {PG_DIAG_DATATYPE_NAME, "PG_DIAG_DATATYPE_NAME"}, + {PG_DIAG_INTERNAL_POSITION, "PG_DIAG_INTERNAL_POSITION"}, + {PG_DIAG_INTERNAL_QUERY, "PG_DIAG_INTERNAL_QUERY"}, + {PG_DIAG_MESSAGE_PRIMARY, "PG_DIAG_MESSAGE_PRIMARY"}, + {PG_DIAG_MESSAGE_DETAIL, "PG_DIAG_MESSAGE_DETAIL"}, + {PG_DIAG_MESSAGE_HINT, "PG_DIAG_MESSAGE_HINT"}, + {PG_DIAG_SEVERITY_NONLOCALIZED, "PG_DIAG_SEVERITY_NONLOCALIZED"}, + {PG_DIAG_SQLSTATE, "PG_DIAG_SQLSTATE"}, + {PG_DIAG_STATEMENT_POSITION, "PG_DIAG_STATEMENT_POSITION"}, + {PG_DIAG_SCHEMA_NAME, "PG_DIAG_SCHEMA_NAME"}, + {PG_DIAG_TABLE_NAME, "PG_DIAG_TABLE_NAME"}, +}; +} // namespace + +AdbcStatusCode SetError(struct AdbcError* error, PGresult* result, const char* format, + ...) { + va_list args; + va_start(args, format); + SetErrorVariadic(error, format, args); + va_end(args); + + AdbcStatusCode code = ADBC_STATUS_IO; + + const char* sqlstate = PQresultErrorField(result, PG_DIAG_SQLSTATE); + if (sqlstate) { + // https://www.postgresql.org/docs/current/errcodes-appendix.html + // This can be extended in the future + if (std::strcmp(sqlstate, "57014") == 0) { + code = ADBC_STATUS_CANCELLED; + } else if (std::strncmp(sqlstate, "42", 0) == 0) { + // Class 42 — Syntax Error or Access Rule Violation + code = ADBC_STATUS_INVALID_ARGUMENT; + } + + static_assert(sizeof(error->sqlstate) == 5, ""); + // N.B. strncpy generates warnings when used for this purpose + int i = 0; + for (; sqlstate[i] != '\0' && i < 5; i++) { + error->sqlstate[i] = sqlstate[i]; + } + for (; i < 5; i++) { + error->sqlstate[i] = '\0'; + } + } + + for (const auto& field : kDetailFields) { + const char* value = PQresultErrorField(result, field.code); + if (value) { + AppendErrorDetail(error, field.key.c_str(), reinterpret_cast<const uint8_t*>(value), + std::strlen(value)); + } + } + return code; +} + +} // namespace adbcpq diff --git a/c/driver/postgresql/error.h b/c/driver/postgresql/error.h new file mode 100644 index 00000000..75c52b46 --- /dev/null +++ b/c/driver/postgresql/error.h @@ -0,0 +1,42 @@ +// Licensed to the Apache Software Foundation (ASF) under one +// or more contributor license agreements. See the NOTICE file +// distributed with this work for additional information +// regarding copyright ownership. The ASF licenses this file +// to you under the Apache License, Version 2.0 (the +// "License"); you may not use this file except in compliance +// with the License. You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, +// software distributed under the License is distributed on an +// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +// KIND, either express or implied. See the License for the +// specific language governing permissions and limitations +// under the License. + +// Error handling utilities. + +#pragma once + +#include <adbc.h> +#include <libpq-fe.h> + +namespace adbcpq { + +// The printf checking attribute doesn't work properly on gcc 4.8 +// and results in spurious compiler warnings +#if defined(__clang__) || (defined(__GNUC__) && __GNUC__ >= 5) +#define ADBC_CHECK_PRINTF_ATTRIBUTE(x, y) __attribute__((format(printf, x, y))) +#else +#define ADBC_CHECK_PRINTF_ATTRIBUTE(x, y) +#endif + +/// \brief Set an error based on a PGresult, inferring the proper ADBC status +/// code from the PGresult. +AdbcStatusCode SetError(struct AdbcError* error, PGresult* result, const char* format, + ...) ADBC_CHECK_PRINTF_ATTRIBUTE(3, 4); + +#undef ADBC_CHECK_PRINTF_ATTRIBUTE + +} // namespace adbcpq diff --git a/c/driver/postgresql/postgresql.cc b/c/driver/postgresql/postgresql.cc index 9f6730eb..d88a0b28 100644 --- a/c/driver/postgresql/postgresql.cc +++ b/c/driver/postgresql/postgresql.cc @@ -34,7 +34,7 @@ using adbcpq::PostgresStatement; // --------------------------------------------------------------------- // ADBC interface implementation - as private functions so that these // don't get replaced by the dynamic linker. If we implemented these -// under the Adbc* names, then DriverInit, the linker may resolve +// under the Adbc* names, then in DriverInit, the linker may resolve // functions to the address of the functions provided by the driver // manager instead of our functions. // @@ -47,6 +47,17 @@ using adbcpq::PostgresStatement; // // So in the end some manual effort here was chosen. +// --------------------------------------------------------------------- +// AdbcError + +int AdbcErrorGetDetailCount(struct AdbcError* error) { + return CommonErrorGetDetailCount(error); +} + +struct AdbcErrorDetail AdbcErrorGetDetail(struct AdbcError* error, int index) { + return CommonErrorGetDetail(error, index); +} + // --------------------------------------------------------------------- // AdbcDatabase @@ -799,6 +810,9 @@ AdbcStatusCode PostgresqlDriverInit(int version, void* raw_driver, if (version >= ADBC_VERSION_1_1_0) { std::memset(driver, 0, ADBC_DRIVER_1_1_0_SIZE); + driver->ErrorGetDetailCount = CommonErrorGetDetailCount; + driver->ErrorGetDetail = CommonErrorGetDetail; + driver->DatabaseGetOption = PostgresDatabaseGetOption; driver->DatabaseGetOptionBytes = PostgresDatabaseGetOptionBytes; driver->DatabaseGetOptionDouble = PostgresDatabaseGetOptionDouble; diff --git a/c/driver/postgresql/postgresql_test.cc b/c/driver/postgresql/postgresql_test.cc index a44721a0..07fdfb2e 100644 --- a/c/driver/postgresql/postgresql_test.cc +++ b/c/driver/postgresql/postgresql_test.cc @@ -25,8 +25,9 @@ #include <gtest/gtest-param-test.h> #include <gtest/gtest.h> #include <nanoarrow/nanoarrow.h> -#include "common/utils.h" +#include "common/utils.h" +#include "database.h" #include "validation/adbc_validation.h" #include "validation/adbc_validation_util.h" @@ -140,6 +141,20 @@ class PostgresDatabaseTest : public ::testing::Test, }; ADBCV_TEST_DATABASE(PostgresDatabaseTest) +TEST_F(PostgresDatabaseTest, AdbcDriverBackwardsCompatibility) { + // XXX: sketchy cast + auto* driver = static_cast<struct AdbcDriver*>(malloc(ADBC_DRIVER_1_0_0_SIZE)); + std::memset(driver, 0, ADBC_DRIVER_1_0_0_SIZE); + + ASSERT_THAT(::PostgresqlDriverInit(ADBC_VERSION_1_0_0, driver, &error), + IsOkStatus(&error)); + + ASSERT_THAT(::PostgresqlDriverInit(424242, driver, &error), + IsStatus(ADBC_STATUS_NOT_IMPLEMENTED, &error)); + + free(driver); +} + class PostgresConnectionTest : public ::testing::Test, public adbc_validation::ConnectionTest { public: @@ -537,13 +552,13 @@ TEST_F(PostgresConnectionTest, MetadataGetTableSchemaInjection) { /*db_schema=*/nullptr, "0'::int; DROP TABLE bulk_ingest;--", &schema.value, &error), - IsStatus(ADBC_STATUS_IO, &error)); + IsStatus(ADBC_STATUS_INVALID_ARGUMENT, &error)); ASSERT_THAT( AdbcConnectionGetTableSchema(&connection, /*catalog=*/nullptr, /*db_schema=*/"0'::int; DROP TABLE bulk_ingest;--", "DROP TABLE bulk_ingest;", &schema.value, &error), - IsStatus(ADBC_STATUS_IO, &error)); + IsStatus(ADBC_STATUS_INVALID_ARGUMENT, &error)); ASSERT_THAT(AdbcConnectionGetTableSchema(&connection, /*catalog=*/nullptr, /*db_schema=*/nullptr, "bulk_ingest", @@ -586,11 +601,27 @@ TEST_F(PostgresConnectionTest, MetadataSetCurrentDbSchema) { IsOkStatus(&error)); // Table does not exist in this schema + error.vendor_code = ADBC_ERROR_VENDOR_CODE_PRIVATE_DATA; ASSERT_THAT( AdbcStatementSetSqlQuery(&statement.value, "SELECT * FROM schematable", &error), IsOkStatus(&error)); ASSERT_THAT(AdbcStatementExecuteQuery(&statement.value, nullptr, nullptr, &error), - IsStatus(ADBC_STATUS_IO, &error)); + IsStatus(ADBC_STATUS_INVALID_ARGUMENT, &error)); + // 42P01 = table not found + ASSERT_EQ("42P01", std::string_view(error.sqlstate, 5)); + ASSERT_NE(0, AdbcErrorGetDetailCount(&error)); + bool found = false; + for (int i = 0; i < AdbcErrorGetDetailCount(&error); i++) { + struct AdbcErrorDetail detail = AdbcErrorGetDetail(&error, i); + if (std::strcmp(detail.key, "PG_DIAG_MESSAGE_PRIMARY") == 0) { + found = true; + std::string_view message(reinterpret_cast<const char*>(detail.value), + detail.value_length); + ASSERT_THAT(message, ::testing::HasSubstr("schematable")); + } + } + error.release(&error); + ASSERT_TRUE(found) << "Did not find expected error detail"; ASSERT_THAT( AdbcConnectionSetOption(&connection, ADBC_CONNECTION_OPTION_CURRENT_DB_SCHEMA, @@ -761,6 +792,28 @@ TEST_F(PostgresStatementTest, BatchSizeHint) { } } +// Test that an ADBC 1.0.0-sized error still works +TEST_F(PostgresStatementTest, AdbcErrorBackwardsCompatibility) { + // XXX: sketchy cast + auto* error = static_cast<struct AdbcError*>(malloc(ADBC_ERROR_1_0_0_SIZE)); + std::memset(error, 0, ADBC_ERROR_1_0_0_SIZE); + + ASSERT_THAT(AdbcStatementNew(&connection, &statement, error), IsOkStatus(error)); + ASSERT_THAT( + AdbcStatementSetSqlQuery(&statement, "SELECT * FROM thistabledoesnotexist", error), + IsOkStatus(error)); + adbc_validation::StreamReader reader; + ASSERT_THAT(AdbcStatementExecuteQuery(&statement, &reader.stream.value, + &reader.rows_affected, error), + IsStatus(ADBC_STATUS_INVALID_ARGUMENT, error)); + + ASSERT_EQ("42P01", std::string_view(error->sqlstate, 5)); + ASSERT_EQ(0, AdbcErrorGetDetailCount(error)); + + error->release(error); + free(error); +} + struct TypeTestCase { std::string name; std::string sql_type; diff --git a/c/driver/postgresql/statement.cc b/c/driver/postgresql/statement.cc index 93290202..f95ee20d 100644 --- a/c/driver/postgresql/statement.cc +++ b/c/driver/postgresql/statement.cc @@ -28,6 +28,7 @@ #include <adbc.h> #include <libpq-fe.h> +#include <postgresql/error.h> #include <nanoarrow/nanoarrow.hpp> #include "common/utils.h" @@ -271,20 +272,23 @@ struct BindStream { if (autocommit) { PGresult* begin_result = PQexec(conn, "BEGIN"); if (PQresultStatus(begin_result) != PGRES_COMMAND_OK) { - SetError(error, "[libpq] Failed to begin transaction for timezone data: %s", - PQerrorMessage(conn)); + AdbcStatusCode code = + SetError(error, begin_result, + "[libpq] Failed to begin transaction for timezone data: %s", + PQerrorMessage(conn)); PQclear(begin_result); - return ADBC_STATUS_IO; + return code; } PQclear(begin_result); } PGresult* get_tz_result = PQexec(conn, "SELECT current_setting('TIMEZONE')"); if (PQresultStatus(get_tz_result) != PGRES_TUPLES_OK) { - SetError(error, "[libpq] Could not query current timezone: %s", - PQerrorMessage(conn)); + AdbcStatusCode code = SetError(error, get_tz_result, + "[libpq] Could not query current timezone: %s", + PQerrorMessage(conn)); PQclear(get_tz_result); - return ADBC_STATUS_IO; + return code; } tz_setting = std::string(PQgetvalue(get_tz_result, 0, 0)); @@ -292,10 +296,11 @@ struct BindStream { PGresult* set_utc_result = PQexec(conn, "SET TIME ZONE 'UTC'"); if (PQresultStatus(set_utc_result) != PGRES_COMMAND_OK) { - SetError(error, "[libpq] Failed to set time zone to UTC: %s", - PQerrorMessage(conn)); + AdbcStatusCode code = SetError(error, set_utc_result, + "[libpq] Failed to set time zone to UTC: %s", + PQerrorMessage(conn)); PQclear(set_utc_result); - return ADBC_STATUS_IO; + return code; } PQclear(set_utc_result); break; @@ -305,10 +310,11 @@ struct BindStream { PGresult* result = PQprepare(conn, /*stmtName=*/"", query.c_str(), /*nParams=*/bind_schema->n_children, param_types.data()); if (PQresultStatus(result) != PGRES_COMMAND_OK) { - SetError(error, "[libpq] Failed to prepare query: %s\nQuery was:%s", - PQerrorMessage(conn), query.c_str()); + AdbcStatusCode code = + SetError(error, result, "[libpq] Failed to prepare query: %s\nQuery was:%s", + PQerrorMessage(conn), query.c_str()); PQclear(result); - return ADBC_STATUS_IO; + return code; } PQclear(result); return ADBC_STATUS_OK; @@ -475,10 +481,11 @@ struct BindStream { /*resultFormat=*/0 /*text*/); if (PQresultStatus(result) != PGRES_COMMAND_OK) { - SetError(error, "%s%s", "[libpq] Failed to execute prepared statement: ", - PQerrorMessage(conn)); + AdbcStatusCode code = SetError( + error, result, "%s%s", + "[libpq] Failed to execute prepared statement: ", PQerrorMessage(conn)); PQclear(result); - return ADBC_STATUS_IO; + return code; } PQclear(result); @@ -489,18 +496,21 @@ struct BindStream { std::string reset_query = "SET TIME ZONE '" + tz_setting + "'"; PGresult* reset_tz_result = PQexec(conn, reset_query.c_str()); if (PQresultStatus(reset_tz_result) != PGRES_COMMAND_OK) { - SetError(error, "[libpq] Failed to reset time zone: %s", PQerrorMessage(conn)); + AdbcStatusCode code = + SetError(error, reset_tz_result, "[libpq] Failed to reset time zone: %s", + PQerrorMessage(conn)); PQclear(reset_tz_result); - return ADBC_STATUS_IO; + return code; } PQclear(reset_tz_result); PGresult* commit_result = PQexec(conn, "COMMIT"); if (PQresultStatus(commit_result) != PGRES_COMMAND_OK) { - SetError(error, "[libpq] Failed to commit transaction: %s", - PQerrorMessage(conn)); + AdbcStatusCode code = + SetError(error, commit_result, "[libpq] Failed to commit transaction: %s", + PQerrorMessage(conn)); PQclear(commit_result); - return ADBC_STATUS_IO; + return code; } PQclear(commit_result); } @@ -807,10 +817,11 @@ AdbcStatusCode PostgresStatement::CreateBulkTable( /*paramLengths=*/nullptr, /*paramFormats=*/nullptr, /*resultFormat=*/1 /*(binary)*/); if (PQresultStatus(result) != PGRES_COMMAND_OK) { - SetError(error, "[libpq] Failed to drop table: %s\nQuery was: %s", - PQerrorMessage(connection_->conn()), drop.c_str()); + AdbcStatusCode code = + SetError(error, result, "[libpq] Failed to drop table: %s\nQuery was: %s", + PQerrorMessage(connection_->conn()), drop.c_str()); PQclear(result); - return ADBC_STATUS_IO; + return code; } PQclear(result); break; @@ -877,10 +888,11 @@ AdbcStatusCode PostgresStatement::CreateBulkTable( /*paramLengths=*/nullptr, /*paramFormats=*/nullptr, /*resultFormat=*/1 /*(binary)*/); if (PQresultStatus(result) != PGRES_COMMAND_OK) { - SetError(error, "[libpq] Failed to create table: %s\nQuery was: %s", - PQerrorMessage(connection_->conn()), create.c_str()); + AdbcStatusCode code = + SetError(error, result, "[libpq] Failed to create table: %s\nQuery was: %s", + PQerrorMessage(connection_->conn()), create.c_str()); PQclear(result); - return ADBC_STATUS_IO; + return code; } PQclear(result); return ADBC_STATUS_OK; @@ -976,11 +988,12 @@ AdbcStatusCode PostgresStatement::ExecuteQuery(struct ArrowArrayStream* stream, /*paramTypes=*/nullptr, /*paramValues=*/nullptr, /*paramLengths=*/nullptr, /*paramFormats=*/nullptr, kPgBinaryFormat); if (PQresultStatus(reader_.result_) != PGRES_COPY_OUT) { - SetError(error, - "[libpq] Failed to execute query: could not begin COPY: %s\nQuery was: %s", - PQerrorMessage(connection_->conn()), copy_query.c_str()); + AdbcStatusCode code = SetError( + error, reader_.result_, + "[libpq] Failed to execute query: could not begin COPY: %s\nQuery was: %s", + PQerrorMessage(connection_->conn()), copy_query.c_str()); ClearResult(); - return ADBC_STATUS_IO; + return code; } // Result is read from the connection, not the result, but we won't clear it here } @@ -1049,10 +1062,11 @@ AdbcStatusCode PostgresStatement::ExecuteUpdateQuery(int64_t* rows_affected, /*paramFormats=*/nullptr, /*resultFormat=*/kPgBinaryFormat); ExecStatusType status = PQresultStatus(result); if (status != PGRES_COMMAND_OK && status != PGRES_TUPLES_OK) { - SetError(error, "[libpq] Failed to execute query: %s\nQuery was:%s", - PQerrorMessage(connection_->conn()), query_.c_str()); + AdbcStatusCode code = + SetError(error, result, "[libpq] Failed to execute query: %s\nQuery was:%s", + PQerrorMessage(connection_->conn()), query_.c_str()); PQclear(result); - return ADBC_STATUS_IO; + return code; } if (rows_affected) *rows_affected = PQntuples(reader_.result_); PQclear(result); @@ -1155,6 +1169,7 @@ AdbcStatusCode PostgresStatement::SetOption(const char* key, const char* value, if (std::strcmp(key, ADBC_INGEST_OPTION_TARGET_TABLE) == 0) { query_.clear(); ingest_.target = value; + prepared_ = false; } else if (std::strcmp(key, ADBC_INGEST_OPTION_MODE) == 0) { if (std::strcmp(value, ADBC_INGEST_OPTION_MODE_CREATE) == 0) { ingest_.mode = IngestMode::kCreate; @@ -1168,6 +1183,7 @@ AdbcStatusCode PostgresStatement::SetOption(const char* key, const char* value, SetError(error, "[libpq] Invalid value '%s' for option '%s'", value, key); return ADBC_STATUS_INVALID_ARGUMENT; } + prepared_ = false; } else if (std::strcmp(key, ADBC_POSTGRESQL_OPTION_BATCH_SIZE_HINT_BYTES) == 0) { int64_t int_value = std::atol(value); if (int_value <= 0) { @@ -1215,22 +1231,24 @@ AdbcStatusCode PostgresStatement::SetupReader(struct AdbcError* error) { PGresult* result = PQprepare(connection_->conn(), /*stmtName=*/"", query_.c_str(), /*nParams=*/0, nullptr); if (PQresultStatus(result) != PGRES_COMMAND_OK) { - SetError(error, - "[libpq] Failed to execute query: could not infer schema: failed to " - "prepare query: %s\nQuery was:%s", - PQerrorMessage(connection_->conn()), query_.c_str()); + AdbcStatusCode code = + SetError(error, result, + "[libpq] Failed to execute query: could not infer schema: failed to " + "prepare query: %s\nQuery was:%s", + PQerrorMessage(connection_->conn()), query_.c_str()); PQclear(result); - return ADBC_STATUS_IO; + return code; } PQclear(result); result = PQdescribePrepared(connection_->conn(), /*stmtName=*/""); if (PQresultStatus(result) != PGRES_COMMAND_OK) { - SetError(error, - "[libpq] Failed to execute query: could not infer schema: failed to " - "describe prepared statement: %s\nQuery was:%s", - PQerrorMessage(connection_->conn()), query_.c_str()); + AdbcStatusCode code = + SetError(error, result, + "[libpq] Failed to execute query: could not infer schema: failed to " + "describe prepared statement: %s\nQuery was:%s", + PQerrorMessage(connection_->conn()), query_.c_str()); PQclear(result); - return ADBC_STATUS_IO; + return code; } // Resolve the information from the PGresult into a PostgresType diff --git a/c/driver_manager/adbc_driver_manager.cc b/c/driver_manager/adbc_driver_manager.cc index ea4ddc4c..370b3eea 100644 --- a/c/driver_manager/adbc_driver_manager.cc +++ b/c/driver_manager/adbc_driver_manager.cc @@ -130,6 +130,12 @@ static AdbcStatusCode ReleaseDriver(struct AdbcDriver* driver, struct AdbcError* // Default stubs +int ErrorGetDetailCount(struct AdbcError* error) { return 0; } + +struct AdbcErrorDetail ErrorGetDetail(struct AdbcError* error, int index) { + return {nullptr, nullptr, 0}; +} + AdbcStatusCode DatabaseGetOption(struct AdbcDatabase* database, const char* key, char* value, size_t* length, struct AdbcError* error) { return ADBC_STATUS_NOT_FOUND; @@ -366,6 +372,27 @@ struct TempConnection { // Direct implementations of API methods +int AdbcErrorGetDetailCount(struct AdbcError* error) { + if (error->vendor_code == ADBC_ERROR_VENDOR_CODE_PRIVATE_DATA && error->private_data && + error->private_driver) { + return error->private_driver->ErrorGetDetailCount(error); + } + return 0; +} + +struct AdbcErrorDetail AdbcErrorGetDetail(struct AdbcError* error, int index) { + if (error->vendor_code == ADBC_ERROR_VENDOR_CODE_PRIVATE_DATA && error->private_data && + error->private_driver) { + return error->private_driver->ErrorGetDetail(error, index); + } + return {nullptr, nullptr, 0}; +} + +#define INIT_ERROR(ERROR, SOURCE) \ + if ((ERROR)->vendor_code == ADBC_ERROR_VENDOR_CODE_PRIVATE_DATA) { \ + (ERROR)->private_driver = (SOURCE)->private_driver; \ + } + AdbcStatusCode AdbcDatabaseNew(struct AdbcDatabase* database, struct AdbcError* error) { // Allocate a temporary structure to store options pre-Init database->private_data = new TempDatabase(); @@ -377,6 +404,7 @@ AdbcStatusCode AdbcDatabaseGetOption(struct AdbcDatabase* database, const char* char* value, size_t* length, struct AdbcError* error) { if (database->private_driver) { + INIT_ERROR(error, database); return database->private_driver->DatabaseGetOption(database, key, value, length, error); } @@ -406,6 +434,7 @@ AdbcStatusCode AdbcDatabaseGetOptionBytes(struct AdbcDatabase* database, const c uint8_t* value, size_t* length, struct AdbcError* error) { if (database->private_driver) { + INIT_ERROR(error, database); return database->private_driver->DatabaseGetOptionBytes(database, key, value, length, error); } @@ -427,6 +456,7 @@ AdbcStatusCode AdbcDatabaseGetOptionBytes(struct AdbcDatabase* database, const c AdbcStatusCode AdbcDatabaseGetOptionInt(struct AdbcDatabase* database, const char* key, int64_t* value, struct AdbcError* error) { if (database->private_driver) { + INIT_ERROR(error, database); return database->private_driver->DatabaseGetOptionInt(database, key, value, error); } const auto* args = reinterpret_cast<const TempDatabase*>(database->private_data); @@ -441,6 +471,7 @@ AdbcStatusCode AdbcDatabaseGetOptionInt(struct AdbcDatabase* database, const cha AdbcStatusCode AdbcDatabaseGetOptionDouble(struct AdbcDatabase* database, const char* key, double* value, struct AdbcError* error) { if (database->private_driver) { + INIT_ERROR(error, database); return database->private_driver->DatabaseGetOptionDouble(database, key, value, error); } const auto* args = reinterpret_cast<const TempDatabase*>(database->private_data); @@ -455,6 +486,7 @@ AdbcStatusCode AdbcDatabaseGetOptionDouble(struct AdbcDatabase* database, const AdbcStatusCode AdbcDatabaseSetOption(struct AdbcDatabase* database, const char* key, const char* value, struct AdbcError* error) { if (database->private_driver) { + INIT_ERROR(error, database); return database->private_driver->DatabaseSetOption(database, key, value, error); } @@ -473,6 +505,7 @@ AdbcStatusCode AdbcDatabaseSetOptionBytes(struct AdbcDatabase* database, const c const uint8_t* value, size_t length, struct AdbcError* error) { if (database->private_driver) { + INIT_ERROR(error, database); return database->private_driver->DatabaseSetOptionBytes(database, key, value, length, error); } @@ -485,6 +518,7 @@ AdbcStatusCode AdbcDatabaseSetOptionBytes(struct AdbcDatabase* database, const c AdbcStatusCode AdbcDatabaseSetOptionInt(struct AdbcDatabase* database, const char* key, int64_t value, struct AdbcError* error) { if (database->private_driver) { + INIT_ERROR(error, database); return database->private_driver->DatabaseSetOptionInt(database, key, value, error); } @@ -496,6 +530,7 @@ AdbcStatusCode AdbcDatabaseSetOptionInt(struct AdbcDatabase* database, const cha AdbcStatusCode AdbcDatabaseSetOptionDouble(struct AdbcDatabase* database, const char* key, double value, struct AdbcError* error) { if (database->private_driver) { + INIT_ERROR(error, database); return database->private_driver->DatabaseSetOptionDouble(database, key, value, error); } @@ -566,6 +601,7 @@ AdbcStatusCode AdbcDatabaseInit(struct AdbcDatabase* database, struct AdbcError* auto double_options = std::move(args->double_options); delete args; + INIT_ERROR(error, database); for (const auto& option : options) { status = database->private_driver->DatabaseSetOption(database, option.first.c_str(), option.second.c_str(), error); @@ -616,6 +652,7 @@ AdbcStatusCode AdbcDatabaseRelease(struct AdbcDatabase* database, } return ADBC_STATUS_INVALID_STATE; } + INIT_ERROR(error, database); auto status = database->private_driver->DatabaseRelease(database, error); if (database->private_driver->release) { database->private_driver->release(database->private_driver, error); @@ -631,6 +668,7 @@ AdbcStatusCode AdbcConnectionCancel(struct AdbcConnection* connection, if (!connection->private_driver) { return ADBC_STATUS_INVALID_STATE; } + INIT_ERROR(error, connection); return connection->private_driver->ConnectionCancel(connection, error); } @@ -639,6 +677,7 @@ AdbcStatusCode AdbcConnectionCommit(struct AdbcConnection* connection, if (!connection->private_driver) { return ADBC_STATUS_INVALID_STATE; } + INIT_ERROR(error, connection); return connection->private_driver->ConnectionCommit(connection, error); } @@ -649,6 +688,7 @@ AdbcStatusCode AdbcConnectionGetInfo(struct AdbcConnection* connection, if (!connection->private_driver) { return ADBC_STATUS_INVALID_STATE; } + INIT_ERROR(error, connection); return connection->private_driver->ConnectionGetInfo(connection, info_codes, info_codes_length, out, error); } @@ -662,6 +702,7 @@ AdbcStatusCode AdbcConnectionGetObjects(struct AdbcConnection* connection, int d if (!connection->private_driver) { return ADBC_STATUS_INVALID_STATE; } + INIT_ERROR(error, connection); return connection->private_driver->ConnectionGetObjects( connection, depth, catalog, db_schema, table_name, table_types, column_name, stream, error); @@ -687,6 +728,7 @@ AdbcStatusCode AdbcConnectionGetOption(struct AdbcConnection* connection, const *length = it->second.size() + 1; return ADBC_STATUS_OK; } + INIT_ERROR(error, connection); return connection->private_driver->ConnectionGetOption(connection, key, value, length, error); } @@ -711,6 +753,7 @@ AdbcStatusCode AdbcConnectionGetOptionBytes(struct AdbcConnection* connection, *length = it->second.size() + 1; return ADBC_STATUS_OK; } + INIT_ERROR(error, connection); return connection->private_driver->ConnectionGetOptionBytes(connection, key, value, length, error); } @@ -732,6 +775,7 @@ AdbcStatusCode AdbcConnectionGetOptionInt(struct AdbcConnection* connection, *value = it->second; return ADBC_STATUS_OK; } + INIT_ERROR(error, connection); return connection->private_driver->ConnectionGetOptionInt(connection, key, value, error); } @@ -753,6 +797,7 @@ AdbcStatusCode AdbcConnectionGetOptionDouble(struct AdbcConnection* connection, *value = it->second; return ADBC_STATUS_OK; } + INIT_ERROR(error, connection); return connection->private_driver->ConnectionGetOptionDouble(connection, key, value, error); } @@ -765,6 +810,7 @@ AdbcStatusCode AdbcConnectionGetTableSchema(struct AdbcConnection* connection, if (!connection->private_driver) { return ADBC_STATUS_INVALID_STATE; } + INIT_ERROR(error, connection); return connection->private_driver->ConnectionGetTableSchema( connection, catalog, db_schema, table_name, schema, error); } @@ -775,6 +821,7 @@ AdbcStatusCode AdbcConnectionGetTableTypes(struct AdbcConnection* connection, if (!connection->private_driver) { return ADBC_STATUS_INVALID_STATE; } + INIT_ERROR(error, connection); return connection->private_driver->ConnectionGetTableTypes(connection, stream, error); } @@ -824,6 +871,7 @@ AdbcStatusCode AdbcConnectionInit(struct AdbcConnection* connection, connection, option.first.c_str(), option.second, error); if (status != ADBC_STATUS_OK) return status; } + INIT_ERROR(error, connection); return connection->private_driver->ConnectionInit(connection, database, error); } @@ -845,6 +893,7 @@ AdbcStatusCode AdbcConnectionReadPartition(struct AdbcConnection* connection, if (!connection->private_driver) { return ADBC_STATUS_INVALID_STATE; } + INIT_ERROR(error, connection); return connection->private_driver->ConnectionReadPartition( connection, serialized_partition, serialized_length, out, error); } @@ -860,6 +909,7 @@ AdbcStatusCode AdbcConnectionRelease(struct AdbcConnection* connection, } return ADBC_STATUS_INVALID_STATE; } + INIT_ERROR(error, connection); auto status = connection->private_driver->ConnectionRelease(connection, error); connection->private_driver = nullptr; return status; @@ -870,6 +920,7 @@ AdbcStatusCode AdbcConnectionRollback(struct AdbcConnection* connection, if (!connection->private_driver) { return ADBC_STATUS_INVALID_STATE; } + INIT_ERROR(error, connection); return connection->private_driver->ConnectionRollback(connection, error); } @@ -885,6 +936,7 @@ AdbcStatusCode AdbcConnectionSetOption(struct AdbcConnection* connection, const args->options[key] = value; return ADBC_STATUS_OK; } + INIT_ERROR(error, connection); return connection->private_driver->ConnectionSetOption(connection, key, value, error); } @@ -901,6 +953,7 @@ AdbcStatusCode AdbcConnectionSetOptionBytes(struct AdbcConnection* connection, args->bytes_options[key] = std::string(reinterpret_cast<const char*>(value), length); return ADBC_STATUS_OK; } + INIT_ERROR(error, connection); return connection->private_driver->ConnectionSetOptionBytes(connection, key, value, length, error); } @@ -918,6 +971,7 @@ AdbcStatusCode AdbcConnectionSetOptionInt(struct AdbcConnection* connection, args->int_options[key] = value; return ADBC_STATUS_OK; } + INIT_ERROR(error, connection); return connection->private_driver->ConnectionSetOptionInt(connection, key, value, error); } @@ -935,6 +989,7 @@ AdbcStatusCode AdbcConnectionSetOptionDouble(struct AdbcConnection* connection, args->double_options[key] = value; return ADBC_STATUS_OK; } + INIT_ERROR(error, connection); return connection->private_driver->ConnectionSetOptionDouble(connection, key, value, error); } @@ -945,6 +1000,7 @@ AdbcStatusCode AdbcStatementBind(struct AdbcStatement* statement, if (!statement->private_driver) { return ADBC_STATUS_INVALID_STATE; } + INIT_ERROR(error, statement); return statement->private_driver->StatementBind(statement, values, schema, error); } @@ -954,6 +1010,7 @@ AdbcStatusCode AdbcStatementBindStream(struct AdbcStatement* statement, if (!statement->private_driver) { return ADBC_STATUS_INVALID_STATE; } + INIT_ERROR(error, statement); return statement->private_driver->StatementBindStream(statement, stream, error); } @@ -962,6 +1019,7 @@ AdbcStatusCode AdbcStatementCancel(struct AdbcStatement* statement, if (!statement->private_driver) { return ADBC_STATUS_INVALID_STATE; } + INIT_ERROR(error, statement); return statement->private_driver->StatementCancel(statement, error); } @@ -974,6 +1032,7 @@ AdbcStatusCode AdbcStatementExecutePartitions(struct AdbcStatement* statement, if (!statement->private_driver) { return ADBC_STATUS_INVALID_STATE; } + INIT_ERROR(error, statement); return statement->private_driver->StatementExecutePartitions( statement, schema, partitions, rows_affected, error); } @@ -985,6 +1044,7 @@ AdbcStatusCode AdbcStatementExecuteQuery(struct AdbcStatement* statement, if (!statement->private_driver) { return ADBC_STATUS_INVALID_STATE; } + INIT_ERROR(error, statement); return statement->private_driver->StatementExecuteQuery(statement, out, rows_affected, error); } @@ -995,6 +1055,7 @@ AdbcStatusCode AdbcStatementExecuteSchema(struct AdbcStatement* statement, if (!statement->private_driver) { return ADBC_STATUS_INVALID_STATE; } + INIT_ERROR(error, statement); return statement->private_driver->StatementExecuteSchema(statement, schema, error); } @@ -1004,6 +1065,7 @@ AdbcStatusCode AdbcStatementGetOption(struct AdbcStatement* statement, const cha if (!statement->private_driver) { return ADBC_STATUS_INVALID_STATE; } + INIT_ERROR(error, statement); return statement->private_driver->StatementGetOption(statement, key, value, length, error); } @@ -1014,6 +1076,7 @@ AdbcStatusCode AdbcStatementGetOptionBytes(struct AdbcStatement* statement, if (!statement->private_driver) { return ADBC_STATUS_INVALID_STATE; } + INIT_ERROR(error, statement); return statement->private_driver->StatementGetOptionBytes(statement, key, value, length, error); } @@ -1023,6 +1086,7 @@ AdbcStatusCode AdbcStatementGetOptionInt(struct AdbcStatement* statement, const if (!statement->private_driver) { return ADBC_STATUS_INVALID_STATE; } + INIT_ERROR(error, statement); return statement->private_driver->StatementGetOptionInt(statement, key, value, error); } @@ -1032,6 +1096,7 @@ AdbcStatusCode AdbcStatementGetOptionDouble(struct AdbcStatement* statement, if (!statement->private_driver) { return ADBC_STATUS_INVALID_STATE; } + INIT_ERROR(error, statement); return statement->private_driver->StatementGetOptionDouble(statement, key, value, error); } @@ -1042,6 +1107,7 @@ AdbcStatusCode AdbcStatementGetParameterSchema(struct AdbcStatement* statement, if (!statement->private_driver) { return ADBC_STATUS_INVALID_STATE; } + INIT_ERROR(error, statement); return statement->private_driver->StatementGetParameterSchema(statement, schema, error); } @@ -1051,6 +1117,7 @@ AdbcStatusCode AdbcStatementNew(struct AdbcConnection* connection, if (!connection->private_driver) { return ADBC_STATUS_INVALID_STATE; } + INIT_ERROR(error, connection); auto status = connection->private_driver->StatementNew(connection, statement, error); statement->private_driver = connection->private_driver; return status; @@ -1061,6 +1128,7 @@ AdbcStatusCode AdbcStatementPrepare(struct AdbcStatement* statement, if (!statement->private_driver) { return ADBC_STATUS_INVALID_STATE; } + INIT_ERROR(error, statement); return statement->private_driver->StatementPrepare(statement, error); } @@ -1069,6 +1137,7 @@ AdbcStatusCode AdbcStatementRelease(struct AdbcStatement* statement, if (!statement->private_driver) { return ADBC_STATUS_INVALID_STATE; } + INIT_ERROR(error, statement); auto status = statement->private_driver->StatementRelease(statement, error); statement->private_driver = nullptr; return status; @@ -1079,6 +1148,7 @@ AdbcStatusCode AdbcStatementSetOption(struct AdbcStatement* statement, const cha if (!statement->private_driver) { return ADBC_STATUS_INVALID_STATE; } + INIT_ERROR(error, statement); return statement->private_driver->StatementSetOption(statement, key, value, error); } @@ -1088,6 +1158,7 @@ AdbcStatusCode AdbcStatementSetOptionBytes(struct AdbcStatement* statement, if (!statement->private_driver) { return ADBC_STATUS_INVALID_STATE; } + INIT_ERROR(error, statement); return statement->private_driver->StatementSetOptionBytes(statement, key, value, length, error); } @@ -1097,6 +1168,7 @@ AdbcStatusCode AdbcStatementSetOptionInt(struct AdbcStatement* statement, const if (!statement->private_driver) { return ADBC_STATUS_INVALID_STATE; } + INIT_ERROR(error, statement); return statement->private_driver->StatementSetOptionInt(statement, key, value, error); } @@ -1106,6 +1178,7 @@ AdbcStatusCode AdbcStatementSetOptionDouble(struct AdbcStatement* statement, if (!statement->private_driver) { return ADBC_STATUS_INVALID_STATE; } + INIT_ERROR(error, statement); return statement->private_driver->StatementSetOptionDouble(statement, key, value, error); } @@ -1115,6 +1188,7 @@ AdbcStatusCode AdbcStatementSetSqlQuery(struct AdbcStatement* statement, if (!statement->private_driver) { return ADBC_STATUS_INVALID_STATE; } + INIT_ERROR(error, statement); return statement->private_driver->StatementSetSqlQuery(statement, query, error); } @@ -1124,6 +1198,7 @@ AdbcStatusCode AdbcStatementSetSubstraitPlan(struct AdbcStatement* statement, if (!statement->private_driver) { return ADBC_STATUS_INVALID_STATE; } + INIT_ERROR(error, statement); return statement->private_driver->StatementSetSubstraitPlan(statement, plan, length, error); } @@ -1373,6 +1448,9 @@ AdbcStatusCode AdbcLoadDriverFromInitFunc(AdbcDriverInitFunc init_func, int vers } if (version >= ADBC_VERSION_1_1_0) { auto* driver = reinterpret_cast<struct AdbcDriver*>(raw_driver); + FILL_DEFAULT(driver, ErrorGetDetailCount); + FILL_DEFAULT(driver, ErrorGetDetail); + FILL_DEFAULT(driver, DatabaseGetOption); FILL_DEFAULT(driver, DatabaseGetOptionBytes); FILL_DEFAULT(driver, DatabaseGetOptionDouble); diff --git a/c/driver_manager/adbc_version_100.h b/c/driver_manager/adbc_version_100.h index b4a8b949..b349f86f 100644 --- a/c/driver_manager/adbc_version_100.h +++ b/c/driver_manager/adbc_version_100.h @@ -23,6 +23,13 @@ extern "C" { #endif +struct AdbcErrorVersion100 { + char* message; + int32_t vendor_code; + char sqlstate[5]; + void (*release)(struct AdbcError* error); +}; + struct AdbcDriverVersion100 { void* private_data; void* private_manager; diff --git a/c/driver_manager/adbc_version_100_compatibility_test.cc b/c/driver_manager/adbc_version_100_compatibility_test.cc index 634c126c..27e5f5d9 100644 --- a/c/driver_manager/adbc_version_100_compatibility_test.cc +++ b/c/driver_manager/adbc_version_100_compatibility_test.cc @@ -55,6 +55,9 @@ class AdbcVersion : public ::testing::Test { }; TEST_F(AdbcVersion, StructSize) { + ASSERT_EQ(sizeof(AdbcErrorVersion100), ADBC_ERROR_1_0_0_SIZE); + ASSERT_EQ(sizeof(AdbcError), ADBC_ERROR_1_1_0_SIZE); + ASSERT_EQ(sizeof(AdbcDriverVersion100), ADBC_DRIVER_1_0_0_SIZE); ASSERT_EQ(sizeof(AdbcDriver), ADBC_DRIVER_1_1_0_SIZE); } @@ -74,30 +77,35 @@ TEST_F(AdbcVersion, OldDriverNewManager) { &driver, &error), IsOkStatus(&error)); - ASSERT_NE(driver.DatabaseGetOption, nullptr); - ASSERT_NE(driver.DatabaseGetOptionInt, nullptr); - ASSERT_NE(driver.DatabaseGetOptionDouble, nullptr); - ASSERT_NE(driver.DatabaseSetOptionInt, nullptr); - ASSERT_NE(driver.DatabaseSetOptionDouble, nullptr); - - ASSERT_NE(driver.ConnectionGetOption, nullptr); - ASSERT_NE(driver.ConnectionGetOptionInt, nullptr); - ASSERT_NE(driver.ConnectionGetOptionDouble, nullptr); - ASSERT_NE(driver.ConnectionSetOptionInt, nullptr); - ASSERT_NE(driver.ConnectionSetOptionDouble, nullptr); - - ASSERT_NE(driver.StatementCancel, nullptr); - ASSERT_NE(driver.StatementExecuteSchema, nullptr); - ASSERT_NE(driver.StatementGetOption, nullptr); - ASSERT_NE(driver.StatementGetOptionInt, nullptr); - ASSERT_NE(driver.StatementGetOptionDouble, nullptr); - ASSERT_NE(driver.StatementSetOptionInt, nullptr); - ASSERT_NE(driver.StatementSetOptionDouble, nullptr); + EXPECT_NE(driver.ErrorGetDetailCount, nullptr); + EXPECT_NE(driver.ErrorGetDetail, nullptr); + + EXPECT_NE(driver.DatabaseGetOption, nullptr); + EXPECT_NE(driver.DatabaseGetOptionBytes, nullptr); + EXPECT_NE(driver.DatabaseGetOptionDouble, nullptr); + EXPECT_NE(driver.DatabaseGetOptionInt, nullptr); + EXPECT_NE(driver.DatabaseSetOptionInt, nullptr); + EXPECT_NE(driver.DatabaseSetOptionDouble, nullptr); + + EXPECT_NE(driver.ConnectionCancel, nullptr); + EXPECT_NE(driver.ConnectionGetOption, nullptr); + EXPECT_NE(driver.ConnectionGetOptionBytes, nullptr); + EXPECT_NE(driver.ConnectionGetOptionDouble, nullptr); + EXPECT_NE(driver.ConnectionGetOptionInt, nullptr); + EXPECT_NE(driver.ConnectionSetOptionInt, nullptr); + EXPECT_NE(driver.ConnectionSetOptionDouble, nullptr); + + EXPECT_NE(driver.StatementCancel, nullptr); + EXPECT_NE(driver.StatementExecuteSchema, nullptr); + EXPECT_NE(driver.StatementGetOption, nullptr); + EXPECT_NE(driver.StatementGetOptionBytes, nullptr); + EXPECT_NE(driver.StatementGetOptionDouble, nullptr); + EXPECT_NE(driver.StatementGetOptionInt, nullptr); + EXPECT_NE(driver.StatementSetOptionInt, nullptr); + EXPECT_NE(driver.StatementSetOptionDouble, nullptr); } -// Initialize a version 1.1.0 driver with the version 1.0.0 driver struct. -TEST_F(AdbcVersion, NewDriverOldLayout) { - // TODO: no new drivers yet. -} +// N.B. see postgresql_test.cc for backwards compatibility test of AdbcError +// N.B. see postgresql_test.cc for backwards compatibility test of AdbcDriver } // namespace adbc diff --git a/c/symbols.map b/c/symbols.map index 5e965b35..110b4d1e 100644 --- a/c/symbols.map +++ b/c/symbols.map @@ -20,6 +20,15 @@ # Only expose symbols from the ADBC API Adbc*; + # Expose driver-specific initialization routines + PostgresqlDriverInit; + SqliteDriverInit; + + # Export Postgres API for testing + extern "C++" { + *adbcpq::*; + }; + local: *; }; diff --git a/docs/source/python/recipe/postgresql_create_append_table.py b/docs/source/python/recipe/postgresql_create_append_table.py index 9b0c66f9..a2f6258c 100644 --- a/docs/source/python/recipe/postgresql_create_append_table.py +++ b/docs/source/python/recipe/postgresql_create_append_table.py @@ -63,7 +63,7 @@ with conn.cursor() as cur: with conn.cursor() as cur: try: cur.adbc_ingest("example", data, mode="create") - except conn.OperationalError: + except conn.ProgrammingError: pass else: raise RuntimeError("Should have failed!") diff --git a/go/adbc/drivermgr/adbc.h b/go/adbc/drivermgr/adbc.h index 9d9fa923..32eaf8b2 100644 --- a/go/adbc/drivermgr/adbc.h +++ b/go/adbc/drivermgr/adbc.h @@ -248,7 +248,25 @@ typedef uint8_t AdbcStatusCode; /// May indicate a database-side error only. #define ADBC_STATUS_UNAUTHORIZED 14 +/// \brief Inform the driver/driver manager that we are using the extended +/// AdbcError struct from ADBC 1.1.0. +/// +/// See the AdbcError documentation for usage. +/// +/// \since ADBC API revision 1.1.0 +/// \addtogroup adbc-1.1.0 +#define ADBC_ERROR_VENDOR_CODE_PRIVATE_DATA INT32_MIN + /// \brief A detailed error message for an operation. +/// +/// The caller must zero-initialize this struct (clarified in ADBC 1.1.0). +/// +/// The structure was extended in ADBC 1.1.0. Drivers and clients using ADBC +/// 1.0.0 will not have the private_data or private_driver fields. Drivers +/// should read/write these fields if and only if vendor_code is equal to +/// ADBC_ERROR_VENDOR_CODE_PRIVATE_DATA. Clients are required to initialize +/// this struct to avoid the possibility of uninitialized values confusing the +/// driver. struct ADBC_EXPORT AdbcError { /// \brief The error message. char* message; @@ -266,8 +284,103 @@ struct ADBC_EXPORT AdbcError { /// 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 Opaque implementation-defined state. + /// + /// This field may not be used unless vendor_code is + /// ADBC_ERROR_VENDOR_CODE_PRIVATE_DATA. If present, this field is NULLPTR + /// iff the error is unintialized/freed. + /// + /// \since ADBC API revision 1.1.0 + /// \addtogroup adbc-1.1.0 + void* private_data; + + /// \brief The associated driver (used by the driver manager to help + /// track state). + /// + /// This field may not be used unless vendor_code is + /// ADBC_ERROR_VENDOR_CODE_PRIVATE_DATA. + /// + /// \since ADBC API revision 1.1.0 + /// \addtogroup adbc-1.1.0 + struct AdbcDriver* private_driver; +}; + +#ifdef __cplusplus +/// \brief A helper to initialize the full AdbcError structure. +/// +/// \since ADBC API revision 1.1.0 +/// \addtogroup adbc-1.1.0 +#define ADBC_ERROR_INIT \ + (AdbcError{nullptr, \ + ADBC_ERROR_VENDOR_CODE_PRIVATE_DATA, \ + {0, 0, 0, 0, 0}, \ + nullptr, \ + nullptr, \ + nullptr}) +#else +/// \brief A helper to initialize the full AdbcError structure. +/// +/// \since ADBC API revision 1.1.0 +/// \addtogroup adbc-1.1.0 +#define ADBC_ERROR_INIT \ + ((struct AdbcError){ \ + NULL, ADBC_ERROR_VENDOR_CODE_PRIVATE_DATA, {0, 0, 0, 0, 0}, NULL, NULL, NULL}) +#endif + +/// \brief The size of the AdbcError structure in ADBC 1.0.0. +/// +/// Drivers written for ADBC 1.1.0 and later should never touch more than this +/// portion of an AdbcDriver struct when vendor_code is not +/// ADBC_ERROR_VENDOR_CODE_PRIVATE_DATA. +/// +/// \since ADBC API revision 1.1.0 +/// \addtogroup adbc-1.1.0 +#define ADBC_ERROR_1_0_0_SIZE (offsetof(struct AdbcError, private_data)) +/// \brief The size of the AdbcError structure in ADBC 1.1.0. +/// +/// Drivers written for ADBC 1.1.0 and later should never touch more than this +/// portion of an AdbcDriver struct when vendor_code is +/// ADBC_ERROR_VENDOR_CODE_PRIVATE_DATA. +/// +/// \since ADBC API revision 1.1.0 +/// \addtogroup adbc-1.1.0 +#define ADBC_ERROR_1_1_0_SIZE (sizeof(struct AdbcError)) + +/// \brief Extra key-value metadata for an error. +/// +/// The fields here are owned by the driver and should not be freed. The +/// fields here are invalidated when the release callback in AdbcError is +/// called. +/// +/// \since ADBC API revision 1.1.0 +/// \addtogroup adbc-1.1.0 +struct ADBC_EXPORT AdbcErrorDetail { + /// \brief The metadata key. + const char* key; + /// \brief The binary metadata value. + const uint8_t* value; + /// \brief The length of the metadata value. + size_t value_length; }; +/// \brief Get the number of metadata values available in an error. +/// +/// \since ADBC API revision 1.1.0 +/// \addtogroup adbc-1.1.0 +ADBC_EXPORT +int AdbcErrorGetDetailCount(struct AdbcError* error); + +/// \brief Get a metadata value in an error by index. +/// +/// If index is invalid, returns an AdbcErrorDetail initialized with NULL/0 +/// fields. +/// +/// \since ADBC API revision 1.1.0 +/// \addtogroup adbc-1.1.0 +ADBC_EXPORT +struct AdbcErrorDetail AdbcErrorGetDetail(struct AdbcError* error, int index); + /// @} /// \defgroup adbc-constants Constants @@ -284,6 +397,7 @@ struct ADBC_EXPORT AdbcError { /// When passed to an AdbcDriverInitFunc(), the driver parameter must /// point to an AdbcDriver. /// +/// \since ADBC API revision 1.1.0 /// \addtogroup adbc-1.1.0 #define ADBC_VERSION_1_1_0 1001000 @@ -894,6 +1008,9 @@ struct ADBC_EXPORT AdbcDriver { /// /// @{ + int (*ErrorGetDetailCount)(struct AdbcError* error); + struct AdbcErrorDetail (*ErrorGetDetail)(struct AdbcError* error, int index); + AdbcStatusCode (*DatabaseGetOption)(struct AdbcDatabase*, const char*, char*, size_t*, struct AdbcError*); AdbcStatusCode (*DatabaseGetOptionBytes)(struct AdbcDatabase*, const char*, uint8_t*, @@ -959,7 +1076,7 @@ struct ADBC_EXPORT AdbcDriver { /// /// \since ADBC API revision 1.1.0 /// \addtogroup adbc-1.1.0 -#define ADBC_DRIVER_1_0_0_SIZE (offsetof(struct AdbcDriver, DatabaseGetOption)) +#define ADBC_DRIVER_1_0_0_SIZE (offsetof(struct AdbcDriver, ErrorGetDetailCount)) /// \brief The size of the AdbcDriver structure in ADBC 1.1.0. /// Drivers written for ADBC 1.1.0 and later should never touch more diff --git a/go/adbc/drivermgr/adbc_driver_manager.cc b/go/adbc/drivermgr/adbc_driver_manager.cc index ea4ddc4c..370b3eea 100644 --- a/go/adbc/drivermgr/adbc_driver_manager.cc +++ b/go/adbc/drivermgr/adbc_driver_manager.cc @@ -130,6 +130,12 @@ static AdbcStatusCode ReleaseDriver(struct AdbcDriver* driver, struct AdbcError* // Default stubs +int ErrorGetDetailCount(struct AdbcError* error) { return 0; } + +struct AdbcErrorDetail ErrorGetDetail(struct AdbcError* error, int index) { + return {nullptr, nullptr, 0}; +} + AdbcStatusCode DatabaseGetOption(struct AdbcDatabase* database, const char* key, char* value, size_t* length, struct AdbcError* error) { return ADBC_STATUS_NOT_FOUND; @@ -366,6 +372,27 @@ struct TempConnection { // Direct implementations of API methods +int AdbcErrorGetDetailCount(struct AdbcError* error) { + if (error->vendor_code == ADBC_ERROR_VENDOR_CODE_PRIVATE_DATA && error->private_data && + error->private_driver) { + return error->private_driver->ErrorGetDetailCount(error); + } + return 0; +} + +struct AdbcErrorDetail AdbcErrorGetDetail(struct AdbcError* error, int index) { + if (error->vendor_code == ADBC_ERROR_VENDOR_CODE_PRIVATE_DATA && error->private_data && + error->private_driver) { + return error->private_driver->ErrorGetDetail(error, index); + } + return {nullptr, nullptr, 0}; +} + +#define INIT_ERROR(ERROR, SOURCE) \ + if ((ERROR)->vendor_code == ADBC_ERROR_VENDOR_CODE_PRIVATE_DATA) { \ + (ERROR)->private_driver = (SOURCE)->private_driver; \ + } + AdbcStatusCode AdbcDatabaseNew(struct AdbcDatabase* database, struct AdbcError* error) { // Allocate a temporary structure to store options pre-Init database->private_data = new TempDatabase(); @@ -377,6 +404,7 @@ AdbcStatusCode AdbcDatabaseGetOption(struct AdbcDatabase* database, const char* char* value, size_t* length, struct AdbcError* error) { if (database->private_driver) { + INIT_ERROR(error, database); return database->private_driver->DatabaseGetOption(database, key, value, length, error); } @@ -406,6 +434,7 @@ AdbcStatusCode AdbcDatabaseGetOptionBytes(struct AdbcDatabase* database, const c uint8_t* value, size_t* length, struct AdbcError* error) { if (database->private_driver) { + INIT_ERROR(error, database); return database->private_driver->DatabaseGetOptionBytes(database, key, value, length, error); } @@ -427,6 +456,7 @@ AdbcStatusCode AdbcDatabaseGetOptionBytes(struct AdbcDatabase* database, const c AdbcStatusCode AdbcDatabaseGetOptionInt(struct AdbcDatabase* database, const char* key, int64_t* value, struct AdbcError* error) { if (database->private_driver) { + INIT_ERROR(error, database); return database->private_driver->DatabaseGetOptionInt(database, key, value, error); } const auto* args = reinterpret_cast<const TempDatabase*>(database->private_data); @@ -441,6 +471,7 @@ AdbcStatusCode AdbcDatabaseGetOptionInt(struct AdbcDatabase* database, const cha AdbcStatusCode AdbcDatabaseGetOptionDouble(struct AdbcDatabase* database, const char* key, double* value, struct AdbcError* error) { if (database->private_driver) { + INIT_ERROR(error, database); return database->private_driver->DatabaseGetOptionDouble(database, key, value, error); } const auto* args = reinterpret_cast<const TempDatabase*>(database->private_data); @@ -455,6 +486,7 @@ AdbcStatusCode AdbcDatabaseGetOptionDouble(struct AdbcDatabase* database, const AdbcStatusCode AdbcDatabaseSetOption(struct AdbcDatabase* database, const char* key, const char* value, struct AdbcError* error) { if (database->private_driver) { + INIT_ERROR(error, database); return database->private_driver->DatabaseSetOption(database, key, value, error); } @@ -473,6 +505,7 @@ AdbcStatusCode AdbcDatabaseSetOptionBytes(struct AdbcDatabase* database, const c const uint8_t* value, size_t length, struct AdbcError* error) { if (database->private_driver) { + INIT_ERROR(error, database); return database->private_driver->DatabaseSetOptionBytes(database, key, value, length, error); } @@ -485,6 +518,7 @@ AdbcStatusCode AdbcDatabaseSetOptionBytes(struct AdbcDatabase* database, const c AdbcStatusCode AdbcDatabaseSetOptionInt(struct AdbcDatabase* database, const char* key, int64_t value, struct AdbcError* error) { if (database->private_driver) { + INIT_ERROR(error, database); return database->private_driver->DatabaseSetOptionInt(database, key, value, error); } @@ -496,6 +530,7 @@ AdbcStatusCode AdbcDatabaseSetOptionInt(struct AdbcDatabase* database, const cha AdbcStatusCode AdbcDatabaseSetOptionDouble(struct AdbcDatabase* database, const char* key, double value, struct AdbcError* error) { if (database->private_driver) { + INIT_ERROR(error, database); return database->private_driver->DatabaseSetOptionDouble(database, key, value, error); } @@ -566,6 +601,7 @@ AdbcStatusCode AdbcDatabaseInit(struct AdbcDatabase* database, struct AdbcError* auto double_options = std::move(args->double_options); delete args; + INIT_ERROR(error, database); for (const auto& option : options) { status = database->private_driver->DatabaseSetOption(database, option.first.c_str(), option.second.c_str(), error); @@ -616,6 +652,7 @@ AdbcStatusCode AdbcDatabaseRelease(struct AdbcDatabase* database, } return ADBC_STATUS_INVALID_STATE; } + INIT_ERROR(error, database); auto status = database->private_driver->DatabaseRelease(database, error); if (database->private_driver->release) { database->private_driver->release(database->private_driver, error); @@ -631,6 +668,7 @@ AdbcStatusCode AdbcConnectionCancel(struct AdbcConnection* connection, if (!connection->private_driver) { return ADBC_STATUS_INVALID_STATE; } + INIT_ERROR(error, connection); return connection->private_driver->ConnectionCancel(connection, error); } @@ -639,6 +677,7 @@ AdbcStatusCode AdbcConnectionCommit(struct AdbcConnection* connection, if (!connection->private_driver) { return ADBC_STATUS_INVALID_STATE; } + INIT_ERROR(error, connection); return connection->private_driver->ConnectionCommit(connection, error); } @@ -649,6 +688,7 @@ AdbcStatusCode AdbcConnectionGetInfo(struct AdbcConnection* connection, if (!connection->private_driver) { return ADBC_STATUS_INVALID_STATE; } + INIT_ERROR(error, connection); return connection->private_driver->ConnectionGetInfo(connection, info_codes, info_codes_length, out, error); } @@ -662,6 +702,7 @@ AdbcStatusCode AdbcConnectionGetObjects(struct AdbcConnection* connection, int d if (!connection->private_driver) { return ADBC_STATUS_INVALID_STATE; } + INIT_ERROR(error, connection); return connection->private_driver->ConnectionGetObjects( connection, depth, catalog, db_schema, table_name, table_types, column_name, stream, error); @@ -687,6 +728,7 @@ AdbcStatusCode AdbcConnectionGetOption(struct AdbcConnection* connection, const *length = it->second.size() + 1; return ADBC_STATUS_OK; } + INIT_ERROR(error, connection); return connection->private_driver->ConnectionGetOption(connection, key, value, length, error); } @@ -711,6 +753,7 @@ AdbcStatusCode AdbcConnectionGetOptionBytes(struct AdbcConnection* connection, *length = it->second.size() + 1; return ADBC_STATUS_OK; } + INIT_ERROR(error, connection); return connection->private_driver->ConnectionGetOptionBytes(connection, key, value, length, error); } @@ -732,6 +775,7 @@ AdbcStatusCode AdbcConnectionGetOptionInt(struct AdbcConnection* connection, *value = it->second; return ADBC_STATUS_OK; } + INIT_ERROR(error, connection); return connection->private_driver->ConnectionGetOptionInt(connection, key, value, error); } @@ -753,6 +797,7 @@ AdbcStatusCode AdbcConnectionGetOptionDouble(struct AdbcConnection* connection, *value = it->second; return ADBC_STATUS_OK; } + INIT_ERROR(error, connection); return connection->private_driver->ConnectionGetOptionDouble(connection, key, value, error); } @@ -765,6 +810,7 @@ AdbcStatusCode AdbcConnectionGetTableSchema(struct AdbcConnection* connection, if (!connection->private_driver) { return ADBC_STATUS_INVALID_STATE; } + INIT_ERROR(error, connection); return connection->private_driver->ConnectionGetTableSchema( connection, catalog, db_schema, table_name, schema, error); } @@ -775,6 +821,7 @@ AdbcStatusCode AdbcConnectionGetTableTypes(struct AdbcConnection* connection, if (!connection->private_driver) { return ADBC_STATUS_INVALID_STATE; } + INIT_ERROR(error, connection); return connection->private_driver->ConnectionGetTableTypes(connection, stream, error); } @@ -824,6 +871,7 @@ AdbcStatusCode AdbcConnectionInit(struct AdbcConnection* connection, connection, option.first.c_str(), option.second, error); if (status != ADBC_STATUS_OK) return status; } + INIT_ERROR(error, connection); return connection->private_driver->ConnectionInit(connection, database, error); } @@ -845,6 +893,7 @@ AdbcStatusCode AdbcConnectionReadPartition(struct AdbcConnection* connection, if (!connection->private_driver) { return ADBC_STATUS_INVALID_STATE; } + INIT_ERROR(error, connection); return connection->private_driver->ConnectionReadPartition( connection, serialized_partition, serialized_length, out, error); } @@ -860,6 +909,7 @@ AdbcStatusCode AdbcConnectionRelease(struct AdbcConnection* connection, } return ADBC_STATUS_INVALID_STATE; } + INIT_ERROR(error, connection); auto status = connection->private_driver->ConnectionRelease(connection, error); connection->private_driver = nullptr; return status; @@ -870,6 +920,7 @@ AdbcStatusCode AdbcConnectionRollback(struct AdbcConnection* connection, if (!connection->private_driver) { return ADBC_STATUS_INVALID_STATE; } + INIT_ERROR(error, connection); return connection->private_driver->ConnectionRollback(connection, error); } @@ -885,6 +936,7 @@ AdbcStatusCode AdbcConnectionSetOption(struct AdbcConnection* connection, const args->options[key] = value; return ADBC_STATUS_OK; } + INIT_ERROR(error, connection); return connection->private_driver->ConnectionSetOption(connection, key, value, error); } @@ -901,6 +953,7 @@ AdbcStatusCode AdbcConnectionSetOptionBytes(struct AdbcConnection* connection, args->bytes_options[key] = std::string(reinterpret_cast<const char*>(value), length); return ADBC_STATUS_OK; } + INIT_ERROR(error, connection); return connection->private_driver->ConnectionSetOptionBytes(connection, key, value, length, error); } @@ -918,6 +971,7 @@ AdbcStatusCode AdbcConnectionSetOptionInt(struct AdbcConnection* connection, args->int_options[key] = value; return ADBC_STATUS_OK; } + INIT_ERROR(error, connection); return connection->private_driver->ConnectionSetOptionInt(connection, key, value, error); } @@ -935,6 +989,7 @@ AdbcStatusCode AdbcConnectionSetOptionDouble(struct AdbcConnection* connection, args->double_options[key] = value; return ADBC_STATUS_OK; } + INIT_ERROR(error, connection); return connection->private_driver->ConnectionSetOptionDouble(connection, key, value, error); } @@ -945,6 +1000,7 @@ AdbcStatusCode AdbcStatementBind(struct AdbcStatement* statement, if (!statement->private_driver) { return ADBC_STATUS_INVALID_STATE; } + INIT_ERROR(error, statement); return statement->private_driver->StatementBind(statement, values, schema, error); } @@ -954,6 +1010,7 @@ AdbcStatusCode AdbcStatementBindStream(struct AdbcStatement* statement, if (!statement->private_driver) { return ADBC_STATUS_INVALID_STATE; } + INIT_ERROR(error, statement); return statement->private_driver->StatementBindStream(statement, stream, error); } @@ -962,6 +1019,7 @@ AdbcStatusCode AdbcStatementCancel(struct AdbcStatement* statement, if (!statement->private_driver) { return ADBC_STATUS_INVALID_STATE; } + INIT_ERROR(error, statement); return statement->private_driver->StatementCancel(statement, error); } @@ -974,6 +1032,7 @@ AdbcStatusCode AdbcStatementExecutePartitions(struct AdbcStatement* statement, if (!statement->private_driver) { return ADBC_STATUS_INVALID_STATE; } + INIT_ERROR(error, statement); return statement->private_driver->StatementExecutePartitions( statement, schema, partitions, rows_affected, error); } @@ -985,6 +1044,7 @@ AdbcStatusCode AdbcStatementExecuteQuery(struct AdbcStatement* statement, if (!statement->private_driver) { return ADBC_STATUS_INVALID_STATE; } + INIT_ERROR(error, statement); return statement->private_driver->StatementExecuteQuery(statement, out, rows_affected, error); } @@ -995,6 +1055,7 @@ AdbcStatusCode AdbcStatementExecuteSchema(struct AdbcStatement* statement, if (!statement->private_driver) { return ADBC_STATUS_INVALID_STATE; } + INIT_ERROR(error, statement); return statement->private_driver->StatementExecuteSchema(statement, schema, error); } @@ -1004,6 +1065,7 @@ AdbcStatusCode AdbcStatementGetOption(struct AdbcStatement* statement, const cha if (!statement->private_driver) { return ADBC_STATUS_INVALID_STATE; } + INIT_ERROR(error, statement); return statement->private_driver->StatementGetOption(statement, key, value, length, error); } @@ -1014,6 +1076,7 @@ AdbcStatusCode AdbcStatementGetOptionBytes(struct AdbcStatement* statement, if (!statement->private_driver) { return ADBC_STATUS_INVALID_STATE; } + INIT_ERROR(error, statement); return statement->private_driver->StatementGetOptionBytes(statement, key, value, length, error); } @@ -1023,6 +1086,7 @@ AdbcStatusCode AdbcStatementGetOptionInt(struct AdbcStatement* statement, const if (!statement->private_driver) { return ADBC_STATUS_INVALID_STATE; } + INIT_ERROR(error, statement); return statement->private_driver->StatementGetOptionInt(statement, key, value, error); } @@ -1032,6 +1096,7 @@ AdbcStatusCode AdbcStatementGetOptionDouble(struct AdbcStatement* statement, if (!statement->private_driver) { return ADBC_STATUS_INVALID_STATE; } + INIT_ERROR(error, statement); return statement->private_driver->StatementGetOptionDouble(statement, key, value, error); } @@ -1042,6 +1107,7 @@ AdbcStatusCode AdbcStatementGetParameterSchema(struct AdbcStatement* statement, if (!statement->private_driver) { return ADBC_STATUS_INVALID_STATE; } + INIT_ERROR(error, statement); return statement->private_driver->StatementGetParameterSchema(statement, schema, error); } @@ -1051,6 +1117,7 @@ AdbcStatusCode AdbcStatementNew(struct AdbcConnection* connection, if (!connection->private_driver) { return ADBC_STATUS_INVALID_STATE; } + INIT_ERROR(error, connection); auto status = connection->private_driver->StatementNew(connection, statement, error); statement->private_driver = connection->private_driver; return status; @@ -1061,6 +1128,7 @@ AdbcStatusCode AdbcStatementPrepare(struct AdbcStatement* statement, if (!statement->private_driver) { return ADBC_STATUS_INVALID_STATE; } + INIT_ERROR(error, statement); return statement->private_driver->StatementPrepare(statement, error); } @@ -1069,6 +1137,7 @@ AdbcStatusCode AdbcStatementRelease(struct AdbcStatement* statement, if (!statement->private_driver) { return ADBC_STATUS_INVALID_STATE; } + INIT_ERROR(error, statement); auto status = statement->private_driver->StatementRelease(statement, error); statement->private_driver = nullptr; return status; @@ -1079,6 +1148,7 @@ AdbcStatusCode AdbcStatementSetOption(struct AdbcStatement* statement, const cha if (!statement->private_driver) { return ADBC_STATUS_INVALID_STATE; } + INIT_ERROR(error, statement); return statement->private_driver->StatementSetOption(statement, key, value, error); } @@ -1088,6 +1158,7 @@ AdbcStatusCode AdbcStatementSetOptionBytes(struct AdbcStatement* statement, if (!statement->private_driver) { return ADBC_STATUS_INVALID_STATE; } + INIT_ERROR(error, statement); return statement->private_driver->StatementSetOptionBytes(statement, key, value, length, error); } @@ -1097,6 +1168,7 @@ AdbcStatusCode AdbcStatementSetOptionInt(struct AdbcStatement* statement, const if (!statement->private_driver) { return ADBC_STATUS_INVALID_STATE; } + INIT_ERROR(error, statement); return statement->private_driver->StatementSetOptionInt(statement, key, value, error); } @@ -1106,6 +1178,7 @@ AdbcStatusCode AdbcStatementSetOptionDouble(struct AdbcStatement* statement, if (!statement->private_driver) { return ADBC_STATUS_INVALID_STATE; } + INIT_ERROR(error, statement); return statement->private_driver->StatementSetOptionDouble(statement, key, value, error); } @@ -1115,6 +1188,7 @@ AdbcStatusCode AdbcStatementSetSqlQuery(struct AdbcStatement* statement, if (!statement->private_driver) { return ADBC_STATUS_INVALID_STATE; } + INIT_ERROR(error, statement); return statement->private_driver->StatementSetSqlQuery(statement, query, error); } @@ -1124,6 +1198,7 @@ AdbcStatusCode AdbcStatementSetSubstraitPlan(struct AdbcStatement* statement, if (!statement->private_driver) { return ADBC_STATUS_INVALID_STATE; } + INIT_ERROR(error, statement); return statement->private_driver->StatementSetSubstraitPlan(statement, plan, length, error); } @@ -1373,6 +1448,9 @@ AdbcStatusCode AdbcLoadDriverFromInitFunc(AdbcDriverInitFunc init_func, int vers } if (version >= ADBC_VERSION_1_1_0) { auto* driver = reinterpret_cast<struct AdbcDriver*>(raw_driver); + FILL_DEFAULT(driver, ErrorGetDetailCount); + FILL_DEFAULT(driver, ErrorGetDetail); + FILL_DEFAULT(driver, DatabaseGetOption); FILL_DEFAULT(driver, DatabaseGetOptionBytes); FILL_DEFAULT(driver, DatabaseGetOptionDouble); diff --git a/python/adbc_driver_manager/adbc_driver_manager/_lib.pyi b/python/adbc_driver_manager/adbc_driver_manager/_lib.pyi index 7723df17..28e6be16 100644 --- a/python/adbc_driver_manager/adbc_driver_manager/_lib.pyi +++ b/python/adbc_driver_manager/adbc_driver_manager/_lib.pyi @@ -38,10 +38,16 @@ class AdbcConnection(_AdbcHandle): def get_info( self, info_codes: Optional[List[Union[int, "AdbcInfoCode"]]] = None ) -> "ArrowArrayStreamHandle": ... - def get_option(self, key: str) -> str: ... - def get_option_bytes(self, key: str) -> bytes: ... - def get_option_float(self, key: str) -> float: ... - def get_option_int(self, key: str) -> int: ... + def get_option( + self, + key: Union[bytes, str], + *, + encoding: str = "utf-8", + errors: str = "strict", + ) -> str: ... + def get_option_bytes(self, key: Union[bytes, str]) -> bytes: ... + def get_option_float(self, key: Union[bytes, str]) -> float: ... + def get_option_int(self, key: Union[bytes, str]) -> int: ... def get_objects( self, depth: "GetObjectsDepth", @@ -66,10 +72,16 @@ class AdbcConnection(_AdbcHandle): class AdbcDatabase(_AdbcHandle): def __init__(self, **kwargs: str) -> None: ... def close(self) -> None: ... - def get_option(self, key: str) -> str: ... - def get_option_bytes(self, key: str) -> bytes: ... - def get_option_float(self, key: str) -> float: ... - def get_option_int(self, key: str) -> int: ... + def get_option( + self, + key: Union[bytes, str], + *, + encoding: str = "utf-8", + errors: str = "strict", + ) -> str: ... + def get_option_bytes(self, key: Union[bytes, str]) -> bytes: ... + def get_option_float(self, key: Union[bytes, str]) -> float: ... + def get_option_int(self, key: Union[bytes, str]) -> int: ... def set_options(self, **kwargs: Union[bytes, float, int, str]) -> None: ... class AdbcInfoCode(enum.IntEnum): @@ -90,10 +102,16 @@ class AdbcStatement(_AdbcHandle): def execute_query(self, *args, **kwargs) -> Any: ... def execute_schema(self) -> "ArrowSchemaHandle": ... def execute_update(self, *args, **kwargs) -> Any: ... - def get_option(self, key: str) -> str: ... - def get_option_bytes(self, key: str) -> bytes: ... - def get_option_float(self, key: str) -> float: ... - def get_option_int(self, key: str) -> int: ... + def get_option( + self, + key: Union[bytes, str], + *, + encoding: str = "utf-8", + errors: str = "strict", + ) -> str: ... + def get_option_bytes(self, key: Union[bytes, str]) -> bytes: ... + def get_option_float(self, key: Union[bytes, str]) -> float: ... + def get_option_int(self, key: Union[bytes, str]) -> int: ... def get_parameter_schema(self, *args, **kwargs) -> Any: ... def prepare(self, *args, **kwargs) -> Any: ... def set_options(self, **kwargs: Union[bytes, float, int, str]) -> None: ... @@ -135,6 +153,7 @@ class Error(Exception): status_code: AdbcStatusCode vendor_code: Optional[int] sqlstate: Optional[str] + details: List[Tuple[str, bytes]] def __init__( self, @@ -142,7 +161,8 @@ class Error(Exception): *, status_code: Union[int, AdbcStatusCode], vendor_code: Optional[str] = None, - sqlstate: Optional[str] = None + sqlstate: Optional[str] = None, + details: Optional[List[Tuple[str, bytes]]] = None, ) -> None: ... class GetObjectsDepth(enum.IntEnum): @@ -162,7 +182,8 @@ class NotSupportedError(DatabaseError): message: str, *, vendor_code: Optional[str] = None, - sqlstate: Optional[str] = None + sqlstate: Optional[str] = None, + details: Optional[List[Tuple[str, bytes]]] = None, ) -> None: ... class OperationalError(DatabaseError): ... diff --git a/python/adbc_driver_manager/adbc_driver_manager/_lib.pyx b/python/adbc_driver_manager/adbc_driver_manager/_lib.pyx index a5ccc23b..2acf0083 100644 --- a/python/adbc_driver_manager/adbc_driver_manager/_lib.pyx +++ b/python/adbc_driver_manager/adbc_driver_manager/_lib.pyx @@ -94,6 +94,16 @@ cdef extern from "adbc.h" nogil: char[5] sqlstate CAdbcErrorRelease release + cdef struct CAdbcErrorDetail"AdbcErrorDetail": + char* key + uint8_t* value + size_t value_length + + int AdbcErrorGetDetailCount(CAdbcError* error) + CAdbcErrorDetail AdbcErrorGetDetail(CAdbcError* error, int index) + + cdef int ADBC_ERROR_VENDOR_CODE_PRIVATE_DATA + cdef struct CAdbcDriver"AdbcDriver": pass @@ -326,13 +336,16 @@ class Error(Exception): A vendor-specific status code if present. sqlstate : str, optional The SQLSTATE code if present. + details : list[tuple[str, bytes]], optional + Additional error details, if present. """ - def __init__(self, message, *, status_code, vendor_code=None, sqlstate=None): + def __init__(self, message, *, status_code, vendor_code=None, sqlstate=None, details=None): super().__init__(message) self.status_code = AdbcStatusCode(status_code) self.vendor_code = vendor_code self.sqlstate = sqlstate + self.details = details or [] class InterfaceError(Error): @@ -366,12 +379,13 @@ class ProgrammingError(DatabaseError): class NotSupportedError(DatabaseError): """An operation or some functionality is not supported.""" - def __init__(self, message, *, vendor_code=None, sqlstate=None): + def __init__(self, message, *, vendor_code=None, sqlstate=None, details=None): super().__init__( message, status_code=AdbcStatusCode.NOT_IMPLEMENTED, vendor_code=vendor_code, sqlstate=sqlstate, + details=details, ) @@ -398,6 +412,8 @@ INGEST_OPTION_TARGET_TABLE = ADBC_INGEST_OPTION_TARGET_TABLE.decode("utf-8") cdef void check_error(CAdbcStatusCode status, CAdbcError* error) except *: + cdef CAdbcErrorDetail c_detail + if status == ADBC_STATUS_OK: return @@ -408,14 +424,26 @@ cdef void check_error(CAdbcStatusCode status, CAdbcError* error) except *: if error != NULL: if error.message != NULL: message += ": " - message += error.message.decode("utf-8") - if error.vendor_code: + message += error.message.decode("utf-8", "replace") + if error.vendor_code and error.vendor_code != ADBC_ERROR_VENDOR_CODE_PRIVATE_DATA: vendor_code = error.vendor_code message += f". Vendor code: {vendor_code}" if error.sqlstate[0] != 0: sqlstate = bytes(error.sqlstate[i] for i in range(5)) - sqlstate = sqlstate.decode("ascii") + sqlstate = sqlstate.decode("ascii", "replace") message += f". SQLSTATE: {sqlstate}" + + num_details = AdbcErrorGetDetailCount(error) + details = [] + for index in range(num_details): + c_detail = AdbcErrorGetDetail(error, index) + if c_detail.key == NULL or c_detail.value == NULL: + # Shouldn't happen... + break + details.append( + (c_detail.key, + PyBytes_FromStringAndSize(<const char*> c_detail.value, c_detail.value_length))) + if error.release: error.release(error) @@ -441,13 +469,15 @@ cdef void check_error(CAdbcStatusCode status, CAdbcError* error) except *: ADBC_STATUS_UNAUTHORIZED): klass = ProgrammingError elif status == ADBC_STATUS_NOT_IMPLEMENTED: - raise NotSupportedError(message, vendor_code=vendor_code, sqlstate=sqlstate) - raise klass(message, status_code=status, vendor_code=vendor_code, sqlstate=sqlstate) + raise NotSupportedError(message, vendor_code=vendor_code, sqlstate=sqlstate, details=details) + raise klass(message, status_code=status, vendor_code=vendor_code, sqlstate=sqlstate, details=details) cdef CAdbcError empty_error(): cdef CAdbcError error memset(&error, 0, cython.sizeof(error)) + # We always want the extended error info + error.vendor_code = ADBC_ERROR_VENDOR_CODE_PRIVATE_DATA return error @@ -607,8 +637,8 @@ cdef class AdbcDatabase(_AdbcHandle): elif value is None: raise ValueError(f"value for key '{key}' cannot be None") else: - key = key.encode("utf-8") - value = value.encode("utf-8") + key = _to_bytes(key, "key") + value = _to_bytes(value, "value") c_key = key c_value = value status = AdbcDatabaseSetOption( @@ -632,10 +662,29 @@ cdef class AdbcDatabase(_AdbcHandle): status = AdbcDatabaseRelease(&self.database, &c_error) check_error(status, &c_error) - def get_option(self, key: str) -> str: - """Get the value of a string option.""" + def get_option( + self, + key: str | bytes, + *, + encoding="utf-8", + errors="strict", + ) -> str: + """ + Get the value of a string option. + + Parameters + ---------- + key : str or bytes + The option to get. + encoding : str + The encoding of the option value. This should almost + always be UTF-8. + errors : str + What to do about errors when decoding the option value + (see bytes.decode). + """ cdef CAdbcError c_error = empty_error() - key_bytes = key.encode("utf-8") + key_bytes = _to_bytes(key, "key") cdef char* c_key = key_bytes cdef uint8_t* c_value = NULL cdef size_t c_len = 0 @@ -663,12 +712,12 @@ cdef class AdbcDatabase(_AdbcHandle): # Remove trailing null terminator if c_len > 0: c_len -= 1 - return buf[:c_len].decode("utf-8") + return buf[:c_len].decode(encoding, errors) def get_option_bytes(self, key: str) -> bytes: """Get the value of a binary option.""" cdef CAdbcError c_error = empty_error() - key_bytes = key.encode("utf-8") + key_bytes = _to_bytes(key, "key") cdef char* c_key = key_bytes cdef uint8_t* c_value = NULL cdef size_t c_len = 0 @@ -698,7 +747,7 @@ cdef class AdbcDatabase(_AdbcHandle): def get_option_float(self, key: str) -> float: """Get the value of a floating-point option.""" cdef CAdbcError c_error = empty_error() - key_bytes = key.encode("utf-8") + key_bytes = _to_bytes(key, "key") cdef char* c_key = key_bytes cdef double c_value = 0.0 check_error( @@ -710,7 +759,7 @@ cdef class AdbcDatabase(_AdbcHandle): def get_option_int(self, key: str) -> int: """Get the value of an integer option.""" cdef CAdbcError c_error = empty_error() - key_bytes = key.encode("utf-8") + key_bytes = _to_bytes(key, "key") cdef char* c_key = key_bytes cdef int64_t c_value = 0 check_error( @@ -733,7 +782,7 @@ cdef class AdbcDatabase(_AdbcHandle): cdef char* c_key = NULL cdef char* c_value = NULL for key, value in kwargs.items(): - key = key.encode("utf-8") + key = _to_bytes(key, "option key") c_key = key if value is None: @@ -741,7 +790,7 @@ cdef class AdbcDatabase(_AdbcHandle): status = AdbcDatabaseSetOption( &self.database, c_key, c_value, &c_error) elif isinstance(value, str): - value = value.encode("utf-8") + value = _to_bytes(value, "option value") c_value = value status = AdbcDatabaseSetOption( &self.database, c_key, c_value, &c_error) @@ -909,10 +958,29 @@ cdef class AdbcConnection(_AdbcHandle): return stream - def get_option(self, key: str) -> str: - """Get the value of a string option.""" + def get_option( + self, + key: str | bytes, + *, + encoding="utf-8", + errors="strict", + ) -> str: + """ + Get the value of a string option. + + Parameters + ---------- + key : str or bytes + The option to get. + encoding : str + The encoding of the option value. This should almost + always be UTF-8. + errors : str + What to do about errors when decoding the option value + (see bytes.decode). + """ cdef CAdbcError c_error = empty_error() - key_bytes = key.encode("utf-8") + key_bytes = _to_bytes(key, "key") cdef char* c_key = key_bytes cdef uint8_t* c_value = NULL cdef size_t c_len = 0 @@ -940,12 +1008,12 @@ cdef class AdbcConnection(_AdbcHandle): # Remove trailing null terminator if c_len > 0: c_len -= 1 - return buf[:c_len].decode("utf-8") + return buf[:c_len].decode(encoding, errors) def get_option_bytes(self, key: str) -> bytes: """Get the value of a binary option.""" cdef CAdbcError c_error = empty_error() - key_bytes = key.encode("utf-8") + key_bytes = _to_bytes(key, "key") cdef char* c_key = key_bytes cdef uint8_t* c_value = NULL cdef size_t c_len = 0 @@ -975,7 +1043,7 @@ cdef class AdbcConnection(_AdbcHandle): def get_option_float(self, key: str) -> float: """Get the value of a floating-point option.""" cdef CAdbcError c_error = empty_error() - key_bytes = key.encode("utf-8") + key_bytes = _to_bytes(key, "key") cdef char* c_key = key_bytes cdef double c_value = 0.0 check_error( @@ -987,7 +1055,7 @@ cdef class AdbcConnection(_AdbcHandle): def get_option_int(self, key: str) -> int: """Get the value of an integer option.""" cdef CAdbcError c_error = empty_error() - key_bytes = key.encode("utf-8") + key_bytes = _to_bytes(key, "key") cdef char* c_key = key_bytes cdef int64_t c_value = 0 check_error( @@ -1086,7 +1154,8 @@ cdef class AdbcConnection(_AdbcHandle): check_error(status, &c_error) def set_options(self, **kwargs) -> None: - """Set arbitrary key-value options. + """ + Set arbitrary key-value options. Note, not all drivers support setting options after creation. @@ -1098,7 +1167,7 @@ cdef class AdbcConnection(_AdbcHandle): cdef char* c_key = NULL cdef char* c_value = NULL for key, value in kwargs.items(): - key = key.encode("utf-8") + key = _to_bytes(key, "option key") c_key = key if value is None: @@ -1106,7 +1175,7 @@ cdef class AdbcConnection(_AdbcHandle): status = AdbcConnectionSetOption( &self.connection, c_key, c_value, &c_error) elif isinstance(value, str): - value = value.encode("utf-8") + value = _to_bytes(value, "option value") c_value = value status = AdbcConnectionSetOption( &self.connection, c_key, c_value, &c_error) @@ -1357,10 +1426,29 @@ cdef class AdbcStatement(_AdbcHandle): check_error(status, &c_error) return rows_affected - def get_option(self, key: str) -> str: - """Get the value of a string option.""" + def get_option( + self, + key: str | bytes, + *, + encoding="utf-8", + errors="strict", + ) -> str: + """ + Get the value of a string option. + + Parameters + ---------- + key : str or bytes + The option to get. + encoding : str + The encoding of the option value. This should almost + always be UTF-8. + errors : str + What to do about errors when decoding the option value + (see bytes.decode). + """ cdef CAdbcError c_error = empty_error() - key_bytes = key.encode("utf-8") + key_bytes = _to_bytes(key, "key") cdef char* c_key = key_bytes cdef uint8_t* c_value = NULL cdef size_t c_len = 0 @@ -1388,12 +1476,12 @@ cdef class AdbcStatement(_AdbcHandle): # Remove trailing null terminator if c_len > 0: c_len -= 1 - return buf[:c_len].decode("utf-8") + return buf[:c_len].decode(encoding, errors) def get_option_bytes(self, key: str) -> bytes: """Get the value of a binary option.""" cdef CAdbcError c_error = empty_error() - key_bytes = key.encode("utf-8") + key_bytes = _to_bytes(key, "key") cdef char* c_key = key_bytes cdef uint8_t* c_value = NULL cdef size_t c_len = 0 @@ -1423,7 +1511,7 @@ cdef class AdbcStatement(_AdbcHandle): def get_option_float(self, key: str) -> float: """Get the value of a floating-point option.""" cdef CAdbcError c_error = empty_error() - key_bytes = key.encode("utf-8") + key_bytes = _to_bytes(key, "key") cdef char* c_key = key_bytes cdef double c_value = 0.0 check_error( @@ -1435,7 +1523,7 @@ cdef class AdbcStatement(_AdbcHandle): def get_option_int(self, key: str) -> int: """Get the value of an integer option.""" cdef CAdbcError c_error = empty_error() - key_bytes = key.encode("utf-8") + key_bytes = _to_bytes(key, "key") cdef char* c_key = key_bytes cdef int64_t c_value = 0 check_error( @@ -1487,6 +1575,8 @@ cdef class AdbcStatement(_AdbcHandle): """ Set arbitrary key-value options. + Note, not all drivers support setting options after creation. + See Also -------- adbc_driver_manager.StatementOptions : Standard option names. @@ -1495,7 +1585,7 @@ cdef class AdbcStatement(_AdbcHandle): cdef char* c_key = NULL cdef char* c_value = NULL for key, value in kwargs.items(): - key = key.encode("utf-8") + key = _to_bytes(key, "option key") c_key = key if value is None: @@ -1503,7 +1593,7 @@ cdef class AdbcStatement(_AdbcHandle): status = AdbcStatementSetOption( &self.statement, c_key, c_value, &c_error) elif isinstance(value, str): - value = value.encode("utf-8") + value = _to_bytes(value, "option value") c_value = value status = AdbcStatementSetOption( &self.statement, c_key, c_value, &c_error) diff --git a/python/adbc_driver_manager/setup.py b/python/adbc_driver_manager/setup.py index dde05a08..9584b78b 100644 --- a/python/adbc_driver_manager/setup.py +++ b/python/adbc_driver_manager/setup.py @@ -72,10 +72,15 @@ version, cmdclass = get_version_and_cmdclass("adbc_driver_manager") # ------------------------------------------------------------ # Resolve compiler flags +build_type = os.environ.get("ADBC_BUILD_TYPE", "release") + if sys.platform == "win32": extra_compile_args = ["/std:c++17", "/DADBC_EXPORTING"] else: extra_compile_args = ["-std=c++17"] + if build_type == "debug": + # Useful to step through driver manager code in GDB + extra_compile_args.extend(["-ggdb", "-Og"]) # ------------------------------------------------------------ # Setup diff --git a/python/adbc_driver_postgresql/tests/test_dbapi.py b/python/adbc_driver_postgresql/tests/test_dbapi.py index e3f86a44..ca6908b6 100644 --- a/python/adbc_driver_postgresql/tests/test_dbapi.py +++ b/python/adbc_driver_postgresql/tests/test_dbapi.py @@ -132,8 +132,70 @@ def test_query_trivial(postgres: dbapi.Connection): assert cur.fetchone() == (1,) +def test_query_invalid(postgres: dbapi.Connection) -> None: + with postgres.cursor() as cur: + with pytest.raises( + postgres.ProgrammingError, match="failed to prepare query" + ) as excinfo: + cur.execute("SELECT * FROM tabledoesnotexist") + + assert excinfo.value.sqlstate == "42P01" + assert len(excinfo.value.details) > 0 + + def test_stmt_ingest(postgres: dbapi.Connection) -> None: - pass + table = pyarrow.table( + [ + [1, 2, 3], + ["a", None, "b"], + ], + names=["ints", "strs"], + ) + double_table = pyarrow.table( + [ + [1, 1, 2, 2, 3, 3], + ["a", "a", None, None, "b", "b"], + ], + names=["ints", "strs"], + ) + + with postgres.cursor() as cur: + cur.execute("DROP TABLE IF EXISTS test_ingest") + + with pytest.raises( + postgres.ProgrammingError, match='"test_ingest" does not exist' + ): + cur.adbc_ingest("test_ingest", table, mode="append") + postgres.rollback() + + cur.adbc_ingest("test_ingest", table, mode="replace") + cur.execute("SELECT * FROM test_ingest ORDER BY ints") + assert cur.fetch_arrow_table() == table + + with pytest.raises( + postgres.ProgrammingError, match='"test_ingest" already exists' + ): + cur.adbc_ingest("test_ingest", table, mode="create") + + cur.adbc_ingest("test_ingest", table, mode="create_append") + cur.execute("SELECT * FROM test_ingest ORDER BY ints") + assert cur.fetch_arrow_table() == double_table + + cur.adbc_ingest("test_ingest", table, mode="replace") + cur.execute("SELECT * FROM test_ingest ORDER BY ints") + assert cur.fetch_arrow_table() == table + + cur.execute("DROP TABLE IF EXISTS test_ingest") + + cur.adbc_ingest("test_ingest", table, mode="create_append") + cur.execute("SELECT * FROM test_ingest ORDER BY ints") + assert cur.fetch_arrow_table() == table + + cur.execute("DROP TABLE IF EXISTS test_ingest") + + cur.adbc_ingest("test_ingest", table, mode="create") + cur.execute("SELECT * FROM test_ingest ORDER BY ints") + assert cur.fetch_arrow_table() == table def test_ddl(postgres: dbapi.Connection):
