From 8a9b179c3b399e106f3d90cb6f1a6ebdb54a3f7b Mon Sep 17 00:00:00 2001 From: David Li Date: Tue, 4 Aug 2026 10:54:04 +0900 Subject: [PATCH 1/4] feat(c/driver/sqlite): ensure batch rows option can be retrieved The getter existed, but the framework needed to be extended to also allow fetching it as a string, and tests were added. Closes #3456. Assisted-by: GPT-5.6 Sol --- c/driver/framework/base_driver.h | 16 +++-- c/driver/sqlite/sqlite_test.cc | 71 +++++++++++++++++++ .../adbc_driver_sqlite/tests/test_lowlevel.py | 7 ++ 3 files changed, 90 insertions(+), 4 deletions(-) diff --git a/c/driver/framework/base_driver.h b/c/driver/framework/base_driver.h index bebdffea6b..c0edad79a8 100644 --- a/c/driver/framework/base_driver.h +++ b/c/driver/framework/base_driver.h @@ -169,11 +169,19 @@ class Option { return std::visit( [&](auto&& value) -> AdbcStatusCode { using T = std::decay_t; - if constexpr (std::is_same_v) { - size_t value_size_with_terminator = value.size() + 1; + if constexpr (std::is_same_v || std::is_same_v) { + std::string formatted; + std::string_view string_value; + if constexpr (std::is_same_v) { + string_value = value; + } else { + formatted = std::to_string(value); + string_value = formatted; + } + size_t value_size_with_terminator = string_value.size() + 1; if (*length >= value_size_with_terminator) { - std::memcpy(out, value.data(), value.size()); - out[value.size()] = 0; + std::memcpy(out, string_value.data(), string_value.size()); + out[string_value.size()] = 0; } *length = value_size_with_terminator; return ADBC_STATUS_OK; diff --git a/c/driver/sqlite/sqlite_test.cc b/c/driver/sqlite/sqlite_test.cc index 243449eb07..77ad60e857 100644 --- a/c/driver/sqlite/sqlite_test.cc +++ b/c/driver/sqlite/sqlite_test.cc @@ -454,6 +454,77 @@ TEST(SqliteUriWrapper, SqliteUriFilename) { adbc_validation::IsOkStatus(&error)); } +TEST(SqliteOptions, BatchRowsGetOption) { + struct AdbcError error = ADBC_ERROR_INIT; + adbc_validation::Handle database; + adbc_validation::Handle connection; + adbc_validation::Handle statement; + constexpr const char* kBatchRows = "adbc.sqlite.query.batch_rows"; + + ASSERT_THAT(AdbcDatabaseNew(&database.value, &error), + adbc_validation::IsOkStatus(&error)); + + int64_t int_value = 0; + ASSERT_THAT(AdbcDatabaseGetOptionInt(&database.value, kBatchRows, &int_value, &error), + adbc_validation::IsOkStatus(&error)); + EXPECT_EQ(1024, int_value); + + char too_small[2] = {'x', 'x'}; + size_t length = sizeof(too_small); + ASSERT_THAT( + AdbcDatabaseGetOption(&database.value, kBatchRows, too_small, &length, &error), + adbc_validation::IsOkStatus(&error)); + EXPECT_EQ(5, length); + EXPECT_THAT(too_small, ::testing::ElementsAre('x', 'x')); + + ASSERT_THAT(AdbcDatabaseSetOption(&database.value, kBatchRows, "41", &error), + adbc_validation::IsOkStatus(&error)); + char string_value[3] = {}; + length = sizeof(string_value); + ASSERT_THAT( + AdbcDatabaseGetOption(&database.value, kBatchRows, string_value, &length, &error), + adbc_validation::IsOkStatus(&error)); + EXPECT_EQ(3, length); + EXPECT_STREQ("41", string_value); + + ASSERT_THAT(AdbcDatabaseInit(&database.value, &error), + adbc_validation::IsOkStatus(&error)); + ASSERT_THAT(AdbcConnectionNew(&connection.value, &error), + adbc_validation::IsOkStatus(&error)); + ASSERT_THAT(AdbcConnectionInit(&connection.value, &database.value, &error), + adbc_validation::IsOkStatus(&error)); + ASSERT_THAT( + AdbcConnectionGetOptionInt(&connection.value, kBatchRows, &int_value, &error), + adbc_validation::IsOkStatus(&error)); + EXPECT_EQ(41, int_value); + + ASSERT_THAT(AdbcStatementNew(&connection.value, &statement.value, &error), + adbc_validation::IsOkStatus(&error)); + ASSERT_THAT(AdbcStatementGetOptionInt(&statement.value, kBatchRows, &int_value, &error), + adbc_validation::IsOkStatus(&error)); + EXPECT_EQ(41, int_value); + + ASSERT_THAT(AdbcStatementSetOptionInt(&statement.value, kBatchRows, 42, &error), + adbc_validation::IsOkStatus(&error)); + char statement_value[3] = {}; + length = sizeof(statement_value); + ASSERT_THAT(AdbcStatementGetOption(&statement.value, kBatchRows, statement_value, + &length, &error), + adbc_validation::IsOkStatus(&error)); + EXPECT_EQ(3, length); + EXPECT_STREQ("42", statement_value); + + ASSERT_THAT(AdbcStatementSetOption(&statement.value, "adbc.statement.bind_by_name", + ADBC_OPTION_VALUE_ENABLED, &error), + adbc_validation::IsOkStatus(&error)); + char bind_by_name[sizeof(ADBC_OPTION_VALUE_ENABLED)] = {}; + length = sizeof(bind_by_name); + ASSERT_THAT(AdbcStatementGetOption(&statement.value, "adbc.statement.bind_by_name", + bind_by_name, &length, &error), + adbc_validation::IsOkStatus(&error)); + EXPECT_STREQ(ADBC_OPTION_VALUE_ENABLED, bind_by_name); +} + class SqliteStatementTest : public ::testing::Test, public adbc_validation::StatementTest { public: diff --git a/python/adbc_driver_sqlite/tests/test_lowlevel.py b/python/adbc_driver_sqlite/tests/test_lowlevel.py index 9c8afcac3b..f1f2174a6f 100644 --- a/python/adbc_driver_sqlite/tests/test_lowlevel.py +++ b/python/adbc_driver_sqlite/tests/test_lowlevel.py @@ -49,6 +49,13 @@ def test_options(sqlite): adbc_driver_sqlite.StatementOptions.BATCH_ROWS.value: "1", } ) + assert ( + stmt.get_option(adbc_driver_sqlite.StatementOptions.BATCH_ROWS.value) == "1" + ) + assert ( + stmt.get_option_int(adbc_driver_sqlite.StatementOptions.BATCH_ROWS.value) + == 1 + ) stmt.set_sql_query("SELECT 1") stream, _ = stmt.execute_query() reader = pyarrow.RecordBatchReader._import_from_c(stream.address) From f6118b39f6d48ae4dce555816674b7c9ccd1992a Mon Sep 17 00:00:00 2001 From: David Li Date: Tue, 4 Aug 2026 13:47:50 +0900 Subject: [PATCH 2/4] fix r --- r/adbcdrivermanager/tests/testthat/test-options.R | 15 ++++++--------- 1 file changed, 6 insertions(+), 9 deletions(-) diff --git a/r/adbcdrivermanager/tests/testthat/test-options.R b/r/adbcdrivermanager/tests/testthat/test-options.R index 8d978f838c..d13068162a 100644 --- a/r/adbcdrivermanager/tests/testthat/test-options.R +++ b/r/adbcdrivermanager/tests/testthat/test-options.R @@ -251,34 +251,31 @@ test_that("void driver errors getting bytes option of incorrect type", { ) }) -test_that("void driver errors getting integer option of incorrect type", { +test_that("void driver errors getting integer option as bytes", { db <- adbc_database_init(adbc_driver_void()) adbc_database_set_options(db, list("some_key" = 123L)) - expect_error( - adbc_database_get_option(db, "some_key"), - class = "adbc_status_not_found" - ) - expect_error( adbc_database_get_option_bytes(db, "some_key"), class = "adbc_status_not_found" ) - - }) test_that("void driver can get integer option of compatible type", { db <- adbc_database_init(adbc_driver_void()) adbc_database_set_options(db, list("some_key" = 123L)) + expect_identical( + adbc_database_get_option(db, "some_key"), + "123" + ) + expect_identical( adbc_database_get_option_double(db, "some_key"), 123.0 ) }) - test_that("void driver errors getting double option of incorrect type", { db <- adbc_database_init(adbc_driver_void()) adbc_database_set_options(db, list("some_key" = 123.4)) From 8748c5b6649498e8d3effdfcee5b24661c278cbe Mon Sep 17 00:00:00 2001 From: David Li Date: Wed, 5 Aug 2026 13:35:13 +0900 Subject: [PATCH 3/4] also handle doubles --- c/driver/framework/base_driver.h | 19 +++++++++++++------ .../tests/testthat/test-options.R | 17 +++++++++++------ 2 files changed, 24 insertions(+), 12 deletions(-) diff --git a/c/driver/framework/base_driver.h b/c/driver/framework/base_driver.h index c0edad79a8..c86a5045a6 100644 --- a/c/driver/framework/base_driver.h +++ b/c/driver/framework/base_driver.h @@ -54,8 +54,8 @@ enum class LifecycleState { kInitialized, }; -/// \brief A typed option value wrapper. It currently does not attempt -/// conversion (i.e., getting a double option as a string). +/// \brief A typed option value wrapper. Numeric values can be retrieved as strings, +/// but other conversions are not attempted. class Option { public: /// \brief The option is unset. @@ -169,14 +169,21 @@ class Option { return std::visit( [&](auto&& value) -> AdbcStatusCode { using T = std::decay_t; - if constexpr (std::is_same_v || std::is_same_v) { - std::string formatted; + if constexpr (std::is_same_v || std::is_same_v || + std::is_same_v) { + char formatted[24]; // Enough room for double/int64_t std::string_view string_value; if constexpr (std::is_same_v) { string_value = value; } else { - formatted = std::to_string(value); - string_value = formatted; + auto result = + std::to_chars(formatted, formatted + sizeof(formatted), value); + if (result.ec != std::errc()) { + return status::Internal("Could not format numeric option value") + .ToAdbc(error); + } + string_value = std::string_view( + formatted, static_cast(result.ptr - formatted)); } size_t value_size_with_terminator = string_value.size() + 1; if (*length >= value_size_with_terminator) { diff --git a/r/adbcdrivermanager/tests/testthat/test-options.R b/r/adbcdrivermanager/tests/testthat/test-options.R index d13068162a..dc8c2f295d 100644 --- a/r/adbcdrivermanager/tests/testthat/test-options.R +++ b/r/adbcdrivermanager/tests/testthat/test-options.R @@ -276,15 +276,10 @@ test_that("void driver can get integer option of compatible type", { ) }) -test_that("void driver errors getting double option of incorrect type", { +test_that("void driver errors getting double option as incompatible type", { db <- adbc_database_init(adbc_driver_void()) adbc_database_set_options(db, list("some_key" = 123.4)) - expect_error( - adbc_database_get_option(db, "some_key"), - class = "adbc_status_not_found" - ) - expect_error( adbc_database_get_option_bytes(db, "some_key"), class = "adbc_status_not_found" @@ -296,6 +291,16 @@ test_that("void driver errors getting double option of incorrect type", { ) }) +test_that("void driver can get double option of compatible type", { + db <- adbc_database_init(adbc_driver_void()) + adbc_database_set_options(db, list("some_key" = 123.4)) + + expect_identical( + adbc_database_get_option(db, "some_key"), + "123.4" + ) +}) + test_that("key_value_options works", { expect_identical( key_value_options(NULL), From ebfdb8f92d2a7b4071a1fc711636b8f3b05dbd85 Mon Sep 17 00:00:00 2001 From: David Li Date: Wed, 5 Aug 2026 13:52:40 +0900 Subject: [PATCH 4/4] nit --- c/driver/framework/base_driver.h | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/c/driver/framework/base_driver.h b/c/driver/framework/base_driver.h index c86a5045a6..36150c8853 100644 --- a/c/driver/framework/base_driver.h +++ b/c/driver/framework/base_driver.h @@ -54,8 +54,7 @@ enum class LifecycleState { kInitialized, }; -/// \brief A typed option value wrapper. Numeric values can be retrieved as strings, -/// but other conversions are not attempted. +/// \brief A typed option value wrapper. Attempts some conversions between types. class Option { public: /// \brief The option is unset.