lidavidm commented on code in PR #900:
URL: https://github.com/apache/arrow-adbc/pull/900#discussion_r1261540990


##########
c/driver/sqlite/sqlite_test.cc:
##########
@@ -39,8 +39,7 @@ class SqliteQuirks : public adbc_validation::DriverQuirks {
   AdbcStatusCode SetupDatabase(struct AdbcDatabase* database,
                                struct AdbcError* error) const override {
     // Shared DB required for transaction tests
-    return AdbcDatabaseSetOption(
-        database, "uri", "file:Sqlite_Transactions?mode=memory&cache=shared", 
error);
+    return AdbcDatabaseSetOption(database, "uri", "file:Sqlite_Transactions", 
error);

Review Comment:
   Hmm, why was this changed? This is specifically to make sure two connections 
in the same process can share the same underlying database



##########
c/driver/postgresql/postgresql_test.cc:
##########
@@ -576,6 +576,91 @@ class PostgresStatementTest : public ::testing::Test,
   }
 
  protected:
+  void TestSqlIngestTemporalType(enum ArrowTimeUnit unit, const char* 
timezone) override {
+    if (!quirks()->supports_bulk_ingest()) {
+      GTEST_SKIP();
+    }
+
+    ASSERT_THAT(quirks()->DropTable(&connection, "bulk_ingest", &error),
+                adbc_validation::IsOkStatus(&error));
+
+    adbc_validation::Handle<struct ArrowSchema> schema;
+    adbc_validation::Handle<struct ArrowArray> array;
+    struct ArrowError na_error;
+    const std::vector<std::optional<int64_t>> values = {std::nullopt, -42, 0, 
42};
+    const ArrowType type = NANOARROW_TYPE_TIMESTAMP;
+
+    // much of this code is shared with TestSqlIngestType with minor
+    // changes to allow for various time units to be tested
+    ArrowSchemaInit(&schema.value);
+    ArrowSchemaSetTypeStruct(&schema.value, 1);
+    ArrowSchemaSetTypeDateTime(schema->children[0], type, unit, timezone);
+    ArrowSchemaSetName(schema->children[0], "col");
+    ASSERT_THAT(adbc_validation::MakeBatch<int64_t>(&schema.value, 
&array.value,
+                                                    &na_error, values),
+                adbc_validation::IsOkErrno());
+
+    ASSERT_THAT(AdbcStatementNew(&connection, &statement, &error),
+                adbc_validation::IsOkStatus(&error));
+    ASSERT_THAT(AdbcStatementSetOption(&statement, 
ADBC_INGEST_OPTION_TARGET_TABLE,
+                                       "bulk_ingest", &error),
+                adbc_validation::IsOkStatus(&error));
+    ASSERT_THAT(AdbcStatementBind(&statement, &array.value, &schema.value, 
&error),
+                adbc_validation::IsOkStatus(&error));
+
+    int64_t rows_affected = 0;
+    ASSERT_THAT(AdbcStatementExecuteQuery(&statement, nullptr, &rows_affected, 
&error),
+                adbc_validation::IsOkStatus(&error));
+    ASSERT_THAT(rows_affected,
+                ::testing::AnyOf(::testing::Eq(values.size()), 
::testing::Eq(-1)));
+
+    ASSERT_THAT(AdbcStatementSetSqlQuery(
+                    &statement,
+                    "SELECT * FROM bulk_ingest ORDER BY \"col\" ASC NULLS 
FIRST", &error),
+                adbc_validation::IsOkStatus(&error));
+    {
+      adbc_validation::StreamReader reader;
+      ASSERT_THAT(AdbcStatementExecuteQuery(&statement, &reader.stream.value,
+                                            &reader.rows_affected, &error),
+                  adbc_validation::IsOkStatus(&error));
+      ASSERT_THAT(reader.rows_affected,
+                  ::testing::AnyOf(::testing::Eq(values.size()), 
::testing::Eq(-1)));
+
+      ASSERT_NO_FATAL_FAILURE(reader.GetSchema());
+
+      ArrowType round_trip_type = quirks()->IngestSelectRoundTripType(type);
+      ASSERT_NO_FATAL_FAILURE(adbc_validation::CompareSchema(
+          &reader.schema.value, {{"col", round_trip_type, 
/*NULLABLE=*/true}}));
+
+      ASSERT_NO_FATAL_FAILURE(reader.Next());
+      ASSERT_NE(nullptr, reader.array->release);
+      ASSERT_EQ(values.size(), reader.array->length);
+      ASSERT_EQ(1, reader.array->n_children);
+
+      std::vector<std::optional<int64_t>> expected;

Review Comment:
   I think that'd be preferable over copy-pasting the entire test (effectively) 
into each subclass



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]

Reply via email to