From 042f9ef4e494bca97307dfa55433f426ec95f416 Mon Sep 17 00:00:00 2001 From: Dewey Dunnington Date: Wed, 25 Oct 2023 20:07:46 +0000 Subject: [PATCH 1/7] feat(c/driver/sqlite): Support BLOB in result sets for SQLite (#1223) It looks like the binding + ingest was supported already, it was just the result sets that were not supported. From the R bindings: ``` r library(adbcdrivermanager) db <- adbc_database_init(adbcsqlite::adbcsqlite(), uri = ":memory:") con <- adbc_connection_init(db) read_adbc(con, "SELECT X'00' as a") |> as.data.frame() #> a #> 1 blob[1 B] ``` Created on 2023-10-25 with [reprex v2.0.2](https://reprex.tidyverse.org) --- c/driver/sqlite/sqlite_test.cc | 36 +++++++++++++- c/driver/sqlite/statement_reader.c | 75 +++++++++++++++++++++++++----- 2 files changed, 99 insertions(+), 12 deletions(-) diff --git a/c/driver/sqlite/sqlite_test.cc b/c/driver/sqlite/sqlite_test.cc index e5566df260..f4455a572e 100644 --- a/c/driver/sqlite/sqlite_test.cc +++ b/c/driver/sqlite/sqlite_test.cc @@ -249,7 +249,6 @@ class SqliteStatementTest : public ::testing::Test, return TestSqlIngestType(NANOARROW_TYPE_UINT64, values); } - void TestSqlIngestBinary() { GTEST_SKIP() << "Cannot ingest BINARY (not implemented)"; } void TestSqlIngestDuration() { GTEST_SKIP() << "Cannot ingest DURATION (not implemented)"; } @@ -555,6 +554,22 @@ TEST_F(SqliteReaderTest, InferIntRejectStr) { "[SQLite] Type mismatch in column 0: expected INT64 but got STRING/BINARY")); } +TEST_F(SqliteReaderTest, InferIntRejectBlob) { + adbc_validation::StreamReader reader; + ASSERT_NO_FATAL_FAILURE( + ExecSelect(R"((1), (NULL), (X''), (NULL))", /*infer_rows=*/2, &reader)); + ASSERT_EQ(NANOARROW_TYPE_INT64, reader.fields[0].type); + ASSERT_NO_FATAL_FAILURE(reader.Next()); + ASSERT_NO_FATAL_FAILURE( + CompareArray(reader.array_view->children[0], {1, std::nullopt})); + + ASSERT_THAT(reader.MaybeNext(), ::testing::Not(IsOkErrno())); + ASSERT_THAT( + reader.stream->get_last_error(&reader.stream.value), + ::testing::HasSubstr( + "[SQLite] Type mismatch in column 0: expected INT64 but got STRING/BINARY")); +} + TEST_F(SqliteReaderTest, InferFloatReadIntFloat) { adbc_validation::StreamReader reader; ASSERT_NO_FATAL_FAILURE( @@ -592,6 +607,25 @@ TEST_F(SqliteReaderTest, InferFloatRejectStr) { "[SQLite] Type mismatch in column 0: expected DOUBLE but got STRING/BINARY")); } +TEST_F(SqliteReaderTest, InferFloatRejectBlob) { + adbc_validation::StreamReader reader; + ASSERT_NO_FATAL_FAILURE(ExecSelect(R"((1E0), (NULL), (2E0), (3), (X''), (NULL))", + /*infer_rows=*/2, &reader)); + ASSERT_EQ(NANOARROW_TYPE_DOUBLE, reader.fields[0].type); + ASSERT_NO_FATAL_FAILURE(reader.Next()); + ASSERT_NO_FATAL_FAILURE( + CompareArray(reader.array_view->children[0], {1.0, std::nullopt})); + ASSERT_NO_FATAL_FAILURE(reader.Next()); + ASSERT_NO_FATAL_FAILURE( + CompareArray(reader.array_view->children[0], {2.0, 3.0})); + + ASSERT_THAT(reader.MaybeNext(), ::testing::Not(IsOkErrno())); + ASSERT_THAT( + reader.stream->get_last_error(&reader.stream.value), + ::testing::HasSubstr( + "[SQLite] Type mismatch in column 0: expected DOUBLE but got STRING/BINARY")); +} + TEST_F(SqliteReaderTest, InferStrReadAll) { adbc_validation::StreamReader reader; ASSERT_NO_FATAL_FAILURE(ExecSelect(R"((""), (NULL), (2), (3E0), ("foo"), (NULL))", diff --git a/c/driver/sqlite/statement_reader.c b/c/driver/sqlite/statement_reader.c index c609e1e416..e3b2525b1b 100644 --- a/c/driver/sqlite/statement_reader.c +++ b/c/driver/sqlite/statement_reader.c @@ -308,7 +308,7 @@ AdbcStatusCode AdbcSqliteBinderBindNext(struct AdbcSqliteBinder* binder, sqlite3 case NANOARROW_TYPE_LARGE_BINARY: { struct ArrowBufferView value = ArrowArrayViewGetBytesUnsafe(binder->batch.children[col], binder->next_row); - status = sqlite3_bind_text(stmt, col + 1, value.data.as_char, value.size_bytes, + status = sqlite3_bind_blob(stmt, col + 1, value.data.as_char, value.size_bytes, SQLITE_STATIC); break; } @@ -529,14 +529,8 @@ int StatementReaderGetOneValue(struct StatementReader* reader, int col, case SQLITE_INTEGER: case SQLITE_FLOAT: case SQLITE_TEXT: - case SQLITE_BLOB: { - // Let SQLite convert - struct ArrowStringView value = { - .data = (const char*)sqlite3_column_text(reader->stmt, col), - .size_bytes = sqlite3_column_bytes(reader->stmt, col), - }; - return ArrowArrayAppendString(out, value); - } + case SQLITE_BLOB: + break; default: { snprintf(reader->error.message, sizeof(reader->error.message), "[SQLite] Type mismatch in column %d: expected STRING but got unknown " @@ -545,7 +539,34 @@ int StatementReaderGetOneValue(struct StatementReader* reader, int col, return ENOTSUP; } } - break; + + // Let SQLite convert + struct ArrowStringView value = { + .data = (const char*)sqlite3_column_text(reader->stmt, col), + .size_bytes = sqlite3_column_bytes(reader->stmt, col), + }; + return ArrowArrayAppendString(out, value); + } + + case NANOARROW_TYPE_BINARY: { + switch (sqlite_type) { + case SQLITE_TEXT: + case SQLITE_BLOB: + break; + default: { + snprintf(reader->error.message, sizeof(reader->error.message), + "[SQLite] Type mismatch in column %d: expected BLOB but got unknown " + "type %d", + col, sqlite_type); + return ENOTSUP; + } + } + + // Let SQLite convert + struct ArrowBufferView value; + value.data.data = sqlite3_column_blob(reader->stmt, col); + value.size_bytes = sqlite3_column_bytes(reader->stmt, col); + return ArrowArrayAppendBytes(out, value); } default: { @@ -1014,7 +1035,39 @@ AdbcStatusCode StatementReaderInferOneValue( CHECK_NA(INTERNAL, ArrowBufferAppend(data, &offset, sizeof(offset)), error); break; } - case SQLITE_BLOB: + case SQLITE_BLOB: { + ArrowBitmapAppendUnsafe(validity, /*set=*/1, /*length=*/1); + + switch (*current_type) { + case NANOARROW_TYPE_INT64: { + AdbcStatusCode status = StatementReaderUpcastInt64ToBinary(data, binary, error); + if (status != ADBC_STATUS_OK) return status; + *current_type = NANOARROW_TYPE_BINARY; + break; + } + case NANOARROW_TYPE_DOUBLE: { + AdbcStatusCode status = + StatementReaderUpcastDoubleToBinary(data, binary, error); + if (status != ADBC_STATUS_OK) return status; + *current_type = NANOARROW_TYPE_BINARY; + break; + } + case NANOARROW_TYPE_STRING: + *current_type = NANOARROW_TYPE_BINARY; + break; + case NANOARROW_TYPE_BINARY: + break; + default: + return ADBC_STATUS_INTERNAL; + } + + const void* value = sqlite3_column_blob(stmt, col); + const int size = sqlite3_column_bytes(stmt, col); + const int32_t offset = ((int32_t*)data->data)[data->size_bytes / 4 - 1] + size; + CHECK_NA(INTERNAL, ArrowBufferAppend(binary, value, size), error); + CHECK_NA(INTERNAL, ArrowBufferAppend(data, &offset, sizeof(offset)), error); + break; + } default: { return ADBC_STATUS_NOT_IMPLEMENTED; } From 7d33768489c0b6ba243b77003de888062b49a5e0 Mon Sep 17 00:00:00 2001 From: Solomon Choe <128758390+ywc88@users.noreply.github.com> Date: Wed, 25 Oct 2023 13:11:23 -0700 Subject: [PATCH 2/7] fix(c/driver/sqlite): Provide # of rows affected for non-SELECT statements instead of 0 (#1179) Fixes #1145 If we prefer consistency with the stream case for # of rows affected instead of the actual # of rows affected, we can change this line: ``` *rows_affected = sqlite3_changes(stmt->conn); ``` to be: ``` *rows_affected = -1; ``` --- c/driver/flightsql/dremio_flightsql_test.cc | 6 +++ c/driver/flightsql/sqlite_flightsql_test.cc | 6 +++ c/driver/postgresql/postgresql_test.cc | 6 +++ c/driver/sqlite/sqlite.c | 8 ++- c/integration/duckdb/duckdb_test.cc | 6 +++ c/validation/adbc_validation.cc | 60 +++++++++++++++++++++ c/validation/adbc_validation.h | 6 +++ 7 files changed, 97 insertions(+), 1 deletion(-) diff --git a/c/driver/flightsql/dremio_flightsql_test.cc b/c/driver/flightsql/dremio_flightsql_test.cc index 52c184f385..8c59eb4a29 100644 --- a/c/driver/flightsql/dremio_flightsql_test.cc +++ b/c/driver/flightsql/dremio_flightsql_test.cc @@ -92,6 +92,12 @@ class DremioFlightSqlStatementTest : public ::testing::Test, void TestSqlIngestColumnEscaping() { GTEST_SKIP() << "Column escaping not implemented"; } + void TestSqlQueryRowsAffectedDelete() { + GTEST_SKIP() << "Cannot query rows affected in delete (not implemented)"; + } + void TestSqlQueryRowsAffectedDeleteStream() { + GTEST_SKIP() << "Cannot query rows affected in delete stream (not implemented)"; + } protected: DremioFlightSqlQuirks quirks_; diff --git a/c/driver/flightsql/sqlite_flightsql_test.cc b/c/driver/flightsql/sqlite_flightsql_test.cc index 41a7dedf1c..a736d25398 100644 --- a/c/driver/flightsql/sqlite_flightsql_test.cc +++ b/c/driver/flightsql/sqlite_flightsql_test.cc @@ -274,6 +274,12 @@ class SqliteFlightSqlStatementTest : public ::testing::Test, void TestSqlIngestInterval() { GTEST_SKIP() << "Cannot ingest Interval (not implemented)"; } + void TestSqlQueryRowsAffectedDelete() { + GTEST_SKIP() << "Cannot query rows affected in delete (not implemented)"; + } + void TestSqlQueryRowsAffectedDeleteStream() { + GTEST_SKIP() << "Cannot query rows affected in delete stream (not implemented)"; + } protected: SqliteFlightSqlQuirks quirks_; diff --git a/c/driver/postgresql/postgresql_test.cc b/c/driver/postgresql/postgresql_test.cc index 932e685e4e..d762ef5b8b 100644 --- a/c/driver/postgresql/postgresql_test.cc +++ b/c/driver/postgresql/postgresql_test.cc @@ -816,6 +816,12 @@ class PostgresStatementTest : public ::testing::Test, void TestSqlPrepareErrorParamCountMismatch() { GTEST_SKIP() << "Not yet implemented"; } void TestSqlPrepareGetParameterSchema() { GTEST_SKIP() << "Not yet implemented"; } void TestSqlPrepareSelectParams() { GTEST_SKIP() << "Not yet implemented"; } + void TestSqlQueryRowsAffectedDelete() { + GTEST_SKIP() << "Cannot query rows affected in delete (not implemented)"; + } + void TestSqlQueryRowsAffectedDeleteStream() { + GTEST_SKIP() << "Cannot query rows affected in delete stream (not implemented)"; + } void TestConcurrentStatements() { // TODO: refactor driver so that we read all the data as soon as diff --git a/c/driver/sqlite/sqlite.c b/c/driver/sqlite/sqlite.c index 05243c03cb..e492801832 100644 --- a/c/driver/sqlite/sqlite.c +++ b/c/driver/sqlite/sqlite.c @@ -1364,7 +1364,13 @@ AdbcStatusCode SqliteStatementExecuteQuery(struct AdbcStatement* statement, sqlite3_mutex_leave(sqlite3_db_mutex(stmt->conn)); AdbcSqliteBinderRelease(&stmt->binder); - if (rows_affected) *rows_affected = rows; + if (rows_affected) { + if (sqlite3_column_count(stmt->stmt) == 0) { + *rows_affected = sqlite3_changes(stmt->conn); + } else { + *rows_affected = rows; + } + } return status; } diff --git a/c/integration/duckdb/duckdb_test.cc b/c/integration/duckdb/duckdb_test.cc index a6bded030c..37bb03e563 100644 --- a/c/integration/duckdb/duckdb_test.cc +++ b/c/integration/duckdb/duckdb_test.cc @@ -101,6 +101,12 @@ class DuckDbStatementTest : public ::testing::Test, } void TestSqlQueryErrors() { GTEST_SKIP() << "DuckDB does not set AdbcError.release"; } + void TestSqlQueryRowsAffectedDelete() { + GTEST_SKIP() << "Cannot query rows affected in delete (not implemented)"; + } + void TestSqlQueryRowsAffectedDeleteStream() { + GTEST_SKIP() << "Cannot query rows affected in delete stream (not implemented)"; + } void TestErrorCompatibility() { GTEST_SKIP() << "DuckDB does not set AdbcError.release"; diff --git a/c/validation/adbc_validation.cc b/c/validation/adbc_validation.cc index c2f32ff502..6dd3fb7b56 100644 --- a/c/validation/adbc_validation.cc +++ b/c/validation/adbc_validation.cc @@ -3434,6 +3434,66 @@ void StatementTest::TestSqlQueryTrailingSemicolons() { ASSERT_THAT(AdbcStatementRelease(&statement, &error), IsOkStatus(&error)); } +void StatementTest::TestSqlQueryRowsAffectedDelete() { + ASSERT_THAT(quirks()->DropTable(&connection, "delete_test", &error), + IsOkStatus(&error)); + ASSERT_THAT(AdbcStatementNew(&connection, &statement, &error), IsOkStatus(&error)); + + ASSERT_THAT(AdbcStatementSetSqlQuery(&statement, + "CREATE TABLE delete_test (foo INT)", &error), + IsOkStatus(&error)); + ASSERT_THAT(AdbcStatementExecuteQuery(&statement, nullptr, nullptr, &error), + IsOkStatus(&error)); + + ASSERT_THAT(AdbcStatementSetSqlQuery(&statement, + "INSERT INTO delete_test (foo) VALUES (1), (2), (3), (4), (5)", &error), + IsOkStatus(&error)); + ASSERT_THAT(AdbcStatementExecuteQuery(&statement, nullptr, nullptr, &error), + IsOkStatus(&error)); + + ASSERT_THAT(AdbcStatementSetSqlQuery(&statement, + "DELETE FROM delete_test WHERE foo >= 3", &error), + IsOkStatus(&error)); + + int64_t rows_affected = 0; + ASSERT_THAT(AdbcStatementExecuteQuery(&statement, nullptr, &rows_affected, &error), + IsOkStatus(&error)); + ASSERT_THAT(rows_affected, + ::testing::AnyOf(::testing::Eq(3), ::testing::Eq(-1))); +} + +void StatementTest::TestSqlQueryRowsAffectedDeleteStream() { + ASSERT_THAT(quirks()->DropTable(&connection, "delete_test", &error), + IsOkStatus(&error)); + ASSERT_THAT(AdbcStatementNew(&connection, &statement, &error), IsOkStatus(&error)); + + ASSERT_THAT(AdbcStatementSetSqlQuery(&statement, + "CREATE TABLE delete_test (foo INT)", &error), + IsOkStatus(&error)); + ASSERT_THAT(AdbcStatementExecuteQuery(&statement, nullptr, nullptr, &error), + IsOkStatus(&error)); + + ASSERT_THAT(AdbcStatementSetSqlQuery(&statement, + "INSERT INTO delete_test (foo) VALUES (1), (2), (3), (4), (5)", &error), + IsOkStatus(&error)); + ASSERT_THAT(AdbcStatementExecuteQuery(&statement, nullptr, nullptr, &error), + IsOkStatus(&error)); + + ASSERT_THAT(AdbcStatementSetSqlQuery(&statement, + "DELETE FROM delete_test WHERE foo >= 3", &error), + IsOkStatus(&error)); + + adbc_validation::StreamReader reader; + ASSERT_THAT(AdbcStatementExecuteQuery(&statement, &reader.stream.value, + &reader.rows_affected, &error), + IsOkStatus(&error)); + ASSERT_THAT(reader.rows_affected, + ::testing::AnyOf(::testing::Eq(5), ::testing::Eq(-1))); +} + + + + void StatementTest::TestTransactions() { if (!quirks()->supports_transactions() || quirks()->ddl_implicit_commit_txn()) { GTEST_SKIP(); diff --git a/c/validation/adbc_validation.h b/c/validation/adbc_validation.h index d125d1be10..2e4c894d8e 100644 --- a/c/validation/adbc_validation.h +++ b/c/validation/adbc_validation.h @@ -365,6 +365,8 @@ class StatementTest { void TestSqlQueryCancel(); void TestSqlQueryErrors(); void TestSqlQueryTrailingSemicolons(); + void TestSqlQueryRowsAffectedDelete(); + void TestSqlQueryRowsAffectedDeleteStream(); void TestSqlSchemaInts(); void TestSqlSchemaFloats(); @@ -455,6 +457,10 @@ class StatementTest { TEST_F(FIXTURE, SqlQueryCancel) { TestSqlQueryCancel(); } \ TEST_F(FIXTURE, SqlQueryErrors) { TestSqlQueryErrors(); } \ TEST_F(FIXTURE, SqlQueryTrailingSemicolons) { TestSqlQueryTrailingSemicolons(); } \ + TEST_F(FIXTURE, SqlQueryRowsAffectedDelete) { TestSqlQueryRowsAffectedDelete(); } \ + TEST_F(FIXTURE, SqlQueryRowsAffectedDeleteStream) { \ + TestSqlQueryRowsAffectedDeleteStream(); \ + } \ TEST_F(FIXTURE, SqlSchemaInts) { TestSqlSchemaInts(); } \ TEST_F(FIXTURE, SqlSchemaFloats) { TestSqlSchemaFloats(); } \ TEST_F(FIXTURE, SqlSchemaStrings) { TestSqlSchemaStrings(); } \ From 4a348962a33031fe51b662b1151b6a98f96b5e24 Mon Sep 17 00:00:00 2001 From: "dependabot[bot]" <49699333+dependabot[bot]@users.noreply.github.com> Date: Wed, 25 Oct 2023 18:17:07 -0400 Subject: [PATCH 3/7] chore(go/adbc): bump google.golang.org/grpc from 1.58.2 to 1.58.3 in /go/adbc (#1227) Bumps [google.golang.org/grpc](https://github.com/grpc/grpc-go) from 1.58.2 to 1.58.3.
Release notes

Sourced from google.golang.org/grpc's releases.

Release 1.58.3

Security

  • server: prohibit more than MaxConcurrentStreams handlers from running at once (CVE-2023-44487)

    In addition to this change, applications should ensure they do not leave running tasks behind related to the RPC before returning from method handlers, or should enforce appropriate limits on any such work.

Commits

[![Dependabot compatibility score](https://dependabot-badges.githubapp.com/badges/compatibility_score?dependency-name=google.golang.org/grpc&package-manager=go_modules&previous-version=1.58.2&new-version=1.58.3)](https://docs.github.com/en/github/managing-security-vulnerabilities/about-dependabot-security-updates#about-compatibility-scores) Dependabot will resolve any conflicts with this PR as long as you don't alter it yourself. You can also trigger a rebase manually by commenting `@dependabot rebase`. [//]: # (dependabot-automerge-start) [//]: # (dependabot-automerge-end) ---
Dependabot commands and options
You can trigger Dependabot actions by commenting on this PR: - `@dependabot rebase` will rebase this PR - `@dependabot recreate` will recreate this PR, overwriting any edits that have been made to it - `@dependabot merge` will merge this PR after your CI passes on it - `@dependabot squash and merge` will squash and merge this PR after your CI passes on it - `@dependabot cancel merge` will cancel a previously requested merge and block automerging - `@dependabot reopen` will reopen this PR if it is closed - `@dependabot close` will close this PR and stop Dependabot recreating it. You can achieve the same result by closing it manually - `@dependabot show ignore conditions` will show all of the ignore conditions of the specified dependency - `@dependabot ignore this major version` will close this PR and stop Dependabot creating any more for this major version (unless you reopen the PR or upgrade to it yourself) - `@dependabot ignore this minor version` will close this PR and stop Dependabot creating any more for this minor version (unless you reopen the PR or upgrade to it yourself) - `@dependabot ignore this dependency` will close this PR and stop Dependabot creating any more for this dependency (unless you reopen the PR or upgrade to it yourself) You can disable automated security fix PRs for this repo from the [Security Alerts page](https://github.com/apache/arrow-adbc/network/alerts).
Signed-off-by: dependabot[bot] Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> --- go/adbc/go.mod | 2 +- go/adbc/go.sum | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/go/adbc/go.mod b/go/adbc/go.mod index 92f934d348..b9e3825ddf 100644 --- a/go/adbc/go.mod +++ b/go/adbc/go.mod @@ -29,7 +29,7 @@ require ( golang.org/x/exp v0.0.0-20230713183714-613f0c0eb8a1 golang.org/x/sync v0.3.0 golang.org/x/tools v0.13.0 - google.golang.org/grpc v1.58.2 + google.golang.org/grpc v1.58.3 google.golang.org/protobuf v1.31.0 ) diff --git a/go/adbc/go.sum b/go/adbc/go.sum index 76df64547c..a7645a8fc3 100644 --- a/go/adbc/go.sum +++ b/go/adbc/go.sum @@ -172,8 +172,8 @@ golang.org/x/xerrors v0.0.0-20220907171357-04be3eba64a2/go.mod h1:K8+ghG5WaK9qNq gonum.org/v1/gonum v0.12.0 h1:xKuo6hzt+gMav00meVPUlXwSdoEJP46BR+wdxQEFK2o= google.golang.org/genproto/googleapis/rpc v0.0.0-20231002182017-d307bd883b97 h1:6GQBEOdGkX6MMTLT9V+TjtIRZCw9VPD5Z+yHY9wMgS0= google.golang.org/genproto/googleapis/rpc v0.0.0-20231002182017-d307bd883b97/go.mod h1:v7nGkzlmW8P3n/bKmWBn2WpBjpOEx8Q6gMueudAmKfY= -google.golang.org/grpc v1.58.2 h1:SXUpjxeVF3FKrTYQI4f4KvbGD5u2xccdYdurwowix5I= -google.golang.org/grpc v1.58.2/go.mod h1:tgX3ZQDlNJGU96V6yHh1T/JeoBQ2TXdr43YbYSsCJk0= +google.golang.org/grpc v1.58.3 h1:BjnpXut1btbtgN/6sp+brB2Kbm2LjNXnidYujAVbSoQ= +google.golang.org/grpc v1.58.3/go.mod h1:tgX3ZQDlNJGU96V6yHh1T/JeoBQ2TXdr43YbYSsCJk0= google.golang.org/protobuf v1.26.0-rc.1/go.mod h1:jlhhOSvTdKEhbULTjvd4ARK9grFBp09yW+WbY/TyQbw= google.golang.org/protobuf v1.26.0/go.mod h1:9q0QmTI4eRPtz6boOQmLYwt+qCgq0jsYwAQnmE0givc= google.golang.org/protobuf v1.31.0 h1:g0LDEJHgrBl9N9r17Ru3sqWhkIx2NB67okBHPwC7hs8= From 1bd874b8e9d4359e5ca5f86f0111b724cfc5d299 Mon Sep 17 00:00:00 2001 From: David Li Date: Thu, 26 Oct 2023 13:06:00 -0400 Subject: [PATCH 4/7] fix(go/adbc/driver/flightsql): take metadata lock for safety (#1228) Fixes #1178. --- go/adbc/driver/flightsql/flightsql_database.go | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/go/adbc/driver/flightsql/flightsql_database.go b/go/adbc/driver/flightsql/flightsql_database.go index 250a2f045c..b671c8a86b 100644 --- a/go/adbc/driver/flightsql/flightsql_database.go +++ b/go/adbc/driver/flightsql/flightsql_database.go @@ -382,7 +382,8 @@ func getFlightClient(ctx context.Context, loc string, d *databaseImpl) (*flights } if md, ok := metadata.FromOutgoingContext(ctx); ok { - // No need to worry about lock here since we are sole owner + authMiddle.mutex.Lock() + defer authMiddle.mutex.Unlock() authMiddle.hdrs.Set("authorization", md.Get("Authorization")[0]) } } From 17898709be7a8a207726beb083afcc9d6d2c0d3d Mon Sep 17 00:00:00 2001 From: Dewey Dunnington Date: Thu, 26 Oct 2023 19:47:35 +0000 Subject: [PATCH 5/7] feat(c/driver/sqlite): Support binding dictionary-encoded string and binary types (#1224) This PR adds the ability to ingest dictionary-encoded string and binary columns. Part of addressing #1008. From the R bindings: ``` r library(adbcdrivermanager) db <- adbc_database_init(adbcsqlite::adbcsqlite(), uri = ":memory:") con <- adbc_connection_init(db) df <- data.frame(x = factor(letters[1:10])) write_adbc(df, con, "tbl") read_adbc(con, "SELECT * from tbl") |> as.data.frame() #> x #> 1 a #> 2 b #> 3 c #> 4 d #> 5 e #> 6 f #> 7 g #> 8 h #> 9 i #> 10 j ``` Created on 2023-10-25 with [reprex v2.0.2](https://reprex.tidyverse.org) --- c/driver/postgresql/postgresql_test.cc | 1 + c/driver/sqlite/sqlite_test.cc | 2 +- c/driver/sqlite/statement_reader.c | 41 ++++++++++++++++- c/validation/adbc_validation.cc | 64 ++++++++++++++++++++------ c/validation/adbc_validation.h | 7 ++- 5 files changed, 98 insertions(+), 17 deletions(-) diff --git a/c/driver/postgresql/postgresql_test.cc b/c/driver/postgresql/postgresql_test.cc index d762ef5b8b..f6df18093f 100644 --- a/c/driver/postgresql/postgresql_test.cc +++ b/c/driver/postgresql/postgresql_test.cc @@ -812,6 +812,7 @@ class PostgresStatementTest : public ::testing::Test, void TestSqlIngestUInt16() { GTEST_SKIP() << "Not implemented"; } void TestSqlIngestUInt32() { GTEST_SKIP() << "Not implemented"; } void TestSqlIngestUInt64() { GTEST_SKIP() << "Not implemented"; } + void TestSqlIngestStringDictionary() { GTEST_SKIP() << "Not implemented"; } void TestSqlPrepareErrorParamCountMismatch() { GTEST_SKIP() << "Not yet implemented"; } void TestSqlPrepareGetParameterSchema() { GTEST_SKIP() << "Not yet implemented"; } diff --git a/c/driver/sqlite/sqlite_test.cc b/c/driver/sqlite/sqlite_test.cc index f4455a572e..13da21c10c 100644 --- a/c/driver/sqlite/sqlite_test.cc +++ b/c/driver/sqlite/sqlite_test.cc @@ -246,7 +246,7 @@ class SqliteStatementTest : public ::testing::Test, void TestSqlIngestUInt64() { std::vector> values = {std::nullopt, 0, INT64_MAX}; - return TestSqlIngestType(NANOARROW_TYPE_UINT64, values); + return TestSqlIngestType(NANOARROW_TYPE_UINT64, values, /*dictionary_encode*/ false); } void TestSqlIngestDuration() { diff --git a/c/driver/sqlite/statement_reader.c b/c/driver/sqlite/statement_reader.c index e3b2525b1b..9e02ee3b80 100644 --- a/c/driver/sqlite/statement_reader.c +++ b/c/driver/sqlite/statement_reader.c @@ -60,7 +60,7 @@ AdbcStatusCode AdbcSqliteBinderSet(struct AdbcSqliteBinder* binder, struct ArrowSchemaView view = {0}; for (int i = 0; i < binder->schema.n_children; i++) { status = ArrowSchemaViewInit(&view, binder->schema.children[i], &arrow_error); - if (status != 0) { + if (status != NANOARROW_OK) { SetError(error, "Failed to parse schema for column %d: %s (%d): %s", i, strerror(status), status, arrow_error.message); return ADBC_STATUS_INVALID_ARGUMENT; @@ -70,6 +70,31 @@ AdbcStatusCode AdbcSqliteBinderSet(struct AdbcSqliteBinder* binder, SetError(error, "Column %d has UNINITIALIZED type", i); return ADBC_STATUS_INTERNAL; } + + if (view.type == NANOARROW_TYPE_DICTIONARY) { + struct ArrowSchemaView value_view = {0}; + status = ArrowSchemaViewInit(&value_view, binder->schema.children[i]->dictionary, + &arrow_error); + if (status != NANOARROW_OK) { + SetError(error, "Failed to parse schema for column %d->dictionary: %s (%d): %s", + i, strerror(status), status, arrow_error.message); + return ADBC_STATUS_INVALID_ARGUMENT; + } + + // We only support string/binary dictionary-encoded values + switch (value_view.type) { + case NANOARROW_TYPE_STRING: + case NANOARROW_TYPE_LARGE_STRING: + case NANOARROW_TYPE_BINARY: + case NANOARROW_TYPE_LARGE_BINARY: + break; + default: + SetError(error, "Column %d dictionary has unsupported type %s", i, + ArrowTypeString(value_view.type)); + return ADBC_STATUS_NOT_IMPLEMENTED; + } + } + binder->types[i] = view.type; } @@ -353,6 +378,20 @@ AdbcStatusCode AdbcSqliteBinderBindNext(struct AdbcSqliteBinder* binder, sqlite3 SQLITE_STATIC); break; } + case NANOARROW_TYPE_DICTIONARY: { + int64_t value_index = + ArrowArrayViewGetIntUnsafe(binder->batch.children[col], binder->next_row); + if (ArrowArrayViewIsNull(binder->batch.children[col]->dictionary, + value_index)) { + status = sqlite3_bind_null(stmt, col + 1); + } else { + struct ArrowBufferView value = ArrowArrayViewGetBytesUnsafe( + binder->batch.children[col]->dictionary, value_index); + status = sqlite3_bind_text(stmt, col + 1, value.data.as_char, + value.size_bytes, SQLITE_STATIC); + } + break; + } case NANOARROW_TYPE_DATE32: { int64_t value = ArrowArrayViewGetIntUnsafe(binder->batch.children[col], binder->next_row); diff --git a/c/validation/adbc_validation.cc b/c/validation/adbc_validation.cc index 6dd3fb7b56..f0f42937f8 100644 --- a/c/validation/adbc_validation.cc +++ b/c/validation/adbc_validation.cc @@ -1366,7 +1366,8 @@ void StatementTest::TestRelease() { template void StatementTest::TestSqlIngestType(ArrowType type, - const std::vector>& values) { + const std::vector>& values, + bool dictionary_encode) { if (!quirks()->supports_bulk_ingest(ADBC_INGEST_OPTION_MODE_CREATE)) { GTEST_SKIP(); } @@ -1381,6 +1382,38 @@ void StatementTest::TestSqlIngestType(ArrowType type, ASSERT_THAT(MakeBatch(&schema.value, &array.value, &na_error, values), IsOkErrno()); + if (dictionary_encode) { + // Create a dictionary-encoded version of the target schema + Handle dict_schema; + ASSERT_THAT(ArrowSchemaInitFromType(&dict_schema.value, NANOARROW_TYPE_INT32), + IsOkErrno()); + ASSERT_THAT(ArrowSchemaSetName(&dict_schema.value, schema.value.children[0]->name), + IsOkErrno()); + ASSERT_THAT(ArrowSchemaSetName(schema.value.children[0], nullptr), IsOkErrno()); + + // Swap it into the target schema + ASSERT_THAT(ArrowSchemaAllocateDictionary(&dict_schema.value), IsOkErrno()); + ArrowSchemaMove(schema.value.children[0], dict_schema.value.dictionary); + ArrowSchemaMove(&dict_schema.value, schema.value.children[0]); + + // Create a dictionary-encoded array with easy 0...n indices so that the + // matched values will be the same. + Handle dict_array; + ASSERT_THAT(ArrowArrayInitFromType(&dict_array.value, NANOARROW_TYPE_INT32), + IsOkErrno()); + ASSERT_THAT(ArrowArrayStartAppending(&dict_array.value), IsOkErrno()); + for (size_t i = 0; i < values.size(); i++) { + ASSERT_THAT(ArrowArrayAppendInt(&dict_array.value, static_cast(i)), + IsOkErrno()); + } + ASSERT_THAT(ArrowArrayFinishBuildingDefault(&dict_array.value, nullptr), IsOkErrno()); + + // Swap it into the target batch + ASSERT_THAT(ArrowArrayAllocateDictionary(&dict_array.value), IsOkErrno()); + ArrowArrayMove(array.value.children[0], dict_array.value.dictionary); + ArrowArrayMove(&dict_array.value, array.value.children[0]); + } + ASSERT_THAT(AdbcStatementNew(&connection, &statement, &error), IsOkStatus(&error)); ASSERT_THAT(AdbcStatementSetOption(&statement, ADBC_INGEST_OPTION_TARGET_TABLE, "bulk_ingest", &error), @@ -1448,7 +1481,7 @@ void StatementTest::TestSqlIngestNumericType(ArrowType type) { values.push_back(std::numeric_limits::max()); } - return TestSqlIngestType(type, values); + return TestSqlIngestType(type, values, false); } void StatementTest::TestSqlIngestBool() { @@ -1497,25 +1530,23 @@ void StatementTest::TestSqlIngestFloat64() { void StatementTest::TestSqlIngestString() { ASSERT_NO_FATAL_FAILURE(TestSqlIngestType( - NANOARROW_TYPE_STRING, {std::nullopt, "", "", "1234", "例"})); + NANOARROW_TYPE_STRING, {std::nullopt, "", "", "1234", "例"}, false)); } void StatementTest::TestSqlIngestLargeString() { ASSERT_NO_FATAL_FAILURE(TestSqlIngestType( - NANOARROW_TYPE_LARGE_STRING, {std::nullopt, "", "", "1234", "例"})); + NANOARROW_TYPE_LARGE_STRING, {std::nullopt, "", "", "1234", "例"}, false)); } void StatementTest::TestSqlIngestBinary() { ASSERT_NO_FATAL_FAILURE(TestSqlIngestType>( NANOARROW_TYPE_BINARY, - { - std::nullopt, std::vector{}, - std::vector{std::byte{0x00}, std::byte{0x01}}, - std::vector{ - std::byte{0x01}, std::byte{0x02}, std::byte{0x03}, std::byte{0x04} - }, - std::vector{std::byte{0xfe}, std::byte{0xff}} - })); + {std::nullopt, std::vector{}, + std::vector{std::byte{0x00}, std::byte{0x01}}, + std::vector{std::byte{0x01}, std::byte{0x02}, std::byte{0x03}, + std::byte{0x04}}, + std::vector{std::byte{0xfe}, std::byte{0xff}}}, + false)); } void StatementTest::TestSqlIngestDate32() { @@ -1737,6 +1768,12 @@ void StatementTest::TestSqlIngestInterval() { ASSERT_THAT(AdbcStatementRelease(&statement, &error), IsOkStatus(&error)); } +void StatementTest::TestSqlIngestStringDictionary() { + ASSERT_NO_FATAL_FAILURE(TestSqlIngestType( + NANOARROW_TYPE_STRING, {std::nullopt, "", "", "1234", "例"}, + /*dictionary_encode*/ true)); +} + void StatementTest::TestSqlIngestTableEscaping() { std::string name = "create_table_escaping"; @@ -2112,8 +2149,7 @@ void StatementTest::TestSqlIngestErrors() { {"coltwo", NANOARROW_TYPE_INT64}}), IsOkErrno()); ASSERT_THAT( - (MakeBatch(&schema.value, &array.value, &na_error, - {-42}, {-42})), + (MakeBatch(&schema.value, &array.value, &na_error, {-42}, {-42})), IsOkErrno(&na_error)); ASSERT_THAT(AdbcStatementBind(&statement, &array.value, &schema.value, &error), diff --git a/c/validation/adbc_validation.h b/c/validation/adbc_validation.h index 2e4c894d8e..e2b5d434d2 100644 --- a/c/validation/adbc_validation.h +++ b/c/validation/adbc_validation.h @@ -327,6 +327,9 @@ class StatementTest { void TestSqlIngestTimestampTz(); void TestSqlIngestInterval(); + // Dictionary-encoded + void TestSqlIngestStringDictionary(); + // ---- End Type-specific tests ---------------- void TestSqlIngestTableEscaping(); @@ -387,7 +390,8 @@ class StatementTest { struct AdbcStatement statement; template - void TestSqlIngestType(ArrowType type, const std::vector>& values); + void TestSqlIngestType(ArrowType type, const std::vector>& values, + bool dictionary_encode); template void TestSqlIngestNumericType(ArrowType type); @@ -424,6 +428,7 @@ class StatementTest { TEST_F(FIXTURE, SqlIngestTimestamp) { TestSqlIngestTimestamp(); } \ TEST_F(FIXTURE, SqlIngestTimestampTz) { TestSqlIngestTimestampTz(); } \ TEST_F(FIXTURE, SqlIngestInterval) { TestSqlIngestInterval(); } \ + TEST_F(FIXTURE, SqlIngestStringDictionary) { TestSqlIngestStringDictionary(); } \ TEST_F(FIXTURE, SqlIngestTableEscaping) { TestSqlIngestTableEscaping(); } \ TEST_F(FIXTURE, SqlIngestColumnEscaping) { TestSqlIngestColumnEscaping(); } \ TEST_F(FIXTURE, SqlIngestAppend) { TestSqlIngestAppend(); } \ From ef72e6fd6b691f88da6f57e3bc6bdd12329ff854 Mon Sep 17 00:00:00 2001 From: Dewey Dunnington Date: Thu, 26 Oct 2023 20:15:08 +0000 Subject: [PATCH 6/7] refactor(r): Improve testing for ADBC 1.1 features in R bindings (#1214) This PR implements option setting/getting in the "void" driver and implements tests for the full grid of set/get by string/bytes/integer/double by database/connection/statement. The error detail information was also not implemented in the dummy driver and so couldn't be tested (so there is an implementation of that here). Implementing a driver that actually did this was sufficient work that I did some rather heavy abstraction to make it easier to write. That abstraction is not unlike a "driver framework" except it is (1) not complete, since the void driver only needs options methods and (2) doesn't provide any result helpers (building streams, etc.). It might be worth porting the driver base to be more general but for now I'd like to keep it constrained to what I need to test in R. Closes #1126. --- c/driver_manager/adbc_driver_manager.cc | 6 + go/adbc/drivermgr/adbc_driver_manager.cc | 6 + r/adbcdrivermanager/R/error.R | 8 +- r/adbcdrivermanager/R/options.R | 74 ++- r/adbcdrivermanager/R/utils.R | 17 - r/adbcdrivermanager/src/driver_base.h | 565 ++++++++++++++++++ r/adbcdrivermanager/src/driver_void.c | 316 ---------- r/adbcdrivermanager/src/driver_void.cc | 41 ++ r/adbcdrivermanager/src/error.cc | 12 +- r/adbcdrivermanager/src/init.c | 31 +- r/adbcdrivermanager/src/options.cc | 111 ++-- r/adbcdrivermanager/src/radbc.cc | 35 +- .../tests/testthat/test-error.R | 32 +- .../tests/testthat/test-options.R | 304 +++++++++- .../tests/testthat/test-utils.R | 32 - 15 files changed, 1089 insertions(+), 501 deletions(-) create mode 100644 r/adbcdrivermanager/src/driver_base.h delete mode 100644 r/adbcdrivermanager/src/driver_void.c create mode 100644 r/adbcdrivermanager/src/driver_void.cc diff --git a/c/driver_manager/adbc_driver_manager.cc b/c/driver_manager/adbc_driver_manager.cc index 9632660599..85110c6424 100644 --- a/c/driver_manager/adbc_driver_manager.cc +++ b/c/driver_manager/adbc_driver_manager.cc @@ -457,6 +457,11 @@ AdbcStatusCode StatementBind(struct AdbcStatement*, struct ArrowArray*, return ADBC_STATUS_NOT_IMPLEMENTED; } +AdbcStatusCode StatementBindStream(struct AdbcStatement*, struct ArrowArrayStream*, + struct AdbcError* error) { + return ADBC_STATUS_NOT_IMPLEMENTED; +} + AdbcStatusCode StatementCancel(struct AdbcStatement* statement, struct AdbcError* error) { return ADBC_STATUS_NOT_IMPLEMENTED; } @@ -1666,6 +1671,7 @@ AdbcStatusCode AdbcLoadDriverFromInitFunc(AdbcDriverInitFunc init_func, int vers CHECK_REQUIRED(driver, StatementNew); CHECK_REQUIRED(driver, StatementRelease); FILL_DEFAULT(driver, StatementBind); + FILL_DEFAULT(driver, StatementBindStream); FILL_DEFAULT(driver, StatementGetParameterSchema); FILL_DEFAULT(driver, StatementPrepare); FILL_DEFAULT(driver, StatementSetOption); diff --git a/go/adbc/drivermgr/adbc_driver_manager.cc b/go/adbc/drivermgr/adbc_driver_manager.cc index 9632660599..85110c6424 100644 --- a/go/adbc/drivermgr/adbc_driver_manager.cc +++ b/go/adbc/drivermgr/adbc_driver_manager.cc @@ -457,6 +457,11 @@ AdbcStatusCode StatementBind(struct AdbcStatement*, struct ArrowArray*, return ADBC_STATUS_NOT_IMPLEMENTED; } +AdbcStatusCode StatementBindStream(struct AdbcStatement*, struct ArrowArrayStream*, + struct AdbcError* error) { + return ADBC_STATUS_NOT_IMPLEMENTED; +} + AdbcStatusCode StatementCancel(struct AdbcStatement* statement, struct AdbcError* error) { return ADBC_STATUS_NOT_IMPLEMENTED; } @@ -1666,6 +1671,7 @@ AdbcStatusCode AdbcLoadDriverFromInitFunc(AdbcDriverInitFunc init_func, int vers CHECK_REQUIRED(driver, StatementNew); CHECK_REQUIRED(driver, StatementRelease); FILL_DEFAULT(driver, StatementBind); + FILL_DEFAULT(driver, StatementBindStream); FILL_DEFAULT(driver, StatementGetParameterSchema); FILL_DEFAULT(driver, StatementPrepare); FILL_DEFAULT(driver, StatementSetOption); diff --git a/r/adbcdrivermanager/R/error.R b/r/adbcdrivermanager/R/error.R index 565f509612..6ea20ec87d 100644 --- a/r/adbcdrivermanager/R/error.R +++ b/r/adbcdrivermanager/R/error.R @@ -42,8 +42,12 @@ adbc_error_from_array_stream <- function(stream) { .Call(RAdbcErrorFromArrayStream, stream) } -adbc_allocate_error <- function(shelter = NULL) { - .Call(RAdbcAllocateError, shelter) +adbc_allocate_error <- function(shelter = NULL, use_legacy_error = NULL) { + if (is.null(use_legacy_error)) { + use_legacy_error <- getOption("adbcdrivermanager.use_legacy_error", FALSE) + } + + .Call(RAdbcAllocateError, shelter, use_legacy_error) } stop_for_error <- function(status, error) { diff --git a/r/adbcdrivermanager/R/options.R b/r/adbcdrivermanager/R/options.R index be258bbd4b..e47437c167 100644 --- a/r/adbcdrivermanager/R/options.R +++ b/r/adbcdrivermanager/R/options.R @@ -22,7 +22,7 @@ adbc_database_set_options <- function(database, options) { error <- adbc_allocate_error() for (i in seq_along(options)) { key <- names(options)[i] - value <- options[i] + value <- options[[i]] status <- .Call( RAdbcDatabaseSetOption, database, @@ -42,7 +42,7 @@ adbc_connection_set_options <- function(connection, options) { error <- adbc_allocate_error() for (i in seq_along(options)) { key <- names(options)[i] - value <- options[i] + value <- options[[i]] status <- .Call( RAdbcConnectionSetOption, connection, @@ -62,7 +62,7 @@ adbc_statement_set_options <- function(statement, options) { error <- adbc_allocate_error() for (i in seq_along(options)) { key <- names(options)[i] - value <- options[i] + value <- options[[i]] status <- .Call( RAdbcStatementSetOption, statement, @@ -160,3 +160,71 @@ adbc_statement_get_option_double <- function(statement, option) { error <- adbc_allocate_error() .Call(RAdbcStatementGetOptionDouble, statement, option, error) } + +# Ensures that options are a list of bare character, raw, integer, or double +key_value_options <- function(options) { + options <- as.list(options) + + if (length(options) == 0) { + names(options) <- character() + } else if (is.null(names(options))) { + # OK to have no names, because options could contain a series of + # adbc_options() objects that will be concatenated + names(options) <- rep("", length(options)) + } + + out <- vector("list", 10L) + out_names <- character(10L) + n_out <- 0L + + for (i in seq_along(options)) { + key <- names(options)[[i]] + item <- options[[i]] + + # Skip NULL item + if (is.null(item)) { + next + } + + # Append all items of an existing adbc_options + if (inherits(item, "adbc_options")) { + out <- c(out[seq_len(n_out)], item) + out_names <- c(out_names[seq_len(n_out)], names(item)) + n_out <- n_out + length(item) + next + } + + # Otherwise, append a single value (coercing to character if item + # is an S3 object) + if (is.object(item)) { + item <- as.character(item) + } + + if (identical(key, "") || identical(key, NA_character_)) { + stop("key/value options must be named") + } + + n_out <- n_out + 1L + out_names[n_out] <- key + if (is.character(item)) { + out[[n_out]] <- item + } else if (is.integer(item)) { + out[[n_out]] <- as.integer(item) + } else if (is.double(item)) { + out[[n_out]] <- as.double(item) + } else if (is.raw(item)) { + out[[n_out]] <- item + } else { + stop( + sprintf( + "Option of type '%s' (key: '%s') not supported", + typeof(item), + key + ) + ) + } + } + + names(out) <- out_names + structure(out[seq_len(n_out)], class = "adbc_options") +} diff --git a/r/adbcdrivermanager/R/utils.R b/r/adbcdrivermanager/R/utils.R index 6caa8228e1..1d7a01be1d 100644 --- a/r/adbcdrivermanager/R/utils.R +++ b/r/adbcdrivermanager/R/utils.R @@ -15,23 +15,6 @@ # specific language governing permissions and limitations # under the License. -key_value_options <- function(options) { - if (!is.character(options)) { - options <- as.list(options) - options <- options[!vapply(options, is.null, logical(1))] - options <- vapply(options, as.character, character(1)) - } - - keys <- names(options) - if (length(options) == 0) { - names(options) <- character() - } else if (is.null(keys) || all(keys == "")) { - stop("key/value options must be named") - } - - options -} - new_env <- function() { new.env(parent = emptyenv()) } diff --git a/r/adbcdrivermanager/src/driver_base.h b/r/adbcdrivermanager/src/driver_base.h new file mode 100644 index 0000000000..6f7f4e4200 --- /dev/null +++ b/r/adbcdrivermanager/src/driver_base.h @@ -0,0 +1,565 @@ +// 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 +#include +#include +#include +#include +#include +#include + +#include + +// This file defines a developer-friendly way to create an ADBC driver, currently intended +// for testing the R driver manager. It handles errors, option getting/setting, and +// managing the export of the many C callables that compose an AdbcDriver. In general, +// functions or methods intended to be called from C are prefixed with "C" and are private +// (i.e., the public and protected methods are the only ones that driver authors should +// ever interact with). +// +// Example: +// class MyDatabase: public DatabaseObjectBase {}; +// class MyConnection: public ConnectionObjectBase {}; +// class MyStatement: public StatementObjectbase {}; +// AdbcStatusCode VoidDriverInitFunc(int version, void* raw_driver, AdbcError* error) { +// return Driver::Init( +// version, raw_driver, error); +// } + +namespace adbc { + +namespace r { + +class Error { + public: + explicit Error(std::string message) : message_(std::move(message)) { + std::memset(sql_state_, 0, sizeof(sql_state_)); + } + + explicit Error(const char* message) : Error(std::string(message)) {} + + Error(std::string message, std::vector> details) + : message_(std::move(message)), details_(std::move(details)) { + std::memset(sql_state_, 0, sizeof(sql_state_)); + } + + void AddDetail(std::string key, std::string value) { + details_.push_back({std::move(key), std::move(value)}); + } + + void ToAdbc(AdbcError* adbc_error, AdbcDriver* driver = nullptr) { + if (adbc_error->vendor_code == ADBC_ERROR_VENDOR_CODE_PRIVATE_DATA) { + auto error_owned_by_adbc_error = + new Error(std::move(message_), std::move(details_)); + adbc_error->message = + const_cast(error_owned_by_adbc_error->message_.c_str()); + adbc_error->private_data = error_owned_by_adbc_error; + adbc_error->private_driver = driver; + } else { + adbc_error->message = reinterpret_cast(std::malloc(message_.size() + 1)); + if (adbc_error->message != nullptr) { + std::memcpy(adbc_error->message, message_.c_str(), message_.size() + 1); + } + } + + std::memcpy(adbc_error->sqlstate, sql_state_, sizeof(sql_state_)); + adbc_error->release = &CRelease; + } + + private: + std::string message_; + std::vector> details_; + char sql_state_[5]; + + // Let the Driver use these to expose C callables wrapping option setters/getters + template + friend class Driver; + + int CDetailCount() const { return details_.size(); } + + AdbcErrorDetail CDetail(int index) const { + const auto& detail = details_[index]; + return {detail.first.c_str(), reinterpret_cast(detail.second.data()), + detail.second.size()}; + } + + static void CRelease(AdbcError* error) { + if (error->vendor_code == ADBC_ERROR_VENDOR_CODE_PRIVATE_DATA) { + auto error_obj = reinterpret_cast(error->private_data); + delete error_obj; + } else { + std::free(error->message); + } + + std::memset(error, 0, sizeof(AdbcError)); + } +}; + +// Variant that handles the option types that can be get/set by databases, +// connections, and statements. It currently does not attempt conversion +// (i.e., getting a double option as a string). +class Option { + public: + enum Type { TYPE_MISSING, TYPE_STRING, TYPE_BYTES, TYPE_INT, TYPE_DOUBLE }; + + Option() : type_(TYPE_MISSING) {} + explicit Option(const std::string& value) : type_(TYPE_STRING), value_string_(value) {} + explicit Option(const std::basic_string& value) + : type_(TYPE_BYTES), value_bytes_(value) {} + explicit Option(double value) : type_(TYPE_DOUBLE), value_double_(value) {} + explicit Option(int64_t value) : type_(TYPE_INT), value_int_(value) {} + + Type type() const { return type_; } + + const std::string& GetStringUnsafe() const { return value_string_; } + + const std::basic_string& GetBytesUnsafe() const { return value_bytes_; } + + int64_t GetIntUnsafe() const { return value_int_; } + + double GetDoubleUnsafe() const { return value_double_; } + + private: + Type type_; + std::string value_string_; + std::basic_string value_bytes_; + double value_double_; + int64_t value_int_; + + // Methods used by trampolines to export option values in C below + friend class ObjectBase; + + AdbcStatusCode CGet(char* out, size_t* length) const { + switch (type_) { + case TYPE_STRING: { + const std::string& value = GetStringUnsafe(); + if (*length < value.size()) { + *length = value.size(); + } else { + memcpy(out, value.data(), value.size()); + } + + return ADBC_STATUS_OK; + } + default: + return ADBC_STATUS_NOT_FOUND; + } + } + + AdbcStatusCode CGet(uint8_t* out, size_t* length) const { + switch (type_) { + case TYPE_BYTES: { + const std::basic_string& value = GetBytesUnsafe(); + if (*length < value.size()) { + *length = value.size(); + } else { + memcpy(out, value.data(), value.size()); + } + + return ADBC_STATUS_OK; + } + default: + return ADBC_STATUS_NOT_FOUND; + } + } + + AdbcStatusCode CGet(int64_t* value) const { + switch (type_) { + case TYPE_INT: + *value = GetIntUnsafe(); + return ADBC_STATUS_OK; + default: + return ADBC_STATUS_NOT_FOUND; + } + } + + AdbcStatusCode CGet(double* value) const { + switch (type_) { + case TYPE_DOUBLE: + *value = GetDoubleUnsafe(); + return ADBC_STATUS_OK; + default: + return ADBC_STATUS_NOT_FOUND; + } + } +}; + +// Base class for private_data of AdbcDatabase, AdbcConnection, and AdbcStatement +// This class handles option setting and getting. +class ObjectBase { + public: + ObjectBase() : driver_(nullptr) {} + + virtual ~ObjectBase() {} + + // Driver authors can override this method to reject options that are not supported or + // that are set at a time not supported by the driver (e.g., to reject options that are + // set after Init() is called if this is not supported). + virtual AdbcStatusCode SetOption(const std::string& key, const Option& value) { + options_[key] = value; + return ADBC_STATUS_OK; + } + + // Called After zero or more SetOption() calls. The parent is the private_data of + // the AdbcDriver, AdbcDatabase, or AdbcConnection when initializing a subclass of + // DatabaseObjectBase, ConnectionObjectBase, and StatementObjectBase (respectively). + // For example, if you have defined Driver, + // you can reinterpret_cast(parent) in MyConnection::Init(). + virtual AdbcStatusCode Init(void* parent, AdbcError* error) { return ADBC_STATUS_OK; } + + // Called when the corresponding AdbcXXXRelease() function is invoked from C. + // Driver authors can override this method to return an error if the object is + // not in a valid state (e.g., if a connection has open statements) or to clean + // up resources when resource cleanup could fail. Resource cleanup that cannot fail + // (e.g., releasing memory) should generally be handled in the deleter. + virtual AdbcStatusCode Release(AdbcError* error) { return ADBC_STATUS_OK; } + + // Get an option that was previously set, providing an optional default value. + const Option& GetOption(const std::string& key, + const Option& default_value = Option()) const { + auto result = options_.find(key); + if (result == options_.end()) { + return default_value; + } else { + return result->second; + } + } + + protected: + // Needed to export errors using Error::ToAdbc() that use 1.1.0 extensions + // (i.e., error details). This will be nullptr before Init() is called. + AdbcDriver* driver() const { return driver_; } + + private: + AdbcDriver* driver_; + std::unordered_map options_; + + // Let the Driver use these to expose C callables wrapping option setters/getters + template + friend class Driver; + + // The AdbcDriver* struct is set right before Init() is called by the Driver + // trampoline. + void set_driver(AdbcDriver* driver) { driver_ = driver; } + + template + AdbcStatusCode CSetOption(const char* key, T value, AdbcError* error) { + Option option(value); + return SetOption(key, option); + } + + AdbcStatusCode CSetOptionBytes(const char* key, const uint8_t* value, size_t length, + AdbcError* error) { + std::basic_string cppvalue(value, length); + Option option(cppvalue); + return SetOption(key, option); + } + + template + AdbcStatusCode CGetOptionStringLike(const char* key, T* value, size_t* length, + AdbcError* error) const { + Option result = GetOption(key); + if (result.type() == Option::TYPE_MISSING) { + InitErrorNotFound(key, error); + return ADBC_STATUS_NOT_FOUND; + } else { + AdbcStatusCode status = result.CGet(value, length); + if (status != ADBC_STATUS_OK) { + InitErrorWrongType(key, error); + } + + return status; + } + } + + template + AdbcStatusCode CGetOptionNumeric(const char* key, T* value, AdbcError* error) const { + Option result = GetOption(key); + if (result.type() == Option::TYPE_MISSING) { + InitErrorNotFound(key, error); + return ADBC_STATUS_NOT_FOUND; + } else { + AdbcStatusCode status = result.CGet(value); + if (status != ADBC_STATUS_OK) { + InitErrorWrongType(key, error); + } + + return status; + } + } + + void InitErrorNotFound(const char* key, AdbcError* error) const { + std::stringstream msg_builder; + msg_builder << "Option not found for key '" << key << "'"; + Error cpperror(msg_builder.str()); + cpperror.AddDetail("adbc.r.option_key", key); + cpperror.ToAdbc(error, driver()); + } + + void InitErrorWrongType(const char* key, AdbcError* error) const { + std::stringstream msg_builder; + msg_builder << "Wrong type requested for option key '" << key << "'"; + Error cpperror(msg_builder.str()); + cpperror.AddDetail("adbc.r.option_key", key); + cpperror.ToAdbc(error, driver()); + } +}; + +// Driver authors can subclass DatabaseObjectBase to track driver-specific +// state pertaining to the AdbcDatbase. The private_data member of an +// AdbcDatabase initialized by the driver will be a pointer to the +// subclass of DatbaseObjectBase. +class DatabaseObjectBase : public ObjectBase { + public: + // (there are no database functions other than option getting/setting) +}; + +// Driver authors can subclass ConnectionObjectBase to track driver-specific +// state pertaining to the AdbcConnection. The private_data member of an +// AdbcConnection initialized by the driver will be a pointer to the +// subclass of ConnectionObjectBase. Driver authors can override methods to +// implement the corresponding ConnectionXXX driver methods. +class ConnectionObjectBase : public ObjectBase { + public: + // TODO: Add connection functions here as methods +}; + +// Driver authors can subclass StatementObjectBase to track driver-specific +// state pertaining to the AdbcStatement. The private_data member of an +// AdbcStatement initialized by the driver will be a pointer to the +// subclass of StatementObjectBase. Driver authors can override methods to +// implement the corresponding StatementXXX driver methods. +class StatementObjectBase : public ObjectBase { + public: + virtual AdbcStatusCode ExecuteQuery(struct ArrowArrayStream* stream, + int64_t* rows_affected, struct AdbcError* error) { + return ADBC_STATUS_NOT_IMPLEMENTED; + } + + // TODO: Add remaining statement functions here as methods +}; + +// Driver authors can declare a template specialization of the Driver class +// and use it to provide their driver init function. It is possible, but +// rarely useful, to subclass a driver. +template +class Driver { + public: + static AdbcStatusCode Init(int version, void* raw_driver, AdbcError* error) { + if (version != ADBC_VERSION_1_1_0) return ADBC_STATUS_NOT_IMPLEMENTED; + struct AdbcDriver* driver = (AdbcDriver*)raw_driver; + std::memset(driver, 0, sizeof(AdbcDriver)); + + // Driver lifecycle + driver->private_data = new Driver(); + driver->release = &CDriverRelease; + + // Driver functions + driver->ErrorGetDetailCount = &CErrorGetDetailCount; + driver->ErrorGetDetail = &CErrorGetDetail; + + // Database lifecycle + driver->DatabaseNew = &CNew; + driver->DatabaseInit = &CDatabaseInit; + driver->DatabaseRelease = &CRelease; + + // Database functions + driver->DatabaseSetOption = &CSetOption; + driver->DatabaseSetOptionBytes = &CSetOptionBytes; + driver->DatabaseSetOptionInt = &CSetOptionInt; + driver->DatabaseSetOptionDouble = &CSetOptionDouble; + driver->DatabaseGetOption = &CGetOption; + driver->DatabaseGetOptionBytes = &CGetOptionBytes; + driver->DatabaseGetOptionInt = &CGetOptionInt; + driver->DatabaseGetOptionDouble = &CGetOptionDouble; + + // Connection lifecycle + driver->ConnectionNew = &CNew; + driver->ConnectionInit = &CConnectionInit; + driver->ConnectionRelease = &CRelease; + + // Connection functions + driver->ConnectionSetOption = &CSetOption; + driver->ConnectionSetOptionBytes = &CSetOptionBytes; + driver->ConnectionSetOptionInt = &CSetOptionInt; + driver->ConnectionSetOptionDouble = &CSetOptionDouble; + driver->ConnectionGetOption = &CGetOption; + driver->ConnectionGetOptionBytes = &CGetOptionBytes; + driver->ConnectionGetOptionInt = &CGetOptionInt; + driver->ConnectionGetOptionDouble = &CGetOptionDouble; + + // Statement lifecycle + driver->StatementNew = &CStatementNew; + driver->StatementRelease = &CRelease; + + // Statement functions + driver->StatementSetOption = &CSetOption; + driver->StatementSetOptionBytes = &CSetOptionBytes; + driver->StatementSetOptionInt = &CSetOptionInt; + driver->StatementSetOptionDouble = &CSetOptionDouble; + driver->StatementGetOption = &CGetOption; + driver->StatementGetOptionBytes = &CGetOptionBytes; + driver->StatementGetOptionInt = &CGetOptionInt; + driver->StatementGetOptionDouble = &CGetOptionDouble; + + driver->StatementExecuteQuery = &CStatementExecuteQuery; + + return ADBC_STATUS_OK; + } + + private: + // Driver trampolines + static AdbcStatusCode CDriverRelease(AdbcDriver* driver, AdbcError* error) { + auto driver_private = reinterpret_cast(driver->private_data); + delete driver_private; + driver->private_data = nullptr; + return ADBC_STATUS_OK; + } + + static int CErrorGetDetailCount(const AdbcError* error) { + if (error->vendor_code != ADBC_ERROR_VENDOR_CODE_PRIVATE_DATA) { + return 0; + } + + auto error_obj = reinterpret_cast(error->private_data); + return error_obj->CDetailCount(); + } + + static AdbcErrorDetail CErrorGetDetail(const AdbcError* error, int index) { + auto error_obj = reinterpret_cast(error->private_data); + return error_obj->CDetail(index); + } + + // Templatable trampolines + template + static AdbcStatusCode CNew(T* obj, AdbcError* error) { + auto private_data = new ObjectT(); + obj->private_data = private_data; + return ADBC_STATUS_OK; + } + + template + static AdbcStatusCode CRelease(T* obj, AdbcError* error) { + auto private_data = reinterpret_cast(obj->private_data); + AdbcStatusCode result = private_data->Release(error); + if (result != ADBC_STATUS_OK) { + return result; + } + + delete private_data; + obj->private_data = nullptr; + return ADBC_STATUS_OK; + } + + template + static AdbcStatusCode CSetOption(T* obj, const char* key, const char* value, + AdbcError* error) { + auto private_data = reinterpret_cast(obj->private_data); + return private_data->template CSetOption<>(key, value, error); + } + + template + static AdbcStatusCode CSetOptionBytes(T* obj, const char* key, const uint8_t* value, + size_t length, AdbcError* error) { + auto private_data = reinterpret_cast(obj->private_data); + return private_data->CSetOptionBytes(key, value, length, error); + } + + template + static AdbcStatusCode CSetOptionInt(T* obj, const char* key, int64_t value, + AdbcError* error) { + auto private_data = reinterpret_cast(obj->private_data); + return private_data->template CSetOption<>(key, value, error); + } + + template + static AdbcStatusCode CSetOptionDouble(T* obj, const char* key, double value, + AdbcError* error) { + auto private_data = reinterpret_cast(obj->private_data); + return private_data->template CSetOption<>(key, value, error); + } + + template + static AdbcStatusCode CGetOption(T* obj, const char* key, char* value, size_t* length, + AdbcError* error) { + auto private_data = reinterpret_cast(obj->private_data); + return private_data->template CGetOptionStringLike<>(key, value, length, error); + } + + template + static AdbcStatusCode CGetOptionBytes(T* obj, const char* key, uint8_t* value, + size_t* length, AdbcError* error) { + auto private_data = reinterpret_cast(obj->private_data); + return private_data->template CGetOptionStringLike<>(key, value, length, error); + } + + template + static AdbcStatusCode CGetOptionInt(T* obj, const char* key, int64_t* value, + AdbcError* error) { + auto private_data = reinterpret_cast(obj->private_data); + return private_data->template CGetOptionNumeric<>(key, value, error); + } + + template + static AdbcStatusCode CGetOptionDouble(T* obj, const char* key, double* value, + AdbcError* error) { + auto private_data = reinterpret_cast(obj->private_data); + return private_data->template CGetOptionNumeric<>(key, value, error); + } + + // Database trampolines + static AdbcStatusCode CDatabaseInit(AdbcDatabase* database, AdbcError* error) { + auto private_data = reinterpret_cast(database->private_data); + private_data->set_driver(database->private_driver); + return private_data->Init(database->private_driver->private_data, error); + } + + // Connection trampolines + static AdbcStatusCode CConnectionInit(AdbcConnection* connection, + AdbcDatabase* database, AdbcError* error) { + auto private_data = reinterpret_cast(connection->private_data); + private_data->set_driver(connection->private_driver); + return private_data->Init(database->private_data, error); + } + + // Statement trampolines + static AdbcStatusCode CStatementNew(AdbcConnection* connection, + AdbcStatement* statement, AdbcError* error) { + auto private_data = new StatementT(); + private_data->set_driver(connection->private_driver); + AdbcStatusCode status = private_data->Init(connection->private_data, error); + if (status != ADBC_STATUS_OK) { + delete private_data; + } + + statement->private_data = private_data; + return ADBC_STATUS_OK; + } + + static AdbcStatusCode CStatementExecuteQuery(AdbcStatement* statement, + struct ArrowArrayStream* stream, + int64_t* rows_affected, + struct AdbcError* error) { + auto private_data = reinterpret_cast(statement->private_data); + return private_data->ExecuteQuery(stream, rows_affected, error); + } +}; + +} // namespace r + +} // namespace adbc diff --git a/r/adbcdrivermanager/src/driver_void.c b/r/adbcdrivermanager/src/driver_void.c deleted file mode 100644 index 3902b71490..0000000000 --- a/r/adbcdrivermanager/src/driver_void.c +++ /dev/null @@ -1,316 +0,0 @@ -// 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. - -#define R_NO_REMAP -#include -#include - -#include - -#include - -struct VoidDriverPrivate { - char token[1024]; -}; - -struct VoidDatabasePrivate { - char token[1024]; -}; - -struct VoidConnectionPrivate { - char token[1024]; -}; - -struct VoidStatementPrivate { - char token[1024]; -}; - -static void ResetError(struct AdbcError* error) { - memset(error, 0, sizeof(struct AdbcError)); -} - -static void SetErrorConst(struct AdbcError* error, const char* value) { - if (error == NULL) { - return; - } - - ResetError(error); - error->message = (char*)value; -} - -static AdbcStatusCode VoidDriverRelease(struct AdbcDriver* driver, - struct AdbcError* error) { - if (driver->private_data == NULL) { - return ADBC_STATUS_OK; - } - - free(driver->private_data); - driver->private_data = NULL; - return ADBC_STATUS_OK; -} - -static AdbcStatusCode VoidDatabaseNew(struct AdbcDatabase* database, - struct AdbcError* error) { - struct VoidDatabasePrivate* database_private = - (struct VoidDatabasePrivate*)malloc(sizeof(struct VoidDatabasePrivate)); - if (database_private == NULL) { - SetErrorConst(error, "failed to allocate VoidDatabasePrivate"); - return ADBC_STATUS_INTERNAL; - } - - memset(database_private, 0, sizeof(struct VoidDatabasePrivate)); - database->private_data = database_private; - return ADBC_STATUS_OK; -} - -static AdbcStatusCode VoidDatabaseInit(struct AdbcDatabase* database, - struct AdbcError* error) { - return ADBC_STATUS_OK; -} - -static AdbcStatusCode VoidDatabaseSetOption(struct AdbcDatabase* database, - const char* key, const char* value, - struct AdbcError* error) { - return ADBC_STATUS_OK; -} - -static AdbcStatusCode VoidDatabaseRelease(struct AdbcDatabase* database, - struct AdbcError* error) { - if (database->private_data == NULL) { - return ADBC_STATUS_OK; - } - - free(database->private_data); - database->private_data = NULL; - return ADBC_STATUS_OK; -} - -static AdbcStatusCode VoidConnectionCommit(struct AdbcConnection* connection, - struct AdbcError* error) { - return ADBC_STATUS_NOT_IMPLEMENTED; -} - -static AdbcStatusCode VoidConnectionGetInfo(struct AdbcConnection* connection, - const uint32_t* info_codes, - size_t info_codes_length, - struct ArrowArrayStream* stream, - struct AdbcError* error) { - return ADBC_STATUS_NOT_IMPLEMENTED; -} - -static AdbcStatusCode VoidConnectionGetObjects( - struct AdbcConnection* connection, int depth, const char* catalog, - const char* db_schema, const char* table_name, const char** table_types, - const char* column_name, struct ArrowArrayStream* stream, struct AdbcError* error) { - return ADBC_STATUS_NOT_IMPLEMENTED; -} - -static AdbcStatusCode VoidConnectionGetTableSchema( - struct AdbcConnection* connection, const char* catalog, const char* db_schema, - const char* table_name, struct ArrowSchema* schema, struct AdbcError* error) { - return ADBC_STATUS_NOT_IMPLEMENTED; -} - -static AdbcStatusCode VoidConnectionGetTableTypes(struct AdbcConnection* connection, - struct ArrowArrayStream* stream, - struct AdbcError* error) { - return ADBC_STATUS_NOT_IMPLEMENTED; -} - -static AdbcStatusCode VoidConnectionInit(struct AdbcConnection* connection, - struct AdbcDatabase* database, - struct AdbcError* error) { - return ADBC_STATUS_OK; -} - -static AdbcStatusCode VoidConnectionNew(struct AdbcConnection* connection, - struct AdbcError* error) { - struct VoidConnectionPrivate* connection_private = - (struct VoidConnectionPrivate*)malloc(sizeof(struct VoidConnectionPrivate)); - if (connection_private == NULL) { - SetErrorConst(error, "failed to allocate VoidConnectionPrivate"); - return ADBC_STATUS_INTERNAL; - } - - memset(connection_private, 0, sizeof(struct VoidConnectionPrivate)); - connection->private_data = connection_private; - return ADBC_STATUS_OK; -} - -static AdbcStatusCode VoidConnectionReadPartition(struct AdbcConnection* connection, - const uint8_t* serialized_partition, - size_t serialized_length, - struct ArrowArrayStream* out, - struct AdbcError* error) { - return ADBC_STATUS_NOT_IMPLEMENTED; -} - -static AdbcStatusCode VoidConnectionRelease(struct AdbcConnection* connection, - struct AdbcError* error) { - if (connection->private_data == NULL) { - return ADBC_STATUS_OK; - } - - free(connection->private_data); - connection->private_data = NULL; - return ADBC_STATUS_OK; -} - -static AdbcStatusCode VoidConnectionRollback(struct AdbcConnection* connection, - struct AdbcError* error) { - return ADBC_STATUS_NOT_IMPLEMENTED; -} - -static AdbcStatusCode VoidConnectionSetOption(struct AdbcConnection* connection, - const char* key, const char* value, - struct AdbcError* error) { - return ADBC_STATUS_OK; -} - -static AdbcStatusCode VoidStatementBind(struct AdbcStatement* statement, - struct ArrowArray* values, - struct ArrowSchema* schema, - struct AdbcError* error) { - return ADBC_STATUS_NOT_IMPLEMENTED; -} // NOLINT(whitespace/indent) - -static AdbcStatusCode VoidStatementBindStream(struct AdbcStatement* statement, - struct ArrowArrayStream* stream, - struct AdbcError* error) { - return ADBC_STATUS_NOT_IMPLEMENTED; -} - -static AdbcStatusCode VoidStatementExecutePartitions(struct AdbcStatement* statement, - struct ArrowSchema* schema, - struct AdbcPartitions* partitions, - int64_t* rows_affected, - struct AdbcError* error) { - return ADBC_STATUS_NOT_IMPLEMENTED; -} // NOLINT(whitespace/indent) - -static AdbcStatusCode VoidStatementExecuteQuery(struct AdbcStatement* statement, - struct ArrowArrayStream* out, - int64_t* rows_affected, - struct AdbcError* error) { - return ADBC_STATUS_NOT_IMPLEMENTED; -} - -static AdbcStatusCode VoidStatementGetParameterSchema(struct AdbcStatement* statement, - struct ArrowSchema* schema, - struct AdbcError* error) { - return ADBC_STATUS_NOT_IMPLEMENTED; -} - -static AdbcStatusCode VoidStatementNew(struct AdbcConnection* connection, - struct AdbcStatement* statement, - struct AdbcError* error) { - struct VoidStatementPrivate* statement_private = - (struct VoidStatementPrivate*)malloc(sizeof(struct VoidStatementPrivate)); - if (statement_private == NULL) { - SetErrorConst(error, "failed to allocate VoidStatementPrivate"); - return ADBC_STATUS_INTERNAL; - } - - memset(statement_private, 0, sizeof(struct VoidStatementPrivate)); - statement->private_data = statement_private; - return ADBC_STATUS_OK; -} - -static AdbcStatusCode VoidStatementPrepare(struct AdbcStatement* statement, - struct AdbcError* error) { - return ADBC_STATUS_NOT_IMPLEMENTED; -} - -static AdbcStatusCode VoidStatementRelease(struct AdbcStatement* statement, - struct AdbcError* error) { - if (statement->private_data == NULL) { - return ADBC_STATUS_OK; - } - - free(statement->private_data); - statement->private_data = NULL; - return ADBC_STATUS_OK; -} - -static AdbcStatusCode VoidStatementSetOption(struct AdbcStatement* statement, - const char* key, const char* value, - struct AdbcError* error) { - return ADBC_STATUS_OK; -} - -static AdbcStatusCode VoidStatementSetSqlQuery(struct AdbcStatement* statement, - const char* query, - struct AdbcError* error) { - return ADBC_STATUS_NOT_IMPLEMENTED; -} - -static AdbcStatusCode VoidDriverInitFunc(int version, void* raw_driver, - struct AdbcError* error) { - if (version != ADBC_VERSION_1_1_0) return ADBC_STATUS_NOT_IMPLEMENTED; - struct AdbcDriver* driver = (struct AdbcDriver*)raw_driver; - memset(driver, 0, sizeof(struct AdbcDriver)); - - struct VoidDriverPrivate* driver_private = - (struct VoidDriverPrivate*)malloc(sizeof(struct VoidDriverPrivate)); - if (driver_private == NULL) { - SetErrorConst(error, "failed to allocate VoidDriverPrivate"); - return ADBC_STATUS_INTERNAL; - } - - memset(driver_private, 0, sizeof(struct VoidDriverPrivate)); - driver->private_data = driver_private; - - driver->DatabaseInit = &VoidDatabaseInit; - driver->DatabaseNew = VoidDatabaseNew; - driver->DatabaseRelease = VoidDatabaseRelease; - driver->DatabaseSetOption = VoidDatabaseSetOption; - - driver->ConnectionCommit = VoidConnectionCommit; - driver->ConnectionGetInfo = VoidConnectionGetInfo; - driver->ConnectionGetObjects = VoidConnectionGetObjects; - driver->ConnectionGetTableSchema = VoidConnectionGetTableSchema; - driver->ConnectionGetTableTypes = VoidConnectionGetTableTypes; - driver->ConnectionInit = VoidConnectionInit; - driver->ConnectionNew = VoidConnectionNew; - driver->ConnectionReadPartition = VoidConnectionReadPartition; - driver->ConnectionRelease = VoidConnectionRelease; - driver->ConnectionRollback = VoidConnectionRollback; - driver->ConnectionSetOption = VoidConnectionSetOption; - - driver->StatementBind = VoidStatementBind; - driver->StatementBindStream = VoidStatementBindStream; - driver->StatementExecutePartitions = VoidStatementExecutePartitions; - driver->StatementExecuteQuery = VoidStatementExecuteQuery; - driver->StatementGetParameterSchema = VoidStatementGetParameterSchema; - driver->StatementNew = VoidStatementNew; - driver->StatementPrepare = VoidStatementPrepare; - driver->StatementRelease = VoidStatementRelease; - driver->StatementSetOption = VoidStatementSetOption; - driver->StatementSetSqlQuery = VoidStatementSetSqlQuery; - - driver->release = VoidDriverRelease; - - return ADBC_STATUS_OK; -} - -SEXP RAdbcVoidDriverInitFunc(void) { - SEXP xptr = - PROTECT(R_MakeExternalPtrFn((DL_FUNC)VoidDriverInitFunc, R_NilValue, R_NilValue)); - Rf_setAttrib(xptr, R_ClassSymbol, Rf_mkString("adbc_driver_init_func")); - UNPROTECT(1); - return xptr; -} diff --git a/r/adbcdrivermanager/src/driver_void.cc b/r/adbcdrivermanager/src/driver_void.cc new file mode 100644 index 0000000000..5898cb875a --- /dev/null +++ b/r/adbcdrivermanager/src/driver_void.cc @@ -0,0 +1,41 @@ +// 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. + +#define R_NO_REMAP +#include +#include + +#include "driver_base.h" + +#include + +using VoidDriver = + adbc::r::Driver; + +static AdbcStatusCode VoidDriverInitFunc(int version, void* raw_driver, + struct AdbcError* error) { + return VoidDriver::Init(version, raw_driver, error); +} + +extern "C" SEXP RAdbcVoidDriverInitFunc(void) { + SEXP xptr = + PROTECT(R_MakeExternalPtrFn((DL_FUNC)VoidDriverInitFunc, R_NilValue, R_NilValue)); + Rf_setAttrib(xptr, R_ClassSymbol, Rf_mkString("adbc_driver_init_func")); + UNPROTECT(1); + return xptr; +} diff --git a/r/adbcdrivermanager/src/error.cc b/r/adbcdrivermanager/src/error.cc index ff24e89812..0ff01b879b 100644 --- a/r/adbcdrivermanager/src/error.cc +++ b/r/adbcdrivermanager/src/error.cc @@ -34,15 +34,17 @@ static void finalize_error_xptr(SEXP error_xptr) { adbc_xptr_default_finalize(error_xptr); } -extern "C" SEXP RAdbcAllocateError(SEXP shelter_sexp) { +extern "C" SEXP RAdbcAllocateError(SEXP shelter_sexp, SEXP use_legacy_error_sexp) { + bool use_legacy_error = adbc_as_bool(use_legacy_error_sexp); + SEXP error_xptr = PROTECT(adbc_allocate_xptr(shelter_sexp)); R_RegisterCFinalizer(error_xptr, &finalize_error_xptr); AdbcError* error = adbc_from_xptr(error_xptr); - error->message = nullptr; - error->vendor_code = 0; - memset(error->sqlstate, 0, sizeof(error->sqlstate)); - error->release = nullptr; + *error = ADBC_ERROR_INIT; + if (use_legacy_error) { + error->vendor_code = 0; + } UNPROTECT(1); return error_xptr; diff --git a/r/adbcdrivermanager/src/init.c b/r/adbcdrivermanager/src/init.c index 7c5ad3f28e..503190544d 100644 --- a/r/adbcdrivermanager/src/init.c +++ b/r/adbcdrivermanager/src/init.c @@ -23,34 +23,16 @@ SEXP RAdbcLogDriverInitFunc(void); SEXP RAdbcMonkeyDriverInitFunc(void); SEXP RAdbcVoidDriverInitFunc(void); -SEXP RAdbcAllocateError(SEXP shelter_sexp); +SEXP RAdbcAllocateError(SEXP shelter_sexp, SEXP use_legacy_error_sexp); SEXP RAdbcErrorProxy(SEXP error_xptr); SEXP RAdbcErrorFromArrayStream(SEXP stream_xptr); SEXP RAdbcStatusCodeMessage(SEXP status_sexp); SEXP RAdbcDatabaseSetOption(SEXP database_xptr, SEXP key_sexp, SEXP value_sexp, SEXP error_xptr); -SEXP RAdbcDatabaseSetOptionBytes(SEXP database_xptr, SEXP key_sexp, SEXP value_sexp, - SEXP error_xptr); -SEXP RAdbcDatabaseSetOptionInt(SEXP database_xptr, SEXP key_sexp, SEXP value_sexp, - SEXP error_xptr); -SEXP RAdbcDatabaseSetOptionDouble(SEXP database_xptr, SEXP key_sexp, SEXP value_sexp, - SEXP error_xptr); SEXP RAdbcConnectionSetOption(SEXP connection_xptr, SEXP key_sexp, SEXP value_sexp, SEXP error_xptr); -SEXP RAdbcConnectionSetOptionBytes(SEXP connection_xptr, SEXP key_sexp, SEXP value_sexp, - SEXP error_xptr); -SEXP RAdbcConnectionSetOptionInt(SEXP connection_xptr, SEXP key_sexp, SEXP value_sexp, - SEXP error_xptr); -SEXP RAdbcConnectionSetOptionDouble(SEXP connection_xptr, SEXP key_sexp, SEXP value_sexp, - SEXP error_xptr); SEXP RAdbcStatementSetOption(SEXP statement_xptr, SEXP key_sexp, SEXP value_sexp, SEXP error_xptr); -SEXP RAdbcStatementSetOptionBytes(SEXP statement_xptr, SEXP key_sexp, SEXP value_sexp, - SEXP error_xptr); -SEXP RAdbcStatementSetOptionInt(SEXP statement_xptr, SEXP key_sexp, SEXP value_sexp, - SEXP error_xptr); -SEXP RAdbcStatementSetOptionDouble(SEXP statement_xptr, SEXP key_sexp, SEXP value_sexp, - SEXP error_xptr); SEXP RAdbcDatabaseGetOption(SEXP database_xptr, SEXP key_sexp, SEXP error_xptr); SEXP RAdbcDatabaseGetOptionBytes(SEXP database_xptr, SEXP key_sexp, SEXP error_xptr); SEXP RAdbcDatabaseGetOptionInt(SEXP database_xptr, SEXP key_sexp, SEXP error_xptr); @@ -121,22 +103,13 @@ static const R_CallMethodDef CallEntries[] = { {"RAdbcLogDriverInitFunc", (DL_FUNC)&RAdbcLogDriverInitFunc, 0}, {"RAdbcMonkeyDriverInitFunc", (DL_FUNC)&RAdbcMonkeyDriverInitFunc, 0}, {"RAdbcVoidDriverInitFunc", (DL_FUNC)&RAdbcVoidDriverInitFunc, 0}, - {"RAdbcAllocateError", (DL_FUNC)&RAdbcAllocateError, 1}, + {"RAdbcAllocateError", (DL_FUNC)&RAdbcAllocateError, 2}, {"RAdbcErrorProxy", (DL_FUNC)&RAdbcErrorProxy, 1}, {"RAdbcErrorFromArrayStream", (DL_FUNC)&RAdbcErrorFromArrayStream, 1}, {"RAdbcStatusCodeMessage", (DL_FUNC)&RAdbcStatusCodeMessage, 1}, {"RAdbcDatabaseSetOption", (DL_FUNC)&RAdbcDatabaseSetOption, 4}, - {"RAdbcDatabaseSetOptionBytes", (DL_FUNC)&RAdbcDatabaseSetOptionBytes, 4}, - {"RAdbcDatabaseSetOptionInt", (DL_FUNC)&RAdbcDatabaseSetOptionInt, 4}, - {"RAdbcDatabaseSetOptionDouble", (DL_FUNC)&RAdbcDatabaseSetOptionDouble, 4}, {"RAdbcConnectionSetOption", (DL_FUNC)&RAdbcConnectionSetOption, 4}, - {"RAdbcConnectionSetOptionBytes", (DL_FUNC)&RAdbcConnectionSetOptionBytes, 4}, - {"RAdbcConnectionSetOptionInt", (DL_FUNC)&RAdbcConnectionSetOptionInt, 4}, - {"RAdbcConnectionSetOptionDouble", (DL_FUNC)&RAdbcConnectionSetOptionDouble, 4}, {"RAdbcStatementSetOption", (DL_FUNC)&RAdbcStatementSetOption, 4}, - {"RAdbcStatementSetOptionBytes", (DL_FUNC)&RAdbcStatementSetOptionBytes, 4}, - {"RAdbcStatementSetOptionInt", (DL_FUNC)&RAdbcStatementSetOptionInt, 4}, - {"RAdbcStatementSetOptionDouble", (DL_FUNC)&RAdbcStatementSetOptionDouble, 4}, {"RAdbcDatabaseGetOption", (DL_FUNC)&RAdbcDatabaseGetOption, 3}, {"RAdbcDatabaseGetOptionBytes", (DL_FUNC)&RAdbcDatabaseGetOptionBytes, 3}, {"RAdbcDatabaseGetOptionInt", (DL_FUNC)&RAdbcDatabaseGetOptionInt, 3}, diff --git a/r/adbcdrivermanager/src/options.cc b/r/adbcdrivermanager/src/options.cc index 377d633094..9aeadbe661 100644 --- a/r/adbcdrivermanager/src/options.cc +++ b/r/adbcdrivermanager/src/options.cc @@ -67,74 +67,65 @@ SEXP adbc_set_option_bytes(SEXP obj_xptr, SEXP key_sexp, SEXP value_sexp, SEXP e extern "C" SEXP RAdbcDatabaseSetOption(SEXP database_xptr, SEXP key_sexp, SEXP value_sexp, SEXP error_xptr) { - return adbc_set_option(database_xptr, key_sexp, value_sexp, - error_xptr, &AdbcDatabaseSetOption); -} - -extern "C" SEXP RAdbcDatabaseSetOptionBytes(SEXP database_xptr, SEXP key_sexp, - SEXP value_sexp, SEXP error_xptr) { - return adbc_set_option_bytes(database_xptr, key_sexp, value_sexp, - error_xptr, &AdbcDatabaseSetOptionBytes); -} - -extern "C" SEXP RAdbcDatabaseSetOptionInt(SEXP database_xptr, SEXP key_sexp, - SEXP value_sexp, SEXP error_xptr) { - return adbc_set_option(database_xptr, key_sexp, value_sexp, - error_xptr, &AdbcDatabaseSetOptionInt); -} - -extern "C" SEXP RAdbcDatabaseSetOptionDouble(SEXP database_xptr, SEXP key_sexp, - SEXP value_sexp, SEXP error_xptr) { - return adbc_set_option(database_xptr, key_sexp, value_sexp, - error_xptr, &AdbcDatabaseSetOptionDouble); + switch (TYPEOF(value_sexp)) { + case STRSXP: + return adbc_set_option( + database_xptr, key_sexp, value_sexp, error_xptr, &AdbcDatabaseSetOption); + case RAWSXP: + return adbc_set_option_bytes(database_xptr, key_sexp, value_sexp, + error_xptr, &AdbcDatabaseSetOptionBytes); + case INTSXP: + return adbc_set_option( + database_xptr, key_sexp, value_sexp, error_xptr, &AdbcDatabaseSetOptionInt); + case REALSXP: + return adbc_set_option( + database_xptr, key_sexp, value_sexp, error_xptr, &AdbcDatabaseSetOptionDouble); + default: + Rf_error("Option value type not suppported"); + } } extern "C" SEXP RAdbcConnectionSetOption(SEXP connection_xptr, SEXP key_sexp, SEXP value_sexp, SEXP error_xptr) { - return adbc_set_option( - connection_xptr, key_sexp, value_sexp, error_xptr, &AdbcConnectionSetOption); -} - -extern "C" SEXP RAdbcConnectionSetOptionBytes(SEXP connection_xptr, SEXP key_sexp, - SEXP value_sexp, SEXP error_xptr) { - return adbc_set_option_bytes(connection_xptr, key_sexp, value_sexp, - error_xptr, &AdbcConnectionSetOptionBytes); -} - -extern "C" SEXP RAdbcConnectionSetOptionInt(SEXP connection_xptr, SEXP key_sexp, - SEXP value_sexp, SEXP error_xptr) { - return adbc_set_option( - connection_xptr, key_sexp, value_sexp, error_xptr, &AdbcConnectionSetOptionInt); -} - -extern "C" SEXP RAdbcConnectionSetOptionDouble(SEXP connection_xptr, SEXP key_sexp, - SEXP value_sexp, SEXP error_xptr) { - return adbc_set_option( - connection_xptr, key_sexp, value_sexp, error_xptr, &AdbcConnectionSetOptionDouble); + switch (TYPEOF(value_sexp)) { + case STRSXP: + return adbc_set_option( + connection_xptr, key_sexp, value_sexp, error_xptr, &AdbcConnectionSetOption); + case RAWSXP: + return adbc_set_option_bytes(connection_xptr, key_sexp, value_sexp, + error_xptr, + &AdbcConnectionSetOptionBytes); + case INTSXP: + return adbc_set_option( + connection_xptr, key_sexp, value_sexp, error_xptr, &AdbcConnectionSetOptionInt); + case REALSXP: + return adbc_set_option(connection_xptr, key_sexp, + value_sexp, error_xptr, + &AdbcConnectionSetOptionDouble); + default: + Rf_error("Option value type not suppported"); + } } extern "C" SEXP RAdbcStatementSetOption(SEXP statement_xptr, SEXP key_sexp, SEXP value_sexp, SEXP error_xptr) { - return adbc_set_option(statement_xptr, key_sexp, value_sexp, - error_xptr, &AdbcStatementSetOption); -} - -extern "C" SEXP RAdbcStatementSetOptionBytes(SEXP statement_xptr, SEXP key_sexp, - SEXP value_sexp, SEXP error_xptr) { - return adbc_set_option_bytes(statement_xptr, key_sexp, value_sexp, - error_xptr, &AdbcStatementSetOptionBytes); -} - -extern "C" SEXP RAdbcStatementSetOptionInt(SEXP statement_xptr, SEXP key_sexp, - SEXP value_sexp, SEXP error_xptr) { - return adbc_set_option(statement_xptr, key_sexp, value_sexp, - error_xptr, &AdbcStatementSetOptionInt); -} - -extern "C" SEXP RAdbcStatementSetOptionDouble(SEXP statement_xptr, SEXP key_sexp, - SEXP value_sexp, SEXP error_xptr) { - return adbc_set_option( - statement_xptr, key_sexp, value_sexp, error_xptr, &AdbcStatementSetOptionDouble); + switch (TYPEOF(value_sexp)) { + case STRSXP: + return adbc_set_option( + statement_xptr, key_sexp, value_sexp, error_xptr, &AdbcStatementSetOption); + case RAWSXP: + return adbc_set_option_bytes( + statement_xptr, key_sexp, value_sexp, error_xptr, &AdbcStatementSetOptionBytes); + case INTSXP: + return adbc_set_option( + statement_xptr, key_sexp, value_sexp, error_xptr, &AdbcStatementSetOptionInt); + case REALSXP: + return adbc_set_option(statement_xptr, key_sexp, value_sexp, + error_xptr, + &AdbcStatementSetOptionDouble); + default: + Rf_error("Option value type not suppported"); + } } template diff --git a/r/adbcdrivermanager/src/radbc.cc b/r/adbcdrivermanager/src/radbc.cc index fb271296e4..368d550350 100644 --- a/r/adbcdrivermanager/src/radbc.cc +++ b/r/adbcdrivermanager/src/radbc.cc @@ -19,7 +19,7 @@ #include #include -#include +#include #include #include @@ -48,8 +48,7 @@ static void finalize_driver_xptr(SEXP driver_xptr) { } if (driver->release != nullptr) { - AdbcError error; - memset(&error, 0, sizeof(AdbcError)); + AdbcError error = ADBC_ERROR_INIT; int status = driver->release(driver, &error); adbc_error_warn(status, &error, "finalize_driver_xptr()"); } @@ -65,8 +64,7 @@ static void finalize_database_xptr(SEXP database_xptr) { } if (database->private_data != nullptr) { - AdbcError error; - memset(&error, 0, sizeof(AdbcError)); + AdbcError error = ADBC_ERROR_INIT; int status = AdbcDatabaseRelease(database, &error); adbc_error_warn(status, &error, "finalize_database_xptr()"); } @@ -127,8 +125,7 @@ extern "C" SEXP RAdbcDatabaseNew(SEXP driver_init_func_xptr) { AdbcDatabase* database = adbc_from_xptr(database_xptr); - AdbcError error; - memset(&error, 0, sizeof(AdbcError)); + AdbcError error = ADBC_ERROR_INIT; int status = AdbcDatabaseNew(database, &error); adbc_error_stop(status, &error); @@ -153,9 +150,9 @@ extern "C" SEXP RAdbcMoveDatabase(SEXP database_xptr) { R_RegisterCFinalizer(database_xptr_new, &finalize_database_xptr); AdbcDatabase* database_new = adbc_from_xptr(database_xptr_new); - memcpy(database_new, database, sizeof(AdbcDatabase)); + std::memcpy(database_new, database, sizeof(AdbcDatabase)); adbc_xptr_move_attrs(database_xptr, database_xptr_new); - memset(database, 0, sizeof(AdbcDatabase)); + std::memset(database, 0, sizeof(AdbcDatabase)); UNPROTECT(1); return database_xptr_new; @@ -186,8 +183,7 @@ static void finalize_connection_xptr(SEXP connection_xptr) { } if (connection->private_data != nullptr) { - AdbcError error; - memset(&error, 0, sizeof(AdbcError)); + AdbcError error = ADBC_ERROR_INIT; int status = AdbcConnectionRelease(connection, &error); adbc_error_warn(status, &error, "finalize_connection_xptr()"); } @@ -201,8 +197,7 @@ extern "C" SEXP RAdbcConnectionNew(void) { AdbcConnection* connection = adbc_from_xptr(connection_xptr); - AdbcError error; - memset(&error, 0, sizeof(AdbcError)); + AdbcError error = ADBC_ERROR_INIT; int status = AdbcConnectionNew(connection, &error); adbc_error_stop(status, &error); @@ -216,9 +211,9 @@ extern "C" SEXP RAdbcMoveConnection(SEXP connection_xptr) { R_RegisterCFinalizer(connection_xptr_new, &finalize_connection_xptr); AdbcConnection* connection_new = adbc_from_xptr(connection_xptr_new); - memcpy(connection_new, connection, sizeof(AdbcConnection)); + std::memcpy(connection_new, connection, sizeof(AdbcConnection)); adbc_xptr_move_attrs(connection_xptr, connection_xptr_new); - memset(connection, 0, sizeof(AdbcConnection)); + std::memset(connection, 0, sizeof(AdbcConnection)); UNPROTECT(1); return connection_xptr_new; @@ -386,8 +381,7 @@ static void finalize_statement_xptr(SEXP statement_xptr) { } if (statement->private_data != nullptr) { - AdbcError error; - memset(&error, 0, sizeof(AdbcError)); + AdbcError error = ADBC_ERROR_INIT; int status = AdbcStatementRelease(statement, &error); adbc_error_warn(status, &error, "finalize_statement_xptr()"); } @@ -402,8 +396,7 @@ extern "C" SEXP RAdbcStatementNew(SEXP connection_xptr) { AdbcStatement* statement = adbc_from_xptr(statement_xptr); - AdbcError error; - memset(&error, 0, sizeof(AdbcError)); + AdbcError error = ADBC_ERROR_INIT; int status = AdbcStatementNew(connection, statement, &error); adbc_error_stop(status, &error); @@ -419,9 +412,9 @@ extern "C" SEXP RAdbcMoveStatement(SEXP statement_xptr) { R_RegisterCFinalizer(statement_xptr_new, &finalize_statement_xptr); AdbcStatement* statement_new = adbc_from_xptr(statement_xptr_new); - memcpy(statement_new, statement, sizeof(AdbcStatement)); + std::memcpy(statement_new, statement, sizeof(AdbcStatement)); adbc_xptr_move_attrs(statement_xptr, statement_xptr_new); - memset(statement, 0, sizeof(AdbcStatement)); + std::memset(statement, 0, sizeof(AdbcStatement)); UNPROTECT(1); return statement_xptr_new; diff --git a/r/adbcdrivermanager/tests/testthat/test-error.R b/r/adbcdrivermanager/tests/testthat/test-error.R index 15a9c21e61..222d76fa0a 100644 --- a/r/adbcdrivermanager/tests/testthat/test-error.R +++ b/r/adbcdrivermanager/tests/testthat/test-error.R @@ -37,7 +37,6 @@ test_that("error allocator works", { expect_identical(length(err), 4L) expect_identical(names(err), c("message", "vendor_code", "sqlstate", "details")) expect_null(err$message) - expect_identical(err$vendor_code, 0L) expect_identical(err$sqlstate, as.raw(c(0x00, 0x00, 0x00, 0x00, 0x00))) expect_identical(err$details, setNames(list(), character())) }) @@ -46,13 +45,36 @@ test_that("stop_for_error() gives a custom error class with extra info", { had_error <- FALSE tryCatch({ db <- adbc_database_init(adbc_driver_void()) - adbc_database_release(db) - adbc_database_release(db) + adbc_database_get_option(db, "this option does not exist") }, adbc_status = function(e) { had_error <<- TRUE expect_s3_class(e, "adbc_status") - expect_s3_class(e, "adbc_status_invalid_state") - expect_identical(e$error$status, 6L) + expect_s3_class(e, "adbc_status_not_found") + expect_identical(e$error$status, 3L) + expect_identical( + e$error$detail[["adbc.r.option_key"]], + charToRaw("this option does not exist") + ) + }) + + expect_true(had_error) +}) + +test_that("void driver can report error to ADBC 1.0.0 structs", { + opts <- options(adbcdrivermanager.use_legacy_error = TRUE) + on.exit(options(opts)) + + had_error <- FALSE + tryCatch({ + db <- adbc_database_init(adbc_driver_void()) + adbc_database_get_option(db, "this option does not exist") + }, adbc_status = function(e) { + had_error <<- TRUE + expect_s3_class(e, "adbc_status") + expect_s3_class(e, "adbc_status_not_found") + expect_identical(e$error$status, 3L) + expect_identical(e$error$vendor_code, 0L) + expect_identical(e$error$details, setNames(list(), character())) }) expect_true(had_error) diff --git a/r/adbcdrivermanager/tests/testthat/test-options.R b/r/adbcdrivermanager/tests/testthat/test-options.R index 74ad63e811..b2faab4555 100644 --- a/r/adbcdrivermanager/tests/testthat/test-options.R +++ b/r/adbcdrivermanager/tests/testthat/test-options.R @@ -15,30 +15,66 @@ # specific language governing permissions and limitations # under the License. -test_that("get option methods work on a database for the void driver", { +test_that("get/set option can roundtrip string options for database", { db <- adbc_database_init(adbc_driver_void()) expect_error( adbc_database_get_option(db, "some_key"), class = "adbc_status_not_found" ) + adbc_database_set_options(db, list("some_key" = "some value")) + expect_identical( + adbc_database_get_option(db, "some_key"), + "some value" + ) +}) + +test_that("get/set option can roundtrip bytes options for database", { + db <- adbc_database_init(adbc_driver_void()) expect_error( - adbc_database_get_option_bytes(db, "some_key"), + adbc_database_get_option(db, "some_key"), class = "adbc_status_not_found" ) + adbc_database_set_options(db, list("some_key" = charToRaw("some value"))) + expect_identical( + adbc_database_get_option_bytes(db, "some_key"), + charToRaw("some value") + ) +}) + +test_that("get/set option can roundtrip integer options for database", { + db <- adbc_database_init(adbc_driver_void()) + expect_error( - adbc_database_get_option_int(db, "some_key"), + adbc_database_get_option(db, "some_key"), class = "adbc_status_not_found" ) + adbc_database_set_options(db, list("some_key" = 123L)) + expect_identical( + adbc_database_get_option_int(db, "some_key"), + 123L + ) +}) + +test_that("get/set option can roundtrip double options for database", { + db <- adbc_database_init(adbc_driver_void()) + expect_error( - adbc_database_get_option_double(db, "some_key"), + adbc_database_get_option(db, "some_key"), class = "adbc_status_not_found" ) + + adbc_database_set_options(db, list("some_key" = 123.4)) + expect_identical( + adbc_database_get_option_double(db, "some_key"), + 123.4 + ) }) -test_that("get option methods work on a connection for the void driver", { + +test_that("get/set option can roundtrip string options for connection", { db <- adbc_database_init(adbc_driver_void()) con <- adbc_connection_init(db) @@ -47,23 +83,62 @@ test_that("get option methods work on a connection for the void driver", { class = "adbc_status_not_found" ) + adbc_connection_set_options(con, list("some_key" = "some value")) + expect_identical( + adbc_connection_get_option(con, "some_key"), + "some value" + ) +}) + +test_that("get/set option can roundtrip bytes options for connection", { + db <- adbc_database_init(adbc_driver_void()) + con <- adbc_connection_init(db) + expect_error( - adbc_connection_get_option_bytes(con, "some_key"), + adbc_connection_get_option(con, "some_key"), class = "adbc_status_not_found" ) + adbc_connection_set_options(con, list("some_key" = charToRaw("some value"))) + expect_identical( + adbc_connection_get_option_bytes(con, "some_key"), + charToRaw("some value") + ) +}) + +test_that("get/set option can roundtrip int options for connection", { + db <- adbc_database_init(adbc_driver_void()) + con <- adbc_connection_init(db) + expect_error( - adbc_connection_get_option_int(con, "some_key"), + adbc_connection_get_option(con, "some_key"), class = "adbc_status_not_found" ) + adbc_connection_set_options(con, list("some_key" = 123L)) + expect_identical( + adbc_connection_get_option_int(con, "some_key"), + 123L + ) +}) + +test_that("get/set option can roundtrip double options for connection", { + db <- adbc_database_init(adbc_driver_void()) + con <- adbc_connection_init(db) + expect_error( - adbc_connection_get_option_double(con, "some_key"), + adbc_connection_get_option(con, "some_key"), class = "adbc_status_not_found" ) + + adbc_connection_set_options(con, list("some_key" = 123.4)) + expect_identical( + adbc_connection_get_option_double(con, "some_key"), + 123.4 + ) }) -test_that("get option methods work on a statment for the void driver", { +test_that("get/set option can roundtrip string options for statement", { db <- adbc_database_init(adbc_driver_void()) con <- adbc_connection_init(db) stmt <- adbc_statement_init(con) @@ -73,18 +148,225 @@ test_that("get option methods work on a statment for the void driver", { class = "adbc_status_not_found" ) + adbc_statement_set_options(stmt, list("some_key" = "some value")) + expect_identical( + adbc_statement_get_option(stmt, "some_key"), + "some value" + ) +}) + +test_that("get/set option can roundtrip bytes options for statement", { + db <- adbc_database_init(adbc_driver_void()) + con <- adbc_connection_init(db) + stmt <- adbc_statement_init(con) + expect_error( - adbc_statement_get_option_bytes(stmt, "some_key"), + adbc_statement_get_option(stmt, "some_key"), class = "adbc_status_not_found" ) + adbc_statement_set_options(stmt, list("some_key" = charToRaw("some value"))) + expect_identical( + adbc_statement_get_option_bytes(stmt, "some_key"), + charToRaw("some value") + ) +}) + +test_that("get/set option can roundtrip int options for statement", { + db <- adbc_database_init(adbc_driver_void()) + con <- adbc_connection_init(db) + stmt <- adbc_statement_init(con) + expect_error( - adbc_statement_get_option_int(stmt, "some_key"), + adbc_statement_get_option(stmt, "some_key"), class = "adbc_status_not_found" ) + adbc_statement_set_options(stmt, list("some_key" = 123L)) + expect_identical( + adbc_statement_get_option_int(stmt, "some_key"), + 123L + ) +}) + +test_that("get/set option can roundtrip double options for statement", { + db <- adbc_database_init(adbc_driver_void()) + con <- adbc_connection_init(db) + stmt <- adbc_statement_init(con) + expect_error( + adbc_statement_get_option(stmt, "some_key"), + class = "adbc_status_not_found" + ) + + adbc_statement_set_options(stmt, list("some_key" = 123.4)) + expect_identical( adbc_statement_get_option_double(stmt, "some_key"), + 123.4 + ) +}) + +test_that("void driver errors getting string option of incorrect type", { + db <- adbc_database_init(adbc_driver_void()) + adbc_database_set_options(db, list("some_key" = "some value")) + + expect_error( + adbc_database_get_option_bytes(db, "some_key"), class = "adbc_status_not_found" ) + + expect_error( + adbc_database_get_option_int(db, "some_key"), + class = "adbc_status_not_found" + ) + + expect_error( + adbc_database_get_option_double(db, "some_key"), + class = "adbc_status_not_found" + ) +}) + +test_that("void driver errors getting bytes option of incorrect type", { + db <- adbc_database_init(adbc_driver_void()) + adbc_database_set_options(db, list("some_key" = charToRaw("some value"))) + + expect_error( + adbc_database_get_option(db, "some_key"), + class = "adbc_status_not_found" + ) + + expect_error( + adbc_database_get_option_int(db, "some_key"), + class = "adbc_status_not_found" + ) + + expect_error( + adbc_database_get_option_double(db, "some_key"), + class = "adbc_status_not_found" + ) +}) + +test_that("void driver errors getting integer option of incorrect type", { + 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" + ) + + expect_error( + adbc_database_get_option_double(db, "some_key"), + class = "adbc_status_not_found" + ) +}) + +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)) + + 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" + ) + + expect_error( + adbc_database_get_option_int(db, "some_key"), + class = "adbc_status_not_found" + ) +}) + +test_that("key_value_options works", { + expect_identical( + key_value_options(NULL), + structure(setNames(list(), character()), class = "adbc_options") + ) + + expect_identical( + key_value_options(c("key" = "value")), + structure(list("key" = "value"), class = "adbc_options") + ) + + # NULLs dropped + expect_identical( + key_value_options(list("key" = "value", "key2" = NULL)), + structure(list("key" = "value"), class = "adbc_options") + ) + + # Integers stay integers + expect_identical( + key_value_options(list("key" = 1L)), + structure(list("key" = 1L), class = "adbc_options") + ) + + # Doubles stay doubles + expect_identical( + key_value_options(list("key" = 1)), + structure(list("key" = 1), class = "adbc_options") + ) + + # Raw vectors are supported + expect_identical( + key_value_options(list("key" = as.raw(c(0x01, 0x05)))), + structure(list("key" = as.raw(c(0x01, 0x05))), class = "adbc_options") + ) + + # S3 objects converted to strings + expect_identical( + key_value_options(list("key" = as.Date("2000-01-01"))), + structure(list("key" = "2000-01-01"), class = "adbc_options") + ) + + # Can handle many options + expect_identical( + key_value_options(setNames(letters, letters)), + structure(as.list(setNames(letters, letters)), class = "adbc_options") + ) + + # adbc_options() arguments are appended + expect_identical( + key_value_options( + list( + "key" = "value", + key_value_options(list("key2" = "value2")), + "key3" = "value3" + ) + ), + structure( + list("key" = "value", "key2" = "value2", "key3" = "value3"), + class = "adbc_options" + ) + ) + + # Errors + expect_error( + key_value_options(list("value")), + "must be named" + ) + + expect_error( + key_value_options(list(key = environment())), + "Option of type 'environment' (key: 'key') not supported", + fixed = TRUE + ) + + expect_error( + key_value_options(setNames(list("value"), "")), + "must be named" + ) + + expect_error( + key_value_options(setNames(list("value"), NA_character_)), + "must be named" + ) }) diff --git a/r/adbcdrivermanager/tests/testthat/test-utils.R b/r/adbcdrivermanager/tests/testthat/test-utils.R index 739f6c2706..0a9c7a49d1 100644 --- a/r/adbcdrivermanager/tests/testthat/test-utils.R +++ b/r/adbcdrivermanager/tests/testthat/test-utils.R @@ -15,38 +15,6 @@ # specific language governing permissions and limitations # under the License. -test_that("key_value_options works", { - expect_identical( - key_value_options(NULL), - setNames(character(), character()) - ) - - expect_identical( - key_value_options(c("key" = "value")), - c("key" = "value") - ) - - expect_identical( - key_value_options(list("key" = "value")), - c("key" = "value") - ) - - expect_identical( - key_value_options(list("key" = "value", "key2" = NULL)), - c("key" = "value") - ) - - expect_error( - key_value_options(list("value")), - "must be named" - ) - - expect_error( - key_value_options(setNames(list("value"), "")), - "must be named" - ) -}) - test_that("external pointer embedded environment works", { db <- adbc_database_init(adbc_driver_void()) expect_identical(names(db), "driver") From 3a8f6dce064a254f5e547e5d9369629e7ae9dc3c Mon Sep 17 00:00:00 2001 From: vleslief-ms <87045788+vleslief-ms@users.noreply.github.com> Date: Thu, 26 Oct 2023 13:41:11 -0700 Subject: [PATCH 7/7] build(csharp): add Arrow as submodule proper for CSharp project (#1229) Can't build the C# project checking out from main directly at the moment. README mentioned the submodule for Arrow and it's included in .gitmodules, but none of it works unless the csharp's .gitignore has src/arrow removed so that it isn't ignored, and the repo can actually pull the submodule into the expected directory path. --- csharp/.gitignore | 1 - csharp/src/arrow | 1 + 2 files changed, 1 insertion(+), 1 deletion(-) create mode 160000 csharp/src/arrow diff --git a/csharp/.gitignore b/csharp/.gitignore index 8782b42804..e2b09a58d0 100644 --- a/csharp/.gitignore +++ b/csharp/.gitignore @@ -27,6 +27,5 @@ x64 *.csproj.user *.pass -src/arrow artifacts/ TestResults/ diff --git a/csharp/src/arrow b/csharp/src/arrow new file mode 160000 index 0000000000..57f643c2ce --- /dev/null +++ b/csharp/src/arrow @@ -0,0 +1 @@ +Subproject commit 57f643c2cecca729109daae18c7a64f3a37e76e4