Merge remote-tracking branch 'upstream/main' into interval-impl

.github/workflows/integration.yml

@@ -246,6 +246,7 @@ jobs:
         shell: bash -l {0}
         env:
           BUILD_ALL: "0"
+          BUILD_DRIVER_MANAGER: "1"
           BUILD_DRIVER_POSTGRESQL: "1"
         run: |
           ./ci/scripts/python_build.sh "$(pwd)" "$(pwd)/build"
@@ -307,8 +308,9 @@ jobs:
         shell: bash -l {0}
         env:
           BUILD_ALL: "0"
+          BUILD_DRIVER_MANAGER: "1"
           BUILD_DRIVER_SNOWFLAKE: "1"
           ADBC_SNOWFLAKE_URI: ${{ secrets.SNOWFLAKE_URI }}
         run: |
           ./ci/scripts/python_build.sh "$(pwd)" "$(pwd)/build"
-          ./ci/scripts/python_test.sh "$(pwd)" "$(pwd)/build"
+          env BUILD_DRIVER_MANAGER=0 ./ci/scripts/python_test.sh "$(pwd)" "$(pwd)/build"

.github/workflows/native-unix.yml

@@ -25,6 +25,7 @@ on:
       - "adbc.h"
       - "c/**"
       - "ci/**"
+      - "docs/**"
       - "glib/**"
       - "go/**"
       - "python/**"
@@ -36,6 +37,7 @@ on:
       - "adbc.h"
       - "c/**"
       - "ci/**"
+      - "docs/**"
       - "glib/**"
       - "go/**"
       - "python/**"
@@ -216,7 +218,8 @@ jobs:
         shell: bash -l {0}
         run: |
           mamba install -c conda-forge \
-            --file ci/conda_env_cpp.txt
+            --file ci/conda_env_cpp.txt \
+            --file ci/conda_env_cpp_lint.txt
 
       - name: clang-tidy
         shell: bash -l {0}
@@ -569,8 +572,11 @@ jobs:
         shell: bash -l {0}
         run: |
           ./ci/scripts/docs_build.sh "$(pwd)"
-      # Docs requires Python packages since it runs doctests
-      - name: Test Docs
+      - name: Test Recipes (C++)
+        shell: bash -l {0}
+        run: |
+          ./ci/scripts/cpp_recipe.sh $(pwd) ~/local build/recipe
+      - name: Test Recipes (Python)
         shell: bash -l {0}
         env:
           ADBC_POSTGRESQL_TEST_URI: "postgres://localhost:5432/postgres?user=postgres&password=password"

c/cmake_modules/AdbcDefines.cmake

@@ -75,7 +75,12 @@ if(MSVC)
 elseif(CMAKE_CXX_COMPILER_ID STREQUAL "AppleClang"
        OR CMAKE_CXX_COMPILER_ID STREQUAL "Clang"
        OR CMAKE_CXX_COMPILER_ID STREQUAL "GNU")
-  set(ADBC_C_CXX_FLAGS_CHECKIN -Wall -Werror)
+  set(ADBC_C_CXX_FLAGS_CHECKIN
+      -Wall
+      -Wextra
+      -Wpedantic
+      -Werror
+      -Wno-unused-parameter)
   set(ADBC_C_CXX_FLAGS_PRODUCTION -Wall)
 else()
   message(WARNING "Unknown compiler: ${CMAKE_CXX_COMPILER_ID}")

c/driver/flightsql/dremio_flightsql_test.cc

@@ -87,6 +87,8 @@ class DremioFlightSqlStatementTest : public ::testing::Test,
   void SetUp() override { ASSERT_NO_FATAL_FAILURE(SetUpTest()); }
   void TearDown() override { ASSERT_NO_FATAL_FAILURE(TearDownTest()); }
 
+  void TestSqlIngestTableEscaping() { GTEST_SKIP() << "Table escaping not implemented"; }
+
  protected:
   DremioFlightSqlQuirks quirks_;
 };

c/driver/flightsql/sqlite_flightsql_test.cc

@@ -228,6 +228,8 @@ class SqliteFlightSqlStatementTest : public ::testing::Test,
   void SetUp() override { ASSERT_NO_FATAL_FAILURE(SetUpTest()); }
   void TearDown() override { ASSERT_NO_FATAL_FAILURE(TearDownTest()); }
 
+  void TestSqlIngestTableEscaping() { GTEST_SKIP() << "Table escaping not implemented"; }
+
  protected:
   SqliteFlightSqlQuirks quirks_;
 };

c/driver/postgresql/connection.cc

@@ -242,7 +242,8 @@ class PqGetObjectsHelper {
   AdbcStatusCode AppendSchemas(std::string db_name) {
     // postgres only allows you to list schemas for the currently connected db
     if (!strcmp(db_name.c_str(), PQdb(conn_))) {
-      struct StringBuilder query = {0};
+      struct StringBuilder query;
+      std::memset(&query, 0, sizeof(query));
       if (StringBuilderInit(&query, /*initial_size*/ 256)) {
         return ADBC_STATUS_INTERNAL;
       }
@@ -291,7 +292,8 @@ class PqGetObjectsHelper {
   }
 
   AdbcStatusCode AppendCatalogs() {
-    struct StringBuilder query = {0};
+    struct StringBuilder query;
+    std::memset(&query, 0, sizeof(query));
     if (StringBuilderInit(&query, /*initial_size=*/256) != 0) return ADBC_STATUS_INTERNAL;
 
     if (StringBuilderAppend(&query, "%s", "SELECT datname FROM pg_catalog.pg_database")) {
@@ -330,7 +332,8 @@ class PqGetObjectsHelper {
   }
 
   AdbcStatusCode AppendTables(std::string schema_name) {
-    struct StringBuilder query = {0};
+    struct StringBuilder query;
+    std::memset(&query, 0, sizeof(query));
     if (StringBuilderInit(&query, /*initial_size*/ 512)) {
       return ADBC_STATUS_INTERNAL;
     }
@@ -429,7 +432,8 @@ class PqGetObjectsHelper {
   }
 
   AdbcStatusCode AppendColumns(std::string schema_name, std::string table_name) {
-    struct StringBuilder query = {0};
+    struct StringBuilder query;
+    std::memset(&query, 0, sizeof(query));
     if (StringBuilderInit(&query, /*initial_size*/ 512)) {
       return ADBC_STATUS_INTERNAL;
     }
@@ -516,7 +520,8 @@ class PqGetObjectsHelper {
   }
 
   AdbcStatusCode AppendConstraints(std::string schema_name, std::string table_name) {
-    struct StringBuilder query = {0};
+    struct StringBuilder query;
+    std::memset(&query, 0, sizeof(query));
     if (StringBuilderInit(&query, /*initial_size*/ 4096)) {
       return ADBC_STATUS_INTERNAL;
     }
@@ -796,8 +801,10 @@ AdbcStatusCode PostgresConnection::GetInfo(struct AdbcConnection* connection,
     info_codes_length = sizeof(kSupportedInfoCodes) / sizeof(kSupportedInfoCodes[0]);
   }
 
-  struct ArrowSchema schema = {0};
-  struct ArrowArray array = {0};
+  struct ArrowSchema schema;
+  std::memset(&schema, 0, sizeof(schema));
+  struct ArrowArray array;
+  std::memset(&array, 0, sizeof(array));
 
   AdbcStatusCode status =
       PostgresConnectionGetInfoImpl(codes, info_codes_length, &schema, &array, error);
@@ -814,8 +821,10 @@ AdbcStatusCode PostgresConnection::GetObjects(
     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* out, struct AdbcError* error) {
-  struct ArrowSchema schema = {0};
-  struct ArrowArray array = {0};
+  struct ArrowSchema schema;
+  std::memset(&schema, 0, sizeof(schema));
+  struct ArrowArray array;
+  std::memset(&array, 0, sizeof(array));
 
   PqGetObjectsHelper helper =
       PqGetObjectsHelper(conn_, depth, catalog, db_schema, table_name, table_types,
@@ -837,7 +846,8 @@ AdbcStatusCode PostgresConnection::GetTableSchema(const char* catalog,
                                                   struct ArrowSchema* schema,
                                                   struct AdbcError* error) {
   AdbcStatusCode final_status = ADBC_STATUS_OK;
-  struct StringBuilder query = {0};
+  struct StringBuilder query;
+  std::memset(&query, 0, sizeof(query));
   std::vector<std::string> params;
   if (StringBuilderInit(&query, /*initial_size=*/256) != 0) return ADBC_STATUS_INTERNAL;
 
@@ -937,8 +947,10 @@ AdbcStatusCode PostgresConnectionGetTableTypesImpl(struct ArrowSchema* schema,
 AdbcStatusCode PostgresConnection::GetTableTypes(struct AdbcConnection* connection,
                                                  struct ArrowArrayStream* out,
                                                  struct AdbcError* error) {
-  struct ArrowSchema schema = {0};
-  struct ArrowArray array = {0};
+  struct ArrowSchema schema;
+  std::memset(&schema, 0, sizeof(schema));
+  struct ArrowArray array;
+  std::memset(&array, 0, sizeof(array));
 
   AdbcStatusCode status = PostgresConnectionGetTableTypesImpl(&schema, &array, error);
   if (status != ADBC_STATUS_OK) {

c/driver/snowflake/snowflake_test.cc

@@ -178,6 +178,28 @@ class SnowflakeStatementTest : public ::testing::Test,
   }
 
  protected:
+  void ValidateIngestedTemporalData(struct ArrowArrayView* values,
+                                    enum ArrowTimeUnit unit,
+                                    const char* timezone) override {
+    std::vector<std::optional<int64_t>> expected;
+    switch (unit) {
+      case NANOARROW_TIME_UNIT_SECOND:
+        expected = {std::nullopt, -42, 0, 42};
+        break;
+      case NANOARROW_TIME_UNIT_MILLI:
+        expected = {std::nullopt, -42000, 0, 42000};
+        break;
+      case NANOARROW_TIME_UNIT_MICRO:
+        expected = {std::nullopt, -42, 0, 42};
+        break;
+      case NANOARROW_TIME_UNIT_NANO:
+        expected = {std::nullopt, -42, 0, 42};
+        break;
+    }
+    ASSERT_NO_FATAL_FAILURE(
+        adbc_validation::CompareArray<std::int64_t>(values, expected));
+  }
+
   SnowflakeQuirks quirks_;
 };
 ADBCV_TEST_STATEMENT(SnowflakeStatementTest)

c/driver/sqlite/sqlite.c

@@ -981,23 +981,22 @@ AdbcStatusCode SqliteStatementInitIngest(struct SqliteStatement* stmt,
   AdbcStatusCode code = ADBC_STATUS_OK;
 
   // Create statements for CREATE TABLE / INSERT
-  struct StringBuilder create_query = {0};
-  struct StringBuilder insert_query = {0};
-
-  if (StringBuilderInit(&create_query, /*initial_size=*/256) != 0) {
-    SetError(error, "[SQLite] Could not initiate StringBuilder");
+  sqlite3_str* create_query = sqlite3_str_new(NULL);
+  if (sqlite3_str_errcode(create_query)) {
+    SetError(error, "[SQLite] %s", sqlite3_errmsg(stmt->conn));
     return ADBC_STATUS_INTERNAL;
   }
+  struct StringBuilder insert_query = {0};
 
   if (StringBuilderInit(&insert_query, /*initial_size=*/256) != 0) {
     SetError(error, "[SQLite] Could not initiate StringBuilder");
-    StringBuilderReset(&create_query);
+    sqlite3_free(sqlite3_str_finish(create_query));
     return ADBC_STATUS_INTERNAL;
   }
 
-  if (StringBuilderAppend(&create_query, "%s%s%s", "CREATE TABLE ", stmt->target_table,
-                          " (") != 0) {
-    SetError(error, "[SQLite] Call to StringBuilderAppend failed");
+  sqlite3_str_appendf(create_query, "%s%Q%s", "CREATE TABLE ", stmt->target_table, " (");
+  if (sqlite3_str_errcode(create_query)) {
+    SetError(error, "[SQLite] %s", sqlite3_errmsg(stmt->conn));
     code = ADBC_STATUS_INTERNAL;
     goto cleanup;
   }
@@ -1010,11 +1009,18 @@ AdbcStatusCode SqliteStatementInitIngest(struct SqliteStatement* stmt,
   }
 
   for (int i = 0; i < stmt->binder.schema.n_children; i++) {
-    if (i > 0) StringBuilderAppend(&create_query, "%s", ", ");
-    // XXX: should escape the column name too
-    if (StringBuilderAppend(&create_query, "%s", stmt->binder.schema.children[i]->name) !=
-        0) {
-      SetError(error, "[SQLite] Call to StringBuilderAppend failed");
+    if (i > 0) {
+      sqlite3_str_appendf(create_query, "%s", ", ");
+      if (sqlite3_str_errcode(create_query)) {
+        SetError(error, "[SQLite] %s", sqlite3_errmsg(stmt->conn));
+        code = ADBC_STATUS_INTERNAL;
+        goto cleanup;
+      }
+    }
+
+    sqlite3_str_appendf(create_query, "%Q", stmt->binder.schema.children[i]->name);
+    if (sqlite3_str_errcode(create_query)) {
+      SetError(error, "[SQLite] %s", sqlite3_errmsg(stmt->conn));
       code = ADBC_STATUS_INTERNAL;
       goto cleanup;
     }
@@ -1033,8 +1039,10 @@ AdbcStatusCode SqliteStatementInitIngest(struct SqliteStatement* stmt,
       goto cleanup;
     }
   }
-  if (StringBuilderAppend(&create_query, "%s", ")") != 0) {
-    SetError(error, "[SQLite] Call to StringBuilderAppend failed");
+
+  sqlite3_str_appendchar(create_query, 1, ')');
+  if (sqlite3_str_errcode(create_query)) {
+    SetError(error, "[SQLite] %s", sqlite3_errmsg(stmt->conn));
     code = ADBC_STATUS_INTERNAL;
     goto cleanup;
   }
@@ -1048,15 +1056,17 @@ AdbcStatusCode SqliteStatementInitIngest(struct SqliteStatement* stmt,
   sqlite3_stmt* create = NULL;
   if (!stmt->append) {
     // Create table
-    int rc = sqlite3_prepare_v2(stmt->conn, create_query.buffer, (int)create_query.size,
-                                &create, /*pzTail=*/NULL);
+    int rc =
+        sqlite3_prepare_v2(stmt->conn, sqlite3_str_value(create_query),
+                           sqlite3_str_length(create_query), &create, /*pzTail=*/NULL);
     if (rc == SQLITE_OK) {
       rc = sqlite3_step(create);
     }
 
     if (rc != SQLITE_OK && rc != SQLITE_DONE) {
-      SetError(error, "[SQLite] Failed to create table: %s (executed '%s')",
-               sqlite3_errmsg(stmt->conn), create_query.buffer);
+      SetError(error, "[SQLite] Failed to create table: %s (executed '%.*s')",
+               sqlite3_errmsg(stmt->conn), sqlite3_str_length(create_query),
+               sqlite3_str_value(create_query));
       code = ADBC_STATUS_INTERNAL;
     }
   }
@@ -1074,7 +1084,7 @@ AdbcStatusCode SqliteStatementInitIngest(struct SqliteStatement* stmt,
   sqlite3_finalize(create);
 
 cleanup:
-  StringBuilderReset(&create_query);
+  sqlite3_free(sqlite3_str_finish(create_query));
   StringBuilderReset(&insert_query);
   return code;
 }
@@ -1091,7 +1101,10 @@ AdbcStatusCode SqliteStatementExecuteIngest(struct SqliteStatement* stmt,
   AdbcStatusCode status = SqliteStatementInitIngest(stmt, &insert, error);
 
   int64_t row_count = 0;
+  int is_autocommit = sqlite3_get_autocommit(stmt->conn);
   if (status == ADBC_STATUS_OK) {
+    if (is_autocommit) sqlite3_exec(stmt->conn, "BEGIN TRANSACTION", 0, 0, 0);
+
     while (1) {
       char finished = 0;
       status =
@@ -1110,6 +1123,8 @@ AdbcStatusCode SqliteStatementExecuteIngest(struct SqliteStatement* stmt,
       }
       row_count++;
     }
+
+    if (is_autocommit) sqlite3_exec(stmt->conn, "COMMIT", 0, 0, 0);
   }
 
   if (rows_affected) *rows_affected = row_count;

c/driver/sqlite/statement_reader.c

@@ -719,7 +719,10 @@ AdbcStatusCode StatementReaderAppendInt64ToBinary(struct ArrowBuffer* offsets,
   int written = 0;
   while (1) {
     written = snprintf(output, buffer_size, "%" PRId64, value);
-    if (written >= buffer_size) {
+    if (written < 0) {
+      SetError(error, "Encoding error when upcasting double to string");
+      return ADBC_STATUS_INTERNAL;
+    } else if (((size_t)written) >= buffer_size) {
       // Truncated, resize and try again
       // Check for overflow - presumably this can never happen...?
       if (UINT_MAX - buffer_size < buffer_size) {
@@ -749,7 +752,10 @@ AdbcStatusCode StatementReaderAppendDoubleToBinary(struct ArrowBuffer* offsets,
   int written = 0;
   while (1) {
     written = snprintf(output, buffer_size, "%e", value);
-    if (written >= buffer_size) {
+    if (written < 0) {
+      SetError(error, "Encoding error when upcasting double to string");
+      return ADBC_STATUS_INTERNAL;
+    } else if (((size_t)written) >= buffer_size) {
       // Truncated, resize and try again
       // Check for overflow - presumably this can never happen...?
       if (UINT_MAX - buffer_size < buffer_size) {
@@ -977,7 +983,7 @@ AdbcStatusCode AdbcSqliteExportReader(sqlite3* db, sqlite3_stmt* stmt,
 
   if (status == ADBC_STATUS_OK && !reader->done) {
     int64_t num_rows = 0;
-    while (num_rows < batch_size) {
+    while (((size_t)num_rows) < batch_size) {
       int rc = sqlite3_step(stmt);
       if (rc == SQLITE_DONE) {
         if (!binder) {

c/integration/duckdb/duckdb_test.cc

@@ -94,6 +94,8 @@ class DuckDbStatementTest : public ::testing::Test,
   // Accepts Prepare() without any query
   void TestSqlPrepareErrorNoQuery() { GTEST_SKIP(); }
 
+  void TestSqlIngestTableEscaping() { GTEST_SKIP() << "Table escaping not implemented"; }
+
  protected:
   DuckDbQuirks quirks_;
 };