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 2023/06/14 13:14:24 UTC
[arrow-adbc] branch main updated: fix(c/driver/sqlite): support PRIMARY KEY constraint in GetObjects (#777)
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 17177460 fix(c/driver/sqlite): support PRIMARY KEY constraint in GetObjects (#777)
17177460 is described below
commit 171774606908f50949fe243579c7a8afb9cd7144
Author: David Li <li...@gmail.com>
AuthorDate: Wed Jun 14 09:14:17 2023 -0400
fix(c/driver/sqlite): support PRIMARY KEY constraint in GetObjects (#777)
Fixes #666.
---
c/driver/postgresql/postgresql_test.cc | 8 ++++
c/driver/sqlite/sqlite.c | 4 +-
c/driver/sqlite/sqlite_test.cc | 10 +++++
c/driver_manager/CMakeLists.txt | 1 +
c/validation/CMakeLists.txt | 3 +-
c/validation/adbc_validation.cc | 73 ++++++++++++++++++++++++++++++++++
c/validation/adbc_validation.h | 18 ++++++++-
7 files changed, 113 insertions(+), 4 deletions(-)
diff --git a/c/driver/postgresql/postgresql_test.cc b/c/driver/postgresql/postgresql_test.cc
index fe1eb3fc..0f66f2d3 100644
--- a/c/driver/postgresql/postgresql_test.cc
+++ b/c/driver/postgresql/postgresql_test.cc
@@ -67,6 +67,14 @@ class PostgresQuirks : public adbc_validation::DriverQuirks {
return "$" + std::to_string(index + 1);
}
+ std::optional<std::string> PrimaryKeyTableDdl(std::string_view name) const override {
+ std::string ddl = "CREATE TABLE ";
+ ddl += name;
+ ddl += " (id SERIAL PRIMARY KEY)";
+ return ddl;
+ }
+
+ std::string catalog() const override { return "postgres"; }
std::string db_schema() const override { return "public"; }
};
diff --git a/c/driver/sqlite/sqlite.c b/c/driver/sqlite/sqlite.c
index 1eacbf39..023c0762 100644
--- a/c/driver/sqlite/sqlite.c
+++ b/c/driver/sqlite/sqlite.c
@@ -441,7 +441,7 @@ AdbcStatusCode SqliteConnectionGetConstraintsImpl(
sqlite3_stmt* fk_stmt, struct AdbcError* error) {
struct ArrowArray* table_constraints_items = table_constraints_col->children[0];
struct ArrowArray* constraint_name_col = table_constraints_items->children[0];
- // Constraints type column would be table_constraints_items->children[1];
+ struct ArrowArray* constraint_type_col = table_constraints_items->children[1];
struct ArrowArray* constraint_column_names_col = table_constraints_items->children[2];
struct ArrowArray* constraint_column_names_items =
constraint_column_names_col->children[0];
@@ -469,7 +469,7 @@ AdbcStatusCode SqliteConnectionGetConstraintsImpl(
has_primary_key = 1;
CHECK_NA(INTERNAL, ArrowArrayAppendNull(constraint_name_col, 1), error);
CHECK_NA(INTERNAL,
- ArrowArrayAppendString(constraint_name_col, ArrowCharView("PRIMARY KEY")),
+ ArrowArrayAppendString(constraint_type_col, ArrowCharView("PRIMARY KEY")),
error);
}
CHECK_NA(
diff --git a/c/driver/sqlite/sqlite_test.cc b/c/driver/sqlite/sqlite_test.cc
index 5bcca0f7..8a580cdb 100644
--- a/c/driver/sqlite/sqlite_test.cc
+++ b/c/driver/sqlite/sqlite_test.cc
@@ -64,7 +64,17 @@ class SqliteQuirks : public adbc_validation::DriverQuirks {
}
}
+ std::optional<std::string> PrimaryKeyTableDdl(std::string_view name) const override {
+ std::string ddl = "CREATE TABLE ";
+ ddl += name;
+ ddl += " (id INTEGER PRIMARY KEY)";
+ return ddl;
+ }
+
bool supports_concurrent_statements() const override { return true; }
+
+ std::string catalog() const override { return "main"; }
+ std::string db_schema() const override { return ""; }
};
class SqliteDatabaseTest : public ::testing::Test, public adbc_validation::DatabaseTest {
diff --git a/c/driver_manager/CMakeLists.txt b/c/driver_manager/CMakeLists.txt
index eb5a8640..dd28470c 100644
--- a/c/driver_manager/CMakeLists.txt
+++ b/c/driver_manager/CMakeLists.txt
@@ -58,6 +58,7 @@ if(ADBC_BUILD_TESTS)
../validation/adbc_validation.cc
../validation/adbc_validation_util.cc
EXTRA_LINK_LIBS
+ adbc_driver_common
nanoarrow
${TEST_LINK_LIBS})
target_compile_features(adbc-driver-manager-test PRIVATE cxx_std_17)
diff --git a/c/validation/CMakeLists.txt b/c/validation/CMakeLists.txt
index a7cacd60..3c83f95c 100644
--- a/c/validation/CMakeLists.txt
+++ b/c/validation/CMakeLists.txt
@@ -20,4 +20,5 @@ target_compile_features(adbc_validation PRIVATE cxx_std_17)
target_include_directories(adbc_validation SYSTEM
PRIVATE "${REPOSITORY_ROOT}" "${REPOSITORY_ROOT}/c/driver/"
"${REPOSITORY_ROOT}/c/vendor/")
-target_link_libraries(adbc_validation PUBLIC nanoarrow GTest::gtest GTest::gmock)
+target_link_libraries(adbc_validation PUBLIC adbc_driver_common nanoarrow GTest::gtest
+ GTest::gmock)
diff --git a/c/validation/adbc_validation.cc b/c/validation/adbc_validation.cc
index 8c25f11f..2d88347c 100644
--- a/c/validation/adbc_validation.cc
+++ b/c/validation/adbc_validation.cc
@@ -838,6 +838,79 @@ void ConnectionTest::TestMetadataGetObjectsConstraints() {
// TODO: can't be done portably (need to create tables with primary keys and such)
}
+void ConnectionTest::TestMetadataGetObjectsPrimaryKey() {
+ ASSERT_THAT(AdbcConnectionNew(&connection, &error), IsOkStatus(&error));
+ ASSERT_THAT(AdbcConnectionInit(&connection, &database, &error), IsOkStatus(&error));
+
+ if (!quirks()->supports_get_objects()) {
+ GTEST_SKIP();
+ }
+
+ std::optional<std::string> maybe_ddl = quirks()->PrimaryKeyTableDdl("adbc_pkey_test");
+ if (!maybe_ddl.has_value()) {
+ GTEST_SKIP();
+ }
+ std::string ddl = std::move(*maybe_ddl);
+
+ ASSERT_THAT(quirks()->DropTable(&connection, "adbc_pkey_test", &error),
+ IsOkStatus(&error));
+
+ {
+ Handle<AdbcStatement> statement;
+ ASSERT_THAT(AdbcStatementNew(&connection, &statement.value, &error),
+ IsOkStatus(&error));
+ ASSERT_THAT(AdbcStatementSetSqlQuery(&statement.value, ddl.c_str(), &error),
+ IsOkStatus(&error));
+ int64_t rows_affected = 0;
+ ASSERT_THAT(
+ AdbcStatementExecuteQuery(&statement.value, nullptr, &rows_affected, &error),
+ IsOkStatus(&error));
+ }
+
+ adbc_validation::StreamReader reader;
+ ASSERT_THAT(
+ AdbcConnectionGetObjects(&connection, ADBC_OBJECT_DEPTH_ALL, nullptr, nullptr,
+ nullptr, nullptr, nullptr, &reader.stream.value, &error),
+ IsOkStatus(&error));
+ ASSERT_NO_FATAL_FAILURE(reader.GetSchema());
+ ASSERT_NO_FATAL_FAILURE(reader.Next());
+ ASSERT_NE(nullptr, reader.array->release);
+ ASSERT_GT(reader.array->length, 0);
+
+ auto get_objects_data = adbc_validation::GetObjectsReader{&reader.array_view.value};
+ ASSERT_NE(*get_objects_data, nullptr)
+ << "could not initialize the AdbcGetObjectsData object";
+
+ struct AdbcGetObjectsTable* table =
+ AdbcGetObjectsDataGetTableByName(*get_objects_data, quirks()->catalog().c_str(),
+ quirks()->db_schema().c_str(), "adbc_pkey_test");
+ ASSERT_NE(table, nullptr) << "could not find adbc_pkey_test table";
+
+ ASSERT_EQ(table->n_table_columns, 1);
+ struct AdbcGetObjectsColumn* column = AdbcGetObjectsDataGetColumnByName(
+ *get_objects_data, quirks()->catalog().c_str(), quirks()->db_schema().c_str(),
+ "adbc_pkey_test", "id");
+ ASSERT_NE(column, nullptr) << "could not find id column on adbc_pkey_test table";
+
+ ASSERT_EQ(table->n_table_constraints, 1)
+ << "expected 1 constraint on adbc_pkey_test table, found: "
+ << table->n_table_constraints;
+
+ struct AdbcGetObjectsConstraint* constraint = table->table_constraints[0];
+
+ std::string_view constraint_type(constraint->constraint_type.data,
+ constraint->constraint_type.size_bytes);
+ ASSERT_EQ(constraint_type, "PRIMARY KEY");
+ ASSERT_EQ(constraint->n_column_names, 1)
+ << "expected constraint adbc_pkey_test_pkey to be applied to 1 column, found: "
+ << constraint->n_column_names;
+
+ std::string_view constraint_column_name(
+ constraint->constraint_column_names[0].data,
+ constraint->constraint_column_names[0].size_bytes);
+ ASSERT_EQ(constraint_column_name, "id");
+}
+
//------------------------------------------------------------
// Tests of AdbcStatement
diff --git a/c/validation/adbc_validation.h b/c/validation/adbc_validation.h
index d3026e0a..aa49fd40 100644
--- a/c/validation/adbc_validation.h
+++ b/c/validation/adbc_validation.h
@@ -60,6 +60,15 @@ class DriverQuirks {
const std::string& name,
struct AdbcError* error) const;
+ /// \brief Get the statement to create a table with a primary key, or nullopt if not
+ /// supported.
+ ///
+ /// The table should have one column:
+ /// - "id" with Arrow type int64 (primary key)
+ virtual std::optional<std::string> PrimaryKeyTableDdl(std::string_view name) const {
+ return std::nullopt;
+ }
+
/// \brief Return the SQL to reference the bind parameter of the given index
virtual std::string BindParameter(int index) const { return "?"; }
@@ -94,6 +103,9 @@ class DriverQuirks {
/// \brief Whether dynamic parameter bindings are supported for prepare
virtual bool supports_dynamic_parameter_binding() const { return true; }
+ /// \brief Default catalog to use for tests
+ virtual std::string catalog() const { return ""; }
+
/// \brief Default Schema to use for tests
virtual std::string db_schema() const { return ""; }
};
@@ -145,6 +157,7 @@ class ConnectionTest {
void TestMetadataGetObjectsTablesTypes();
void TestMetadataGetObjectsColumns();
void TestMetadataGetObjectsConstraints();
+ void TestMetadataGetObjectsPrimaryKey();
protected:
struct AdbcError error;
@@ -170,7 +183,10 @@ class ConnectionTest {
TestMetadataGetObjectsTablesTypes(); \
} \
TEST_F(FIXTURE, MetadataGetObjectsColumns) { TestMetadataGetObjectsColumns(); } \
- TEST_F(FIXTURE, MetadataGetObjectsConstraints) { TestMetadataGetObjectsConstraints(); }
+ TEST_F(FIXTURE, MetadataGetObjectsConstraints) { \
+ TestMetadataGetObjectsConstraints(); \
+ } \
+ TEST_F(FIXTURE, MetadataGetObjectsPrimaryKey) { TestMetadataGetObjectsPrimaryKey(); }
class StatementTest {
public: