You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@arrow.apache.org by li...@apache.org on 2022/07/22 16:53:10 UTC
[arrow-adbc] branch main updated: [C] Fix compatibility issues noticed with Ibis (#44)
This is an automated email from the ASF dual-hosted git repository.
lidavidm pushed a commit to branch main
in repository https://gitbox.apache.org/repos/asf/arrow-adbc.git
The following commit(s) were added to refs/heads/main by this push:
new 41daacc [C] Fix compatibility issues noticed with Ibis (#44)
41daacc is described below
commit 41daacca08db041b52b458503e713a80528ba65a
Author: David Li <li...@gmail.com>
AuthorDate: Fri Jul 22 12:53:05 2022 -0400
[C] Fix compatibility issues noticed with Ibis (#44)
---
c/driver_manager/adbc_driver_manager.cc | 2 ++
c/drivers/sqlite/sqlite.cc | 25 ++++++++++++++++++++++++-
c/drivers/sqlite/sqlite_test.cc | 25 +++++++++++++------------
3 files changed, 39 insertions(+), 13 deletions(-)
diff --git a/c/driver_manager/adbc_driver_manager.cc b/c/driver_manager/adbc_driver_manager.cc
index 91b2f79..a3f8bf9 100644
--- a/c/driver_manager/adbc_driver_manager.cc
+++ b/c/driver_manager/adbc_driver_manager.cc
@@ -652,6 +652,8 @@ AdbcStatusCode AdbcLoadDriver(const char* driver_name, const char* entrypoint,
}
if (!handle) {
SetError(error, error_message);
+ // AdbcDatabaseInit tries to call this if set
+ driver->release = nullptr;
return ADBC_STATUS_INTERNAL;
}
diff --git a/c/drivers/sqlite/sqlite.cc b/c/drivers/sqlite/sqlite.cc
index cb2cedb..ab2a2c7 100644
--- a/c/drivers/sqlite/sqlite.cc
+++ b/c/drivers/sqlite/sqlite.cc
@@ -120,6 +120,9 @@ AdbcStatusCode FromArrowStatus(const Status& status, struct AdbcError* error) {
}
std::shared_ptr<arrow::Schema> StatementToSchema(sqlite3_stmt* stmt) {
+ // TODO: this is fundamentally the wrong way to go about
+ // this. instead, we need to act like the CSV/JSON readers: sample
+ // several rows and dynamically update the column type as we go.
const int num_columns = sqlite3_column_count(stmt);
arrow::FieldVector fields(num_columns);
for (int i = 0; i < num_columns; i++) {
@@ -394,6 +397,8 @@ class SqliteStatementReader : public arrow::RecordBatchReader {
done_(false) {}
AdbcStatusCode Init(struct AdbcError* error) {
+ // TODO: this crashes if the statement is closed while the reader
+ // is still open.
// Step the statement and get the schema (SQLite doesn't
// necessarily know the schema until it begins to execute it)
@@ -451,6 +456,13 @@ class SqliteStatementReader : public arrow::RecordBatchReader {
for (int col = 0; col < schema_->num_fields(); col++) {
const auto& field = schema_->field(col);
switch (field->type()->id()) {
+ case arrow::Type::DOUBLE: {
+ // TODO: handle null values
+ const sqlite3_int64 value = sqlite3_column_double(stmt_, col);
+ ARROW_RETURN_NOT_OK(
+ dynamic_cast<arrow::DoubleBuilder*>(builders[col].get())->Append(value));
+ break;
+ }
case arrow::Type::INT64: {
// TODO: handle null values
const sqlite3_int64 value = sqlite3_column_int64(stmt_, col);
@@ -458,6 +470,12 @@ class SqliteStatementReader : public arrow::RecordBatchReader {
dynamic_cast<arrow::Int64Builder*>(builders[col].get())->Append(value));
break;
}
+ case arrow::Type::NA: {
+ // TODO: handle null values
+ ARROW_RETURN_NOT_OK(
+ dynamic_cast<arrow::NullBuilder*>(builders[col].get())->AppendNull());
+ break;
+ }
case arrow::Type::STRING: {
const char* value =
reinterpret_cast<const char*>(sqlite3_column_text(stmt_, col));
@@ -728,8 +746,13 @@ class SqliteStatementImpl {
auto* constraint_column_usage_fk_column_name = static_cast<arrow::StringBuilder*>(
constraint_column_usage_items->child_builder(3).get());
+ // TODO: filter properly, also implement other attached databases
if (!catalog || std::strlen(catalog) == 0) {
- ADBC_RETURN_NOT_OK(FromArrowStatus(catalog_name.AppendNull(), error));
+ // https://www.sqlite.org/cli.html
+ // > The ".databases" command shows a list of all databases open
+ // > in the current connection. There will always be at least
+ // > 2. The first one is "main", the original database opened.
+ ADBC_RETURN_NOT_OK(FromArrowStatus(catalog_name.Append("main"), error));
if (depth == ADBC_OBJECT_DEPTH_CATALOGS) {
ADBC_RETURN_NOT_OK(FromArrowStatus(catalog_schemas->AppendNull(), error));
diff --git a/c/drivers/sqlite/sqlite_test.cc b/c/drivers/sqlite/sqlite_test.cc
index 40bc77c..9f5035a 100644
--- a/c/drivers/sqlite/sqlite_test.cc
+++ b/c/drivers/sqlite/sqlite_test.cc
@@ -458,7 +458,7 @@ TEST_F(Sqlite, MetadataGetObjects) {
AdbcConnectionGetObjects(&connection, ADBC_OBJECT_DEPTH_CATALOGS, nullptr, nullptr,
nullptr, nullptr, nullptr, &statement, &error));
ReadStatement(&statement, &schema, &batches);
- EXPECT_THAT(batches, BatchesAre(catalog_schema, {R"([[null, null]])"}));
+ EXPECT_THAT(batches, BatchesAre(catalog_schema, {R"([["main", null]])"}));
batches.clear();
ADBC_ASSERT_OK_WITH_ERROR(error, AdbcStatementNew(&connection, &statement, &error));
@@ -479,8 +479,9 @@ TEST_F(Sqlite, MetadataGetObjects) {
ReadStatement(&statement, &schema, &batches);
EXPECT_THAT(
batches,
- BatchesAre(catalog_schema,
- {R"([[null, [{"db_schema_name": null, "db_schema_tables": null}]]])"}));
+ BatchesAre(
+ catalog_schema,
+ {R"([["main", [{"db_schema_name": null, "db_schema_tables": null}]]])"}));
batches.clear();
ADBC_ASSERT_OK_WITH_ERROR(error, AdbcStatementNew(&connection, &statement, &error));
@@ -489,7 +490,7 @@ TEST_F(Sqlite, MetadataGetObjects) {
AdbcConnectionGetObjects(&connection, ADBC_OBJECT_DEPTH_DB_SCHEMAS, nullptr,
"schema", nullptr, nullptr, nullptr, &statement, &error));
ReadStatement(&statement, &schema, &batches);
- EXPECT_THAT(batches, BatchesAre(catalog_schema, {R"([[null, []]])"}));
+ EXPECT_THAT(batches, BatchesAre(catalog_schema, {R"([["main", []]])"}));
batches.clear();
// Query for tables
@@ -501,7 +502,7 @@ TEST_F(Sqlite, MetadataGetObjects) {
ReadStatement(&statement, &schema, &batches);
EXPECT_THAT(batches,
BatchesAre(catalog_schema,
- {R"([[null, [{"db_schema_name": null, "db_schema_tables": [
+ {R"([["main", [{"db_schema_name": null, "db_schema_tables": [
{"table_name": "bulk_insert", "table_type": "table", "table_columns": null, "table_constraints": null}
]}]]])"}));
batches.clear();
@@ -514,7 +515,7 @@ TEST_F(Sqlite, MetadataGetObjects) {
ReadStatement(&statement, &schema, &batches);
EXPECT_THAT(batches,
BatchesAre(catalog_schema,
- {R"([[null, [{"db_schema_name": null, "db_schema_tables": [
+ {R"([["main", [{"db_schema_name": null, "db_schema_tables": [
{"table_name": "bulk_insert", "table_type": "table", "table_columns": null, "table_constraints": null}
]}]]])"}));
batches.clear();
@@ -528,7 +529,7 @@ TEST_F(Sqlite, MetadataGetObjects) {
EXPECT_THAT(
batches,
BatchesAre(catalog_schema,
- {R"([[null, [{"db_schema_name": null, "db_schema_tables": []}]]])"}));
+ {R"([["main", [{"db_schema_name": null, "db_schema_tables": []}]]])"}));
batches.clear();
// Query for table types
@@ -543,7 +544,7 @@ TEST_F(Sqlite, MetadataGetObjects) {
ReadStatement(&statement, &schema, &batches);
EXPECT_THAT(batches,
BatchesAre(catalog_schema,
- {R"([[null, [{"db_schema_name": null, "db_schema_tables": [
+ {R"([["main", [{"db_schema_name": null, "db_schema_tables": [
{
"table_name": "bulk_insert",
"table_type": "table",
@@ -567,7 +568,7 @@ TEST_F(Sqlite, MetadataGetObjects) {
EXPECT_THAT(
batches,
BatchesAre(catalog_schema,
- {R"([[null, [{"db_schema_name": null, "db_schema_tables": []}]]])"}));
+ {R"([["main", [{"db_schema_name": null, "db_schema_tables": []}]]])"}));
batches.clear();
// Query for columns
@@ -579,7 +580,7 @@ TEST_F(Sqlite, MetadataGetObjects) {
ReadStatement(&statement, &schema, &batches);
EXPECT_THAT(batches,
BatchesAre(catalog_schema,
- {R"([[null, [{"db_schema_name": null, "db_schema_tables": [
+ {R"([["main", [{"db_schema_name": null, "db_schema_tables": [
{
"table_name": "bulk_insert",
"table_type": "table",
@@ -600,7 +601,7 @@ TEST_F(Sqlite, MetadataGetObjects) {
ReadStatement(&statement, &schema, &batches);
EXPECT_THAT(batches,
BatchesAre(catalog_schema,
- {R"([[null, [{"db_schema_name": null, "db_schema_tables": [
+ {R"([["main", [{"db_schema_name": null, "db_schema_tables": [
{
"table_name": "bulk_insert",
"table_type": "table",
@@ -653,7 +654,7 @@ TEST_F(Sqlite, MetadataGetObjectsColumns) {
ReadStatement(&statement, &schema, &batches);
EXPECT_THAT(batches,
BatchesAre(catalog_schema,
- {R"([[null, [{"db_schema_name": null, "db_schema_tables": [
+ {R"([["main", [{"db_schema_name": null, "db_schema_tables": [
{
"table_name": "child",
"table_type": "table",