This is an automated email from the ASF dual-hosted git repository.
lidavidm pushed a commit to branch main
in repository https://gitbox.apache.org/repos/asf/arrow-adbc.git
The following commit(s) were added to refs/heads/main by this push:
new 47e1a3cbd2 feat(c/driver_manager): polyfill numeric options for 1.0
drivers (#4774)
47e1a3cbd2 is described below
commit 47e1a3cbd2f1a0f2be9353473e98c4aec89f5864
Author: Yifan Chen <[email protected]>
AuthorDate: Sun Oct 4 18:33:40 2026 -0700
feat(c/driver_manager): polyfill numeric options for 1.0 drivers (#4774)
## Summary
- track the version actually accepted during driver negotiation
- for drivers negotiated down to ADBC 1.0, implement integer and double
setters through the legacy string setter
- use locale-independent numeric formatting
OpenAI Codex was used to help implement and test this change.
Closes #4115
---
c/driver_manager/adbc_driver_manager_api.cc | 64 +++++++-
.../adbc_version_100_compatibility_test.cc | 168 +++++++++++++++++++++
go/adbc/drivermgr/adbc_driver_manager_api.cc | 64 +++++++-
3 files changed, 294 insertions(+), 2 deletions(-)
diff --git a/c/driver_manager/adbc_driver_manager_api.cc
b/c/driver_manager/adbc_driver_manager_api.cc
index 19f8e7b4c5..6d4b66df7a 100644
--- a/c/driver_manager/adbc_driver_manager_api.cc
+++ b/c/driver_manager/adbc_driver_manager_api.cc
@@ -21,6 +21,10 @@
#include <cassert>
#include <cerrno>
#include <cstring>
+#include <iomanip>
+#include <limits>
+#include <locale>
+#include <sstream>
#include <string>
#include <unordered_map>
#include <utility>
@@ -404,6 +408,52 @@ AdbcStatusCode StatementSetOptionDouble(struct
AdbcStatement* statement, const c
return ADBC_STATUS_NOT_IMPLEMENTED;
}
+template <typename Object, typename Value, typename Setter>
+AdbcStatusCode SetNumericOptionAsString(Object* object, const char* key, Value
value,
+ struct AdbcError* error, Setter
setter) {
+ std::ostringstream text;
+ text.imbue(std::locale::classic());
+ text << std::setprecision(std::numeric_limits<double>::max_digits10) <<
value;
+ return setter(object, key, text.str().c_str(), error);
+}
+
+AdbcStatusCode DatabaseSetOptionIntLegacy(AdbcDatabase* database, const char*
key,
+ int64_t value, AdbcError* error) {
+ return SetNumericOptionAsString(database, key, value, error,
+ database->private_driver->DatabaseSetOption);
+}
+
+AdbcStatusCode DatabaseSetOptionDoubleLegacy(AdbcDatabase* database, const
char* key,
+ double value, AdbcError* error) {
+ return SetNumericOptionAsString(database, key, value, error,
+ database->private_driver->DatabaseSetOption);
+}
+
+AdbcStatusCode ConnectionSetOptionIntLegacy(AdbcConnection* connection, const
char* key,
+ int64_t value, AdbcError* error) {
+ return SetNumericOptionAsString(connection, key, value, error,
+
connection->private_driver->ConnectionSetOption);
+}
+
+AdbcStatusCode ConnectionSetOptionDoubleLegacy(AdbcConnection* connection,
+ const char* key, double value,
+ AdbcError* error) {
+ return SetNumericOptionAsString(connection, key, value, error,
+
connection->private_driver->ConnectionSetOption);
+}
+
+AdbcStatusCode StatementSetOptionIntLegacy(AdbcStatement* statement, const
char* key,
+ int64_t value, AdbcError* error) {
+ return SetNumericOptionAsString(statement, key, value, error,
+
statement->private_driver->StatementSetOption);
+}
+
+AdbcStatusCode StatementSetOptionDoubleLegacy(AdbcStatement* statement, const
char* key,
+ double value, AdbcError* error) {
+ return SetNumericOptionAsString(statement, key, value, error,
+
statement->private_driver->StatementSetOption);
+}
+
AdbcStatusCode StatementSetSqlQuery(struct AdbcStatement*, const char*,
struct AdbcError* error) {
SetError(error, "AdbcStatementSetSqlQuery not implemented");
@@ -1475,10 +1525,14 @@ AdbcStatusCode
AdbcLoadDriverFromInitFunc(AdbcDriverInitFunc init_func, int vers
// succession with the underlying driver until we find one that's
// accepted.
AdbcStatusCode result = ADBC_STATUS_NOT_IMPLEMENTED;
+ int negotiated_version = version;
for (const int try_version : kSupportedVersions) {
if (try_version > version) continue;
result = init_func(try_version, raw_driver, error);
- if (result != ADBC_STATUS_NOT_IMPLEMENTED) break;
+ if (result != ADBC_STATUS_NOT_IMPLEMENTED) {
+ negotiated_version = try_version;
+ break;
+ }
}
if (result != ADBC_STATUS_OK) {
return result;
@@ -1517,6 +1571,14 @@ AdbcStatusCode
AdbcLoadDriverFromInitFunc(AdbcDriverInitFunc init_func, int vers
}
if (version >= ADBC_VERSION_1_1_0) {
auto* driver = reinterpret_cast<struct AdbcDriver*>(raw_driver);
+ if (negotiated_version == ADBC_VERSION_1_0_0) {
+ driver->DatabaseSetOptionInt = DatabaseSetOptionIntLegacy;
+ driver->DatabaseSetOptionDouble = DatabaseSetOptionDoubleLegacy;
+ driver->ConnectionSetOptionInt = ConnectionSetOptionIntLegacy;
+ driver->ConnectionSetOptionDouble = ConnectionSetOptionDoubleLegacy;
+ driver->StatementSetOptionInt = StatementSetOptionIntLegacy;
+ driver->StatementSetOptionDouble = StatementSetOptionDoubleLegacy;
+ }
FILL_DEFAULT(driver, ErrorGetDetailCount);
FILL_DEFAULT(driver, ErrorGetDetail);
FILL_DEFAULT(driver, ErrorFromArrayStream);
diff --git a/c/driver_manager/adbc_version_100_compatibility_test.cc
b/c/driver_manager/adbc_version_100_compatibility_test.cc
index 43079ecb3e..8e7428d597 100644
--- a/c/driver_manager/adbc_version_100_compatibility_test.cc
+++ b/c/driver_manager/adbc_version_100_compatibility_test.cc
@@ -18,7 +18,12 @@
#include <gmock/gmock.h>
#include <gtest/gtest.h>
+#include <cmath>
+#include <limits>
+#include <locale>
#include <memory>
+#include <string>
+#include <vector>
#include "adbc_version_100.h"
#include "arrow-adbc/adbc.h"
@@ -30,11 +35,66 @@ namespace adbc {
using adbc_validation::IsOkStatus;
using adbc_validation::IsStatus;
+std::vector<std::string> option_values;
+AdbcStatusCode option_status = ADBC_STATUS_OK;
+
+template <typename Object>
+AdbcStatusCode CaptureOption(Object*, const char* key, const char* value,
AdbcError*) {
+ EXPECT_STREQ(key, "option");
+ option_values.emplace_back(value);
+ return option_status;
+}
+
+AdbcStatusCode LegacyOptionDriverInit(int version, void* raw_driver,
AdbcError* error) {
+ auto status = Version100DriverInit(version, raw_driver, error);
+ if (status != ADBC_STATUS_OK) return status;
+
+ auto* driver = static_cast<AdbcDriver*>(raw_driver);
+ driver->DatabaseSetOption = CaptureOption<AdbcDatabase>;
+ driver->ConnectionSetOption = CaptureOption<AdbcConnection>;
+ driver->StatementSetOption = CaptureOption<AdbcStatement>;
+ return ADBC_STATUS_OK;
+}
+
+AdbcStatusCode Version110OptionDriverInit(int, void* raw_driver, AdbcError*
error) {
+ return LegacyOptionDriverInit(ADBC_VERSION_1_0_0, raw_driver, error);
+}
+
+AdbcStatusCode NativeStatementSetOptionInt(AdbcStatement*, const char*,
int64_t,
+ AdbcError*) {
+ return ADBC_STATUS_CANCELLED;
+}
+
+AdbcStatusCode Version110NativeOptionDriverInit(int version, void* raw_driver,
+ AdbcError* error) {
+ auto status = Version110OptionDriverInit(version, raw_driver, error);
+ static_cast<AdbcDriver*>(raw_driver)->StatementSetOptionInt =
+ NativeStatementSetOptionInt;
+ return status;
+}
+
+struct CommaDecimal : std::numpunct<char> {
+ char do_decimal_point() const override { return ','; }
+};
+
+class ScopedCommaLocale {
+ public:
+ ScopedCommaLocale() : previous_(std::locale()) {
+ std::locale::global(std::locale(previous_, new CommaDecimal));
+ }
+ ~ScopedCommaLocale() { std::locale::global(previous_); }
+
+ private:
+ std::locale previous_;
+};
+
class AdbcVersion : public ::testing::Test {
public:
void SetUp() override {
std::memset(&driver, 0, sizeof(driver));
std::memset(&error, 0, sizeof(error));
+ option_values.clear();
+ option_status = ADBC_STATUS_OK;
}
void TearDown() override {
@@ -105,6 +165,114 @@ TEST_F(AdbcVersion, OldDriverNewManager) {
EXPECT_NE(driver.StatementSetOptionDouble, nullptr);
}
+TEST_F(AdbcVersion, OldDriverNumericSetters) {
+ ASSERT_THAT(AdbcLoadDriverFromInitFunc(&LegacyOptionDriverInit,
ADBC_VERSION_1_1_0,
+ &driver, &error),
+ IsOkStatus(&error));
+
+ AdbcDatabase database = {nullptr, &driver};
+ AdbcConnection connection = {nullptr, &driver};
+ AdbcStatement statement = {nullptr, &driver};
+ ASSERT_THAT(driver.DatabaseNew(&database, &error), IsOkStatus(&error));
+ ASSERT_THAT(driver.ConnectionNew(&connection, &error), IsOkStatus(&error));
+ ASSERT_THAT(driver.StatementNew(&connection, &statement, &error),
IsOkStatus(&error));
+
+ ScopedCommaLocale locale;
+ for (int64_t value : {std::numeric_limits<int64_t>::min(), int64_t{0},
+ std::numeric_limits<int64_t>::max()}) {
+ ASSERT_THAT(AdbcDatabaseSetOptionInt(&database, "option", value, &error),
+ IsOkStatus(&error));
+ ASSERT_THAT(AdbcConnectionSetOptionInt(&connection, "option", value,
&error),
+ IsOkStatus(&error));
+ ASSERT_THAT(AdbcStatementSetOptionInt(&statement, "option", value, &error),
+ IsOkStatus(&error));
+ for (size_t i = option_values.size() - 3; i < option_values.size(); ++i) {
+ EXPECT_EQ(std::to_string(value), option_values[i]);
+ }
+ }
+
+ for (double value : {1.2345678901234567, 1e-12, -0.0, 1e200}) {
+ ASSERT_THAT(AdbcDatabaseSetOptionDouble(&database, "option", value,
&error),
+ IsOkStatus(&error));
+ ASSERT_THAT(AdbcConnectionSetOptionDouble(&connection, "option", value,
&error),
+ IsOkStatus(&error));
+ ASSERT_THAT(AdbcStatementSetOptionDouble(&statement, "option", value,
&error),
+ IsOkStatus(&error));
+ for (size_t i = option_values.size() - 3; i < option_values.size(); ++i) {
+ EXPECT_EQ(std::string::npos, option_values[i].find(','));
+ EXPECT_EQ(value, std::stod(option_values[i]));
+ EXPECT_EQ(std::signbit(value),
std::signbit(std::stod(option_values[i])));
+ }
+ }
+
+ EXPECT_THAT(driver.StatementRelease(&statement, &error), IsOkStatus(&error));
+ EXPECT_THAT(driver.ConnectionRelease(&connection, &error),
IsOkStatus(&error));
+ EXPECT_THAT(driver.DatabaseRelease(&database, &error), IsOkStatus(&error));
+}
+
+TEST_F(AdbcVersion, OldDriverQueuedOptionsAndErrorStatus) {
+ AdbcDatabase database = {};
+ AdbcConnection connection = {};
+ AdbcStatement statement = {};
+ ASSERT_THAT(AdbcDatabaseNew(&database, &error), IsOkStatus(&error));
+ ASSERT_THAT(
+ AdbcDriverManagerDatabaseSetInitFunc(&database, LegacyOptionDriverInit,
&error),
+ IsOkStatus(&error));
+ ASSERT_THAT(AdbcDatabaseSetOptionInt(&database, "option", 42, &error),
+ IsOkStatus(&error));
+ ASSERT_THAT(AdbcDatabaseInit(&database, &error), IsOkStatus(&error));
+ ASSERT_THAT(AdbcConnectionNew(&connection, &error), IsOkStatus(&error));
+ ASSERT_THAT(AdbcConnectionSetOptionDouble(&connection, "option", 1e-12,
&error),
+ IsOkStatus(&error));
+ ASSERT_THAT(AdbcConnectionInit(&connection, &database, &error),
IsOkStatus(&error));
+ ASSERT_THAT(AdbcStatementNew(&connection, &statement, &error),
IsOkStatus(&error));
+
+ option_status = ADBC_STATUS_INVALID_ARGUMENT;
+ EXPECT_THAT(AdbcStatementSetOptionInt(&statement, "option", 1, &error),
+ IsStatus(option_status, &error));
+ ASSERT_EQ(3U, option_values.size());
+ EXPECT_EQ("42", option_values[0]);
+ EXPECT_EQ(1e-12, std::stod(option_values[1]));
+ EXPECT_EQ("1", option_values[2]);
+
+ option_status = ADBC_STATUS_OK;
+ EXPECT_THAT(AdbcStatementRelease(&statement, &error), IsOkStatus(&error));
+ EXPECT_THAT(AdbcConnectionRelease(&connection, &error), IsOkStatus(&error));
+ EXPECT_THAT(AdbcDatabaseRelease(&database, &error), IsOkStatus(&error));
+}
+
+TEST_F(AdbcVersion, Version110DriverDoesNotUseFallback) {
+ ASSERT_THAT(AdbcLoadDriverFromInitFunc(&Version110OptionDriverInit,
ADBC_VERSION_1_1_0,
+ &driver, &error),
+ IsOkStatus(&error));
+ AdbcStatement statement = {nullptr, &driver};
+ EXPECT_THAT(AdbcStatementSetOptionInt(&statement, "option", 1, &error),
+ IsStatus(ADBC_STATUS_NOT_IMPLEMENTED, &error));
+ EXPECT_TRUE(option_values.empty());
+}
+
+TEST_F(AdbcVersion, OldDriverBytesRemainUnsupported) {
+ ASSERT_THAT(AdbcLoadDriverFromInitFunc(&LegacyOptionDriverInit,
ADBC_VERSION_1_1_0,
+ &driver, &error),
+ IsOkStatus(&error));
+ AdbcStatement statement = {nullptr, &driver};
+ const uint8_t value[] = {'a', 0, 'b'};
+ EXPECT_THAT(
+ AdbcStatementSetOptionBytes(&statement, "option", value, sizeof(value),
&error),
+ IsStatus(ADBC_STATUS_NOT_IMPLEMENTED, &error));
+ EXPECT_TRUE(option_values.empty());
+}
+
+TEST_F(AdbcVersion, Version110NativeNumericSetterIsPreserved) {
+ ASSERT_THAT(AdbcLoadDriverFromInitFunc(&Version110NativeOptionDriverInit,
+ ADBC_VERSION_1_1_0, &driver, &error),
+ IsOkStatus(&error));
+ AdbcStatement statement = {nullptr, &driver};
+ EXPECT_THAT(AdbcStatementSetOptionInt(&statement, "option", 1, &error),
+ IsStatus(ADBC_STATUS_CANCELLED, &error));
+ EXPECT_TRUE(option_values.empty());
+}
+
// N.B. see postgresql_test.cc for backwards compatibility test of AdbcError
// N.B. see postgresql_test.cc for backwards compatibility test of AdbcDriver
diff --git a/go/adbc/drivermgr/adbc_driver_manager_api.cc
b/go/adbc/drivermgr/adbc_driver_manager_api.cc
index 19f8e7b4c5..6d4b66df7a 100644
--- a/go/adbc/drivermgr/adbc_driver_manager_api.cc
+++ b/go/adbc/drivermgr/adbc_driver_manager_api.cc
@@ -21,6 +21,10 @@
#include <cassert>
#include <cerrno>
#include <cstring>
+#include <iomanip>
+#include <limits>
+#include <locale>
+#include <sstream>
#include <string>
#include <unordered_map>
#include <utility>
@@ -404,6 +408,52 @@ AdbcStatusCode StatementSetOptionDouble(struct
AdbcStatement* statement, const c
return ADBC_STATUS_NOT_IMPLEMENTED;
}
+template <typename Object, typename Value, typename Setter>
+AdbcStatusCode SetNumericOptionAsString(Object* object, const char* key, Value
value,
+ struct AdbcError* error, Setter
setter) {
+ std::ostringstream text;
+ text.imbue(std::locale::classic());
+ text << std::setprecision(std::numeric_limits<double>::max_digits10) <<
value;
+ return setter(object, key, text.str().c_str(), error);
+}
+
+AdbcStatusCode DatabaseSetOptionIntLegacy(AdbcDatabase* database, const char*
key,
+ int64_t value, AdbcError* error) {
+ return SetNumericOptionAsString(database, key, value, error,
+ database->private_driver->DatabaseSetOption);
+}
+
+AdbcStatusCode DatabaseSetOptionDoubleLegacy(AdbcDatabase* database, const
char* key,
+ double value, AdbcError* error) {
+ return SetNumericOptionAsString(database, key, value, error,
+ database->private_driver->DatabaseSetOption);
+}
+
+AdbcStatusCode ConnectionSetOptionIntLegacy(AdbcConnection* connection, const
char* key,
+ int64_t value, AdbcError* error) {
+ return SetNumericOptionAsString(connection, key, value, error,
+
connection->private_driver->ConnectionSetOption);
+}
+
+AdbcStatusCode ConnectionSetOptionDoubleLegacy(AdbcConnection* connection,
+ const char* key, double value,
+ AdbcError* error) {
+ return SetNumericOptionAsString(connection, key, value, error,
+
connection->private_driver->ConnectionSetOption);
+}
+
+AdbcStatusCode StatementSetOptionIntLegacy(AdbcStatement* statement, const
char* key,
+ int64_t value, AdbcError* error) {
+ return SetNumericOptionAsString(statement, key, value, error,
+
statement->private_driver->StatementSetOption);
+}
+
+AdbcStatusCode StatementSetOptionDoubleLegacy(AdbcStatement* statement, const
char* key,
+ double value, AdbcError* error) {
+ return SetNumericOptionAsString(statement, key, value, error,
+
statement->private_driver->StatementSetOption);
+}
+
AdbcStatusCode StatementSetSqlQuery(struct AdbcStatement*, const char*,
struct AdbcError* error) {
SetError(error, "AdbcStatementSetSqlQuery not implemented");
@@ -1475,10 +1525,14 @@ AdbcStatusCode
AdbcLoadDriverFromInitFunc(AdbcDriverInitFunc init_func, int vers
// succession with the underlying driver until we find one that's
// accepted.
AdbcStatusCode result = ADBC_STATUS_NOT_IMPLEMENTED;
+ int negotiated_version = version;
for (const int try_version : kSupportedVersions) {
if (try_version > version) continue;
result = init_func(try_version, raw_driver, error);
- if (result != ADBC_STATUS_NOT_IMPLEMENTED) break;
+ if (result != ADBC_STATUS_NOT_IMPLEMENTED) {
+ negotiated_version = try_version;
+ break;
+ }
}
if (result != ADBC_STATUS_OK) {
return result;
@@ -1517,6 +1571,14 @@ AdbcStatusCode
AdbcLoadDriverFromInitFunc(AdbcDriverInitFunc init_func, int vers
}
if (version >= ADBC_VERSION_1_1_0) {
auto* driver = reinterpret_cast<struct AdbcDriver*>(raw_driver);
+ if (negotiated_version == ADBC_VERSION_1_0_0) {
+ driver->DatabaseSetOptionInt = DatabaseSetOptionIntLegacy;
+ driver->DatabaseSetOptionDouble = DatabaseSetOptionDoubleLegacy;
+ driver->ConnectionSetOptionInt = ConnectionSetOptionIntLegacy;
+ driver->ConnectionSetOptionDouble = ConnectionSetOptionDoubleLegacy;
+ driver->StatementSetOptionInt = StatementSetOptionIntLegacy;
+ driver->StatementSetOptionDouble = StatementSetOptionDoubleLegacy;
+ }
FILL_DEFAULT(driver, ErrorGetDetailCount);
FILL_DEFAULT(driver, ErrorGetDetail);
FILL_DEFAULT(driver, ErrorFromArrayStream);
