emecii commented on code in PR #4774:
URL: https://github.com/apache/arrow-adbc/pull/4774#discussion_r4058714474


##########
c/driver_manager/adbc_driver_manager_api.cc:
##########
@@ -404,6 +408,55 @@ AdbcStatusCode StatementSetOptionDouble(struct 
AdbcStatement* statement, const c
   return ADBC_STATUS_NOT_IMPLEMENTED;
 }
 
+// Bridge numeric options only for drivers negotiated down to ADBC 1.0.0.
+// Bytes deliberately remain unsupported: embedded NUL bytes have no lossless
+// representation in the string-only API.

Review Comment:
   Removed in 2022a37 from both the C implementation and Go/CGo mirror.
   
   AI-generated reply (OpenAI Codex).



##########
c/driver_manager/adbc_version_100_compatibility_test.cc:
##########
@@ -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));

Review Comment:
   `std::locale` owns the facet: `std::numpunct` defaults to `refs = 0`, so the 
last locale referencing it deletes it ([C++ lifetime 
rules](https://eel.is/c++draft/locale.facet#3)). Restoring `previous_` releases 
the comma locale. A destructor-counting probe confirmed cleanup, including 
exception unwinding and a retained locale copy; no ownership change is needed.
   
   AI-generated reply (OpenAI Codex).



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]

Reply via email to