From 537edfeb3aa64af54163ea913060bf8a3e196be5 Mon Sep 17 00:00:00 2001 From: Will Ayd Date: Fri, 14 Jul 2023 21:46:32 -0700 Subject: [PATCH 01/14] Compiling --- c/driver/postgresql/statement.cc | 33 +++++++++++----- c/validation/adbc_validation.cc | 65 +++++++++++++++++++++----------- c/validation/adbc_validation.h | 6 ++- 3 files changed, 70 insertions(+), 34 deletions(-) diff --git a/c/driver/postgresql/statement.cc b/c/driver/postgresql/statement.cc index 4cae15b631..4f345610c2 100644 --- a/c/driver/postgresql/statement.cc +++ b/c/driver/postgresql/statement.cc @@ -223,6 +223,9 @@ struct BindStream { type_id = PostgresTypeId::kTimestamp; param_lengths[i] = 8; break; + case ArrowType::NANOARROW_TYPE_DURATION: + type_id = PostgresTypeId::kInterval; + param_lengths[i] = 16; default: SetError(error, "%s%" PRIu64 "%s%s%s%s", "[libpq] Field #", static_cast(i + 1), " ('", bind_schema->children[i]->name, @@ -385,11 +388,10 @@ struct BindStream { param_values[col] = const_cast(view.data.as_char); break; } - case ArrowType::NANOARROW_TYPE_TIMESTAMP: { + case ArrowType::NANOARROW_TYPE_TIMESTAMP: + case ArrowType::NANOARROW_TYPE_DURATION: { int64_t val = array_view->children[col]->buffer_views[1].data.as_int64[row]; - // 2000-01-01 00:00:00.000000 in microseconds - constexpr int64_t kPostgresTimestampEpoch = 946684800000000; constexpr int64_t kSecOverflowLimit = 9223372036854; constexpr int64_t kmSecOverflowLimit = 9223372036854775; @@ -399,8 +401,7 @@ struct BindStream { if (abs(val) > kSecOverflowLimit) { SetError(error, "[libpq] Field #%" PRId64 "%s%s%s%" PRId64 "%s", col + 1, "('", bind_schema->children[col]->name, "') Row #", - row + 1, - "has value which exceeds postgres timestamp limits"); + row + 1, "has value which exceeds postgres temporal limits"); return ADBC_STATUS_INVALID_ARGUMENT; } val *= 1000000; @@ -409,8 +410,7 @@ struct BindStream { if (abs(val) > kmSecOverflowLimit) { SetError(error, "[libpq] Field #%" PRId64 "%s%s%s%" PRId64 "%s", col + 1, "('", bind_schema->children[col]->name, "') Row #", - row + 1, - "has value which exceeds postgres timestamp limits"); + row + 1, "has value which exceeds postgres temporal limits"); return ADBC_STATUS_INVALID_ARGUMENT; } val *= 1000; @@ -422,8 +422,20 @@ struct BindStream { break; } - const uint64_t value = ToNetworkInt64(val - kPostgresTimestampEpoch); - std::memcpy(param_values[col], &value, sizeof(int64_t)); + if (bind_schema_fields[col].type == ArrowType::NANOARROW_TYPE_TIMESTAMP) { + // 2000-01-01 00:00:00.000000 in microseconds + constexpr int64_t kPostgresTimestampEpoch = 946684800000000; + const uint64_t value = ToNetworkInt64(val - kPostgresTimestampEpoch); + std::memcpy(param_values[col], &value, sizeof(int64_t)); + } else if (bind_schema_fields[col].type == + ArrowType::NANOARROW_TYPE_TIMESTAMP) { + // postgres stores an interval as a 64 bit offset in microsecond + // resolution alongside a 32 bit day and 32 bit month + // for now we just send 0 for the day / month values + const uint64_t value = ToNetworkInt64(val); + std::memcpy(param_values[col], &value, sizeof(int64_t)); + std::memset(param_values[col] + sizeof(int64_t), 0, sizeof(int64_t)); + } break; } default: @@ -787,6 +799,9 @@ AdbcStatusCode PostgresStatement::CreateBulkTable( create += " TIMESTAMP"; } break; + case ArrowType::NANOARROW_TYPE_DURATION: + create += " INTERVAL"; + break; default: SetError(error, "%s%" PRIu64 "%s%s%s%s", "[libpq] Field #", static_cast(i + 1), " ('", source_schema.children[i]->name, diff --git a/c/validation/adbc_validation.cc b/c/validation/adbc_validation.cc index bba51588f4..0ba5759d5f 100644 --- a/c/validation/adbc_validation.cc +++ b/c/validation/adbc_validation.cc @@ -1095,8 +1095,9 @@ void StatementTest::TestSqlIngestBinary() { NANOARROW_TYPE_BINARY, {std::nullopt, "", "\x00\x01\x02\x04", "\xFE\xFF"})); } -template -void StatementTest::TestSqlIngestTemporalType(const char* timezone) { +template +void StatementTest::TestSqlIngestTemporalType(enum ArrowTimeUnit unit, + const char* timezone) { if (!quirks()->supports_bulk_ingest()) { GTEST_SKIP(); } @@ -1108,13 +1109,12 @@ void StatementTest::TestSqlIngestTemporalType(const char* timezone) { Handle array; struct ArrowError na_error; const std::vector> 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, TU, timezone); + ArrowSchemaSetTypeDateTime(schema->children[0], T, unit, timezone); ArrowSchemaSetName(schema->children[0], "col"); ASSERT_THAT(MakeBatch(&schema.value, &array.value, &na_error, values), IsOkErrno()); @@ -1146,7 +1146,7 @@ void StatementTest::TestSqlIngestTemporalType(const char* timezone) { ASSERT_NO_FATAL_FAILURE(reader.GetSchema()); - ArrowType round_trip_type = quirks()->IngestSelectRoundTripType(type); + ArrowType round_trip_type = quirks()->IngestSelectRoundTripType(T); ASSERT_NO_FATAL_FAILURE( CompareSchema(&reader.schema.value, {{"col", round_trip_type, NULLABLE}})); @@ -1155,7 +1155,7 @@ void StatementTest::TestSqlIngestTemporalType(const char* timezone) { ASSERT_EQ(values.size(), reader.array->length); ASSERT_EQ(1, reader.array->n_children); - ValidateIngestedTemporalData(reader.array_view->children[0], TU, timezone); + ValidateIngestedTemporalData(reader.array_view->children[0], unit, timezone); ASSERT_NO_FATAL_FAILURE(reader.Next()); ASSERT_EQ(nullptr, reader.array->release); @@ -1170,27 +1170,46 @@ void StatementTest::ValidateIngestedTemporalData(struct ArrowArrayView* values, FAIL() << "ValidateIngestedTemporalData is not implemented in the base class"; } +void StatementTest::TestSqlIngestDuration() { + ASSERT_NO_FATAL_FAILURE(TestSqlIngestTemporalType( + NANOARROW_TIME_UNIT_SECOND, nullptr)); + ASSERT_NO_FATAL_FAILURE(TestSqlIngestTemporalType( + NANOARROW_TIME_UNIT_MICRO, nullptr)); + ASSERT_NO_FATAL_FAILURE(TestSqlIngestTemporalType( + NANOARROW_TIME_UNIT_MILLI, nullptr)); + ASSERT_NO_FATAL_FAILURE(TestSqlIngestTemporalType( + NANOARROW_TIME_UNIT_NANO, nullptr)); +} + void StatementTest::TestSqlIngestTimestamp() { - ASSERT_NO_FATAL_FAILURE(TestSqlIngestTemporalType(nullptr)); - ASSERT_NO_FATAL_FAILURE(TestSqlIngestTemporalType(nullptr)); - ASSERT_NO_FATAL_FAILURE(TestSqlIngestTemporalType(nullptr)); - ASSERT_NO_FATAL_FAILURE(TestSqlIngestTemporalType(nullptr)); + ASSERT_NO_FATAL_FAILURE(TestSqlIngestTemporalType( + NANOARROW_TIME_UNIT_SECOND, nullptr)); + ASSERT_NO_FATAL_FAILURE(TestSqlIngestTemporalType( + NANOARROW_TIME_UNIT_MICRO, nullptr)); + ASSERT_NO_FATAL_FAILURE(TestSqlIngestTemporalType( + NANOARROW_TIME_UNIT_MILLI, nullptr)); + ASSERT_NO_FATAL_FAILURE(TestSqlIngestTemporalType( + NANOARROW_TIME_UNIT_NANO, nullptr)); } void StatementTest::TestSqlIngestTimestampTz() { - ASSERT_NO_FATAL_FAILURE(TestSqlIngestTemporalType("UTC")); - ASSERT_NO_FATAL_FAILURE(TestSqlIngestTemporalType("UTC")); - ASSERT_NO_FATAL_FAILURE(TestSqlIngestTemporalType("UTC")); - ASSERT_NO_FATAL_FAILURE(TestSqlIngestTemporalType("UTC")); - - ASSERT_NO_FATAL_FAILURE( - TestSqlIngestTemporalType("America/Los_Angeles")); - ASSERT_NO_FATAL_FAILURE( - TestSqlIngestTemporalType("America/Los_Angeles")); - ASSERT_NO_FATAL_FAILURE( - TestSqlIngestTemporalType("America/Los_Angeles")); - ASSERT_NO_FATAL_FAILURE( - TestSqlIngestTemporalType("America/Los_Angeles")); + ASSERT_NO_FATAL_FAILURE(TestSqlIngestTemporalType( + NANOARROW_TIME_UNIT_SECOND, nullptr)); + ASSERT_NO_FATAL_FAILURE(TestSqlIngestTemporalType( + NANOARROW_TIME_UNIT_MICRO, nullptr)); + ASSERT_NO_FATAL_FAILURE(TestSqlIngestTemporalType( + NANOARROW_TIME_UNIT_MILLI, nullptr)); + ASSERT_NO_FATAL_FAILURE(TestSqlIngestTemporalType( + NANOARROW_TIME_UNIT_NANO, nullptr)); + + ASSERT_NO_FATAL_FAILURE(TestSqlIngestTemporalType( + NANOARROW_TIME_UNIT_SECOND, "America/Los_Angeles")); + ASSERT_NO_FATAL_FAILURE(TestSqlIngestTemporalType( + NANOARROW_TIME_UNIT_MICRO, "America/Los_Angeles")); + ASSERT_NO_FATAL_FAILURE(TestSqlIngestTemporalType( + NANOARROW_TIME_UNIT_MILLI, "America/Los_Angeles")); + ASSERT_NO_FATAL_FAILURE(TestSqlIngestTemporalType( + NANOARROW_TIME_UNIT_NANO, "America/Los_Angeles")); } void StatementTest::TestSqlIngestAppend() { diff --git a/c/validation/adbc_validation.h b/c/validation/adbc_validation.h index aeafa9c3f6..ebff172125 100644 --- a/c/validation/adbc_validation.h +++ b/c/validation/adbc_validation.h @@ -230,6 +230,7 @@ class StatementTest { void TestSqlIngestBinary(); // Temporal + void TestSqlIngestDuration(); void TestSqlIngestTimestamp(); void TestSqlIngestTimestampTz(); @@ -274,8 +275,8 @@ class StatementTest { template void TestSqlIngestNumericType(ArrowType type); - template - void TestSqlIngestTemporalType(const char* timezone); + template + void TestSqlIngestTemporalType(enum ArrowTimeUnit, const char* timezone); virtual void ValidateIngestedTemporalData(struct ArrowArrayView* values, enum ArrowTimeUnit unit, @@ -299,6 +300,7 @@ class StatementTest { TEST_F(FIXTURE, SqlIngestFloat64) { TestSqlIngestFloat64(); } \ TEST_F(FIXTURE, SqlIngestString) { TestSqlIngestString(); } \ TEST_F(FIXTURE, SqlIngestBinary) { TestSqlIngestBinary(); } \ + TEST_F(FIXTURE, SqlIngestDuration) { TestSqlIngestDuration(); } \ TEST_F(FIXTURE, SqlIngestTimestamp) { TestSqlIngestTimestamp(); } \ TEST_F(FIXTURE, SqlIngestTimestampTz) { TestSqlIngestTimestampTz(); } \ TEST_F(FIXTURE, SqlIngestAppend) { TestSqlIngestAppend(); } \ From 8a0f3df1c079a06349857bb6b7843e0a870a34e6 Mon Sep 17 00:00:00 2001 From: Will Ayd Date: Fri, 14 Jul 2023 22:22:39 -0700 Subject: [PATCH 02/14] duration support --- c/driver/postgresql/postgres_copy_reader.h | 35 ++++++++++++++++++++++ c/driver/postgresql/postgres_type.h | 6 ++++ c/driver/postgresql/statement.cc | 3 +- 3 files changed, 43 insertions(+), 1 deletion(-) diff --git a/c/driver/postgresql/postgres_copy_reader.h b/c/driver/postgresql/postgres_copy_reader.h index 7d3844f86b..39bc3abede 100644 --- a/c/driver/postgresql/postgres_copy_reader.h +++ b/c/driver/postgresql/postgres_copy_reader.h @@ -212,6 +212,32 @@ class PostgresCopyNetworkEndianFieldReader : public PostgresCopyFieldReader { } }; +// Reader for Durations; Similar to PostgresCopyNetworkEndianFieldReader but +// discards an extra 64 bits per read (representing the postgres day / month) +class PostgresIntervalReader : public PostgresCopyFieldReader { + public: + ArrowErrorCode Read(ArrowBufferView* data, int32_t field_size_bytes, ArrowArray* array, + ArrowError* error) override { + if (field_size_bytes <= 0) { + return ArrowArrayAppendNull(array, 1); + } + + if (field_size_bytes != static_cast(sizeof(int64_t) * 2)) { + ArrowErrorSet(error, "Expected field with %d bytes but found field with %d bytes", + static_cast(sizeof(int64_t)), + static_cast(field_size_bytes)); // NOLINT(runtime/int) + return EINVAL; + } + + int64_t value = ReadUnsafe(data); + NANOARROW_RETURN_NOT_OK(ArrowBufferAppend(data_, &value, sizeof(int64_t))); + + // discard unnecessary bits + ReadUnsafe(data); + return AppendValid(array); + } +}; + // Converts COPY resulting from the Postgres NUMERIC type into a string. // Rewritten based on the Postgres implementation of NUMERIC cast to string in // src/backend/utils/adt/numeric.c : get_str_from_var() (Note that in the initial source, @@ -836,6 +862,15 @@ static inline ArrowErrorCode MakeCopyFieldReader(const PostgresType& pg_type, default: return ErrorCantConvert(error, pg_type, schema_view); } + case NANOARROW_TYPE_DURATION: + switch (pg_type.type_id()) { + case PostgresTypeId::kInterval: { + *out = new PostgresIntervalReader(); + return NANOARROW_OK; + } + default: + return ErrorCantConvert(error, pg_type, schema_view); + } default: return ErrorCantConvert(error, pg_type, schema_view); } diff --git a/c/driver/postgresql/postgres_type.h b/c/driver/postgresql/postgres_type.h index 7b4197bfbe..cc9b631e14 100644 --- a/c/driver/postgresql/postgres_type.h +++ b/c/driver/postgresql/postgres_type.h @@ -258,6 +258,12 @@ class PostgresType { NANOARROW_TIME_UNIT_MICRO, /*timezone=*/"UTC")); break; + case PostgresTypeId::kInterval: + NANOARROW_RETURN_NOT_OK( + ArrowSchemaSetTypeDateTime(schema, NANOARROW_TYPE_DURATION, + NANOARROW_TIME_UNIT_MICRO, /*timezone=*/nullptr)); + break; + // ---- Nested -------------------- case PostgresTypeId::kRecord: NANOARROW_RETURN_NOT_OK(ArrowSchemaSetTypeStruct(schema, n_children())); diff --git a/c/driver/postgresql/statement.cc b/c/driver/postgresql/statement.cc index 4f345610c2..98f3b1c5e3 100644 --- a/c/driver/postgresql/statement.cc +++ b/c/driver/postgresql/statement.cc @@ -226,6 +226,7 @@ struct BindStream { case ArrowType::NANOARROW_TYPE_DURATION: type_id = PostgresTypeId::kInterval; param_lengths[i] = 16; + break; default: SetError(error, "%s%" PRIu64 "%s%s%s%s", "[libpq] Field #", static_cast(i + 1), " ('", bind_schema->children[i]->name, @@ -428,7 +429,7 @@ struct BindStream { const uint64_t value = ToNetworkInt64(val - kPostgresTimestampEpoch); std::memcpy(param_values[col], &value, sizeof(int64_t)); } else if (bind_schema_fields[col].type == - ArrowType::NANOARROW_TYPE_TIMESTAMP) { + ArrowType::NANOARROW_TYPE_DURATION) { // postgres stores an interval as a 64 bit offset in microsecond // resolution alongside a 32 bit day and 32 bit month // for now we just send 0 for the day / month values From 45223e772724c305dcb93c3263751ee34b4499ae Mon Sep 17 00:00:00 2001 From: Will Ayd Date: Fri, 14 Jul 2023 22:27:33 -0700 Subject: [PATCH 03/14] not implemented skips --- c/driver/sqlite/sqlite_test.cc | 3 +++ c/driver_manager/adbc_driver_manager_test.cc | 3 +++ 2 files changed, 6 insertions(+) diff --git a/c/driver/sqlite/sqlite_test.cc b/c/driver/sqlite/sqlite_test.cc index 3ab21fe3fa..50fa5512b1 100644 --- a/c/driver/sqlite/sqlite_test.cc +++ b/c/driver/sqlite/sqlite_test.cc @@ -195,6 +195,9 @@ class SqliteStatementTest : public ::testing::Test, } void TestSqlIngestBinary() { GTEST_SKIP() << "Cannot ingest BINARY (not implemented)"; } + void TestSqlIngestDuration() { + GTEST_SKIP() << "Cannot ingest DURATION (not implemented)"; + } protected: void ValidateIngestedTemporalData(struct ArrowArrayView* values, diff --git a/c/driver_manager/adbc_driver_manager_test.cc b/c/driver_manager/adbc_driver_manager_test.cc index 4475bd1964..cad33bfbde 100644 --- a/c/driver_manager/adbc_driver_manager_test.cc +++ b/c/driver_manager/adbc_driver_manager_test.cc @@ -232,6 +232,9 @@ class SqliteStatementTest : public ::testing::Test, void TestSqlIngestTimestampTz() { GTEST_SKIP() << "Cannot ingest TIMESTAMP WITH TIMEZONE (not implemented)"; } + void TestSqlIngestDuration() { + GTEST_SKIP() << "Cannot ingest DURATION (not implemented)"; + } protected: SqliteQuirks quirks_; From ba468c3a7e7162519ffd61c1d4134b64e182d9e6 Mon Sep 17 00:00:00 2001 From: Will Ayd Date: Wed, 6 Sep 2023 11:56:02 -0400 Subject: [PATCH 04/14] better diff --- c/driver/postgresql/postgres_copy_reader.h | 1 + c/driver/postgresql/statement.cc | 5 ++--- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/c/driver/postgresql/postgres_copy_reader.h b/c/driver/postgresql/postgres_copy_reader.h index 81990b1616..81898874ec 100644 --- a/c/driver/postgresql/postgres_copy_reader.h +++ b/c/driver/postgresql/postgres_copy_reader.h @@ -278,6 +278,7 @@ class PostgresIntervalReader : public PostgresCopyFieldReader { } }; +// // Converts COPY resulting from the Postgres NUMERIC type into a string. // Rewritten based on the Postgres implementation of NUMERIC cast to string in // src/backend/utils/adt/numeric.c : get_str_from_var() (Note that in the initial source, // DEC_DIGITS is always 4 and DBASE is always 10000). diff --git a/c/driver/postgresql/statement.cc b/c/driver/postgresql/statement.cc index 74cfbec390..b933a64358 100644 --- a/c/driver/postgresql/statement.cc +++ b/c/driver/postgresql/statement.cc @@ -418,8 +418,8 @@ struct BindStream { std::memcpy(param_values[col], &value, sizeof(int32_t)); break; } - case ArrowType::NANOARROW_TYPE_TIMESTAMP: - case ArrowType::NANOARROW_TYPE_DURATION: { + case ArrowType::NANOARROW_TYPE_DURATION: + case ArrowType::NANOARROW_TYPE_TIMESTAMP: { int64_t val = array_view->children[col]->buffer_views[1].data.as_int64[row]; // 2000-01-01 00:00:00.000000 in microseconds @@ -430,7 +430,6 @@ struct BindStream { switch (unit) { case NANOARROW_TIME_UNIT_SECOND: - overflow_safe = psnip_safe_int64_mul(&val, val, 1000000); break; case NANOARROW_TIME_UNIT_MILLI: From c6cbc30bebf7036ccd24f7057a836c01913794f2 Mon Sep 17 00:00:00 2001 From: Will Ayd Date: Wed, 6 Sep 2023 13:13:14 -0400 Subject: [PATCH 05/14] more diff cleanup --- c/driver/postgresql/postgres_copy_reader.h | 35 --------- c/driver/sqlite/sqlite_test.cc | 4 +- c/driver_manager/adbc_driver_manager_test.cc | 83 ++++++++++---------- 3 files changed, 43 insertions(+), 79 deletions(-) diff --git a/c/driver/postgresql/postgres_copy_reader.h b/c/driver/postgresql/postgres_copy_reader.h index 81898874ec..5c7214dc04 100644 --- a/c/driver/postgresql/postgres_copy_reader.h +++ b/c/driver/postgresql/postgres_copy_reader.h @@ -252,32 +252,6 @@ class PostgresCopyIntervalFieldReader : public PostgresCopyFieldReader { } }; -// Reader for Durations; Similar to PostgresCopyNetworkEndianFieldReader but -// discards an extra 64 bits per read (representing the postgres day / month) -class PostgresIntervalReader : public PostgresCopyFieldReader { - public: - ArrowErrorCode Read(ArrowBufferView* data, int32_t field_size_bytes, ArrowArray* array, - ArrowError* error) override { - if (field_size_bytes <= 0) { - return ArrowArrayAppendNull(array, 1); - } - - if (field_size_bytes != static_cast(sizeof(int64_t) * 2)) { - ArrowErrorSet(error, "Expected field with %d bytes but found field with %d bytes", - static_cast(sizeof(int64_t)), - static_cast(field_size_bytes)); // NOLINT(runtime/int) - return EINVAL; - } - - int64_t value = ReadUnsafe(data); - NANOARROW_RETURN_NOT_OK(ArrowBufferAppend(data_, &value, sizeof(int64_t))); - - // discard unnecessary bits - ReadUnsafe(data); - return AppendValid(array); - } -}; - // // Converts COPY resulting from the Postgres NUMERIC type into a string. // Rewritten based on the Postgres implementation of NUMERIC cast to string in // src/backend/utils/adt/numeric.c : get_str_from_var() (Note that in the initial source, @@ -911,15 +885,6 @@ static inline ArrowErrorCode MakeCopyFieldReader(const PostgresType& pg_type, default: return ErrorCantConvert(error, pg_type, schema_view); } - case NANOARROW_TYPE_DURATION: - switch (pg_type.type_id()) { - case PostgresTypeId::kInterval: { - *out = new PostgresIntervalReader(); - return NANOARROW_OK; - } - default: - return ErrorCantConvert(error, pg_type, schema_view); - } default: return ErrorCantConvert(error, pg_type, schema_view); diff --git a/c/driver/sqlite/sqlite_test.cc b/c/driver/sqlite/sqlite_test.cc index 70cf7e7396..e2e85dbb12 100644 --- a/c/driver/sqlite/sqlite_test.cc +++ b/c/driver/sqlite/sqlite_test.cc @@ -574,8 +574,8 @@ TEST_F(SqliteReaderTest, InferOneParam) { ASSERT_THAT(adbc_validation::MakeSchema(&schema.value, {{"", NANOARROW_TYPE_INT64}}), IsOkErrno()); ASSERT_THAT( - adbc_validation::MakeBatch(&schema.value, &batch.value, - /*error=*/nullptr, {std::nullopt, 2, 4, -1}), + adbc_validation::MakeBatch(&schema.value, &batch.value, /*error=*/nullptr, + {std::nullopt, 2, 4, -1}), IsOkErrno()); ASSERT_NO_FATAL_FAILURE(Bind(&batch.value, &schema.value)); diff --git a/c/driver_manager/adbc_driver_manager_test.cc b/c/driver_manager/adbc_driver_manager_test.cc index 00fe073415..262171f1c7 100644 --- a/c/driver_manager/adbc_driver_manager_test.cc +++ b/c/driver_manager/adbc_driver_manager_test.cc @@ -272,51 +272,50 @@ class SqliteStatementTest : public ::testing::Test, } void TestSqlIngestDuration() { GTEST_SKIP() << "Cannot ingest DURATION (not implemented)"; - void TestSqlIngestInterval() { - GTEST_SKIP() << "Cannot ingest Interval (not implemented)"; - } + } + void TestSqlIngestInterval() { + GTEST_SKIP() << "Cannot ingest Interval (not implemented)"; + } - protected: - SqliteQuirks quirks_; - }; - ADBCV_TEST_STATEMENT(SqliteStatementTest) - - TEST(AdbcDriverManagerInternal, AdbcDriverManagerDefaultEntrypoint) { - for (const auto& driver : { - "adbc_driver_sqlite", - "adbc_driver_sqlite.dll", - "driver_sqlite", - "libadbc_driver_sqlite", - "libadbc_driver_sqlite.so", - "libadbc_driver_sqlite.so.6.0.0", - "/usr/lib/libadbc_driver_sqlite.so", - "/usr/lib/libadbc_driver_sqlite.so.6.0.0", - "C:\\System32\\adbc_driver_sqlite.dll", - }) { - SCOPED_TRACE(driver); - EXPECT_EQ("AdbcDriverSqliteInit", ::AdbcDriverManagerDefaultEntrypoint(driver)); - } + protected: + SqliteQuirks quirks_; +}; +ADBCV_TEST_STATEMENT(SqliteStatementTest) + +TEST(AdbcDriverManagerInternal, AdbcDriverManagerDefaultEntrypoint) { + for (const auto& driver : { + "adbc_driver_sqlite", + "adbc_driver_sqlite.dll", + "driver_sqlite", + "libadbc_driver_sqlite", + "libadbc_driver_sqlite.so", + "libadbc_driver_sqlite.so.6.0.0", + "/usr/lib/libadbc_driver_sqlite.so", + "/usr/lib/libadbc_driver_sqlite.so.6.0.0", + "C:\\System32\\adbc_driver_sqlite.dll", + }) { + SCOPED_TRACE(driver); + EXPECT_EQ("AdbcDriverSqliteInit", ::AdbcDriverManagerDefaultEntrypoint(driver)); + } - for (const auto& driver : { - "adbc_sqlite", - "sqlite", - "/usr/lib/sqlite.so", - "C:\\System32\\sqlite.dll", - }) { - SCOPED_TRACE(driver); - EXPECT_EQ("AdbcSqliteInit", ::AdbcDriverManagerDefaultEntrypoint(driver)); - } + for (const auto& driver : { + "adbc_sqlite", + "sqlite", + "/usr/lib/sqlite.so", + "C:\\System32\\sqlite.dll", + }) { + SCOPED_TRACE(driver); + EXPECT_EQ("AdbcSqliteInit", ::AdbcDriverManagerDefaultEntrypoint(driver)); + } - for (const auto& driver : { - "proprietary_engine", - "libproprietary_engine.so.6.0.0", - "/usr/lib/proprietary_engine.so", - "C:\\System32\\proprietary_engine.dll", - }) { - SCOPED_TRACE(driver); - EXPECT_EQ("AdbcProprietaryEngineInit", - ::AdbcDriverManagerDefaultEntrypoint(driver)); - } + for (const auto& driver : { + "proprietary_engine", + "libproprietary_engine.so.6.0.0", + "/usr/lib/proprietary_engine.so", + "C:\\System32\\proprietary_engine.dll", + }) { + SCOPED_TRACE(driver); + EXPECT_EQ("AdbcProprietaryEngineInit", ::AdbcDriverManagerDefaultEntrypoint(driver)); } } } // namespace adbc From 719c3d5d0ffd72d3fade7fe59ee27e03c5216fab Mon Sep 17 00:00:00 2001 From: Will Ayd Date: Wed, 6 Sep 2023 13:35:35 -0400 Subject: [PATCH 06/14] buildable --- c/driver/postgresql/postgresql_test.cc | 2 + c/validation/adbc_validation.cc | 74 +++++++++++++++++--------- c/validation/adbc_validation.h | 2 +- 3 files changed, 53 insertions(+), 25 deletions(-) diff --git a/c/driver/postgresql/postgresql_test.cc b/c/driver/postgresql/postgresql_test.cc index e49e8e2080..4a43cfcb1b 100644 --- a/c/driver/postgresql/postgresql_test.cc +++ b/c/driver/postgresql/postgresql_test.cc @@ -92,6 +92,8 @@ class PostgresQuirks : public adbc_validation::DriverQuirks { switch (ingest_type) { case NANOARROW_TYPE_INT8: return NANOARROW_TYPE_INT16; + case NANOARROW_TYPE_DURATION: + return NANOARROW_TYPE_INTERVAL_MONTH_DAY_NANO; default: return ingest_type; } diff --git a/c/validation/adbc_validation.cc b/c/validation/adbc_validation.cc index 692c984fb2..b551dd6744 100644 --- a/c/validation/adbc_validation.cc +++ b/c/validation/adbc_validation.cc @@ -1261,7 +1261,7 @@ void StatementTest::TestSqlIngestDate32() { ASSERT_NO_FATAL_FAILURE(TestSqlIngestNumericType(NANOARROW_TYPE_DATE32)); } -template +template void StatementTest::TestSqlIngestTimestampType(const char* timezone) { if (!quirks()->supports_bulk_ingest(ADBC_INGEST_OPTION_MODE_CREATE)) { GTEST_SKIP(); @@ -1279,7 +1279,7 @@ void StatementTest::TestSqlIngestTimestampType(const char* timezone) { // changes to allow for various time units to be tested ArrowSchemaInit(&schema.value); ArrowSchemaSetTypeStruct(&schema.value, 1); - ArrowSchemaSetTypeDateTime(schema->children[0], T, unit, timezone); + ArrowSchemaSetTypeDateTime(schema->children[0], type, TU, timezone); ArrowSchemaSetName(schema->children[0], "col"); ASSERT_THAT(MakeBatch(&schema.value, &array.value, &na_error, values), IsOkErrno()); @@ -1311,7 +1311,7 @@ void StatementTest::TestSqlIngestTimestampType(const char* timezone) { ASSERT_NO_FATAL_FAILURE(reader.GetSchema()); - ArrowType round_trip_type = quirks()->IngestSelectRoundTripType(T); + ArrowType round_trip_type = quirks()->IngestSelectRoundTripType(type); ASSERT_NO_FATAL_FAILURE( CompareSchema(&reader.schema.value, {{"col", round_trip_type, NULLABLE}})); @@ -1320,7 +1320,10 @@ void StatementTest::TestSqlIngestTimestampType(const char* timezone) { ASSERT_EQ(values.size(), reader.array->length); ASSERT_EQ(1, reader.array->n_children); - ValidateIngestedTimestampData(reader.array_view->children[0], TU, timezone); + // TODO: add duration round trip testing + if (type == NANOARROW_TYPE_DURATION) { + ValidateIngestedTimestampData(reader.array_view->children[0], TU, timezone); + } ASSERT_NO_FATAL_FAILURE(reader.Next()); ASSERT_EQ(nullptr, reader.array->release); @@ -1336,38 +1339,61 @@ void StatementTest::ValidateIngestedTimestampData(struct ArrowArrayView* values, } void StatementTest::TestSqlIngestDuration() { - ASSERT_NO_FATAL_FAILURE(TestSqlIngestTemporalType( - NANOARROW_TIME_UNIT_SECOND, nullptr)); - ASSERT_NO_FATAL_FAILURE(TestSqlIngestTemporalType( - NANOARROW_TIME_UNIT_MICRO, nullptr)); - ASSERT_NO_FATAL_FAILURE(TestSqlIngestTemporalType( - NANOARROW_TIME_UNIT_MILLI, nullptr)); - ASSERT_NO_FATAL_FAILURE(TestSqlIngestTemporalType( - NANOARROW_TIME_UNIT_NANO, nullptr)); + ASSERT_NO_FATAL_FAILURE( + (TestSqlIngestTimestampType( + nullptr))); + ASSERT_NO_FATAL_FAILURE( + (TestSqlIngestTimestampType( + nullptr))); + ASSERT_NO_FATAL_FAILURE( + (TestSqlIngestTimestampType( + nullptr))); + ASSERT_NO_FATAL_FAILURE( + (TestSqlIngestTimestampType( + nullptr))); } void StatementTest::TestSqlIngestTimestamp() { ASSERT_NO_FATAL_FAILURE( - TestSqlIngestTimestampType(nullptr)); - ASSERT_NO_FATAL_FAILURE(TestSqlIngestTimestampType(nullptr)); - ASSERT_NO_FATAL_FAILURE(TestSqlIngestTimestampType(nullptr)); - ASSERT_NO_FATAL_FAILURE(TestSqlIngestTimestampType(nullptr)); + (TestSqlIngestTimestampType( + nullptr))); + ASSERT_NO_FATAL_FAILURE( + (TestSqlIngestTimestampType( + nullptr))); + ASSERT_NO_FATAL_FAILURE( + (TestSqlIngestTimestampType( + nullptr))); + ASSERT_NO_FATAL_FAILURE( + (TestSqlIngestTimestampType( + nullptr))); } void StatementTest::TestSqlIngestTimestampTz() { - ASSERT_NO_FATAL_FAILURE(TestSqlIngestTimestampType("UTC")); - ASSERT_NO_FATAL_FAILURE(TestSqlIngestTimestampType("UTC")); - ASSERT_NO_FATAL_FAILURE(TestSqlIngestTimestampType("UTC")); - ASSERT_NO_FATAL_FAILURE(TestSqlIngestTimestampType("UTC")); + ASSERT_NO_FATAL_FAILURE( + (TestSqlIngestTimestampType( + "UTC"))); + ASSERT_NO_FATAL_FAILURE( + (TestSqlIngestTimestampType( + "UTC"))); + ASSERT_NO_FATAL_FAILURE( + (TestSqlIngestTimestampType( + "UTC"))); + ASSERT_NO_FATAL_FAILURE( + (TestSqlIngestTimestampType( + "UTC"))); ASSERT_NO_FATAL_FAILURE( - TestSqlIngestTimestampType("America/Los_Angeles")); + (TestSqlIngestTimestampType( + "America/Los_Angeles"))); ASSERT_NO_FATAL_FAILURE( - TestSqlIngestTimestampType("America/Los_Angeles")); + (TestSqlIngestTimestampType( + "America/Los_Angeles"))); ASSERT_NO_FATAL_FAILURE( - TestSqlIngestTimestampType("America/Los_Angeles")); + (TestSqlIngestTimestampType( + "America/Los_Angeles"))); ASSERT_NO_FATAL_FAILURE( - TestSqlIngestTimestampType("America/Los_Angeles")); + (TestSqlIngestTimestampType( + "America/Los_Angeles"))); } void StatementTest::TestSqlIngestInterval() { diff --git a/c/validation/adbc_validation.h b/c/validation/adbc_validation.h index 4ae4b503e5..89c1929997 100644 --- a/c/validation/adbc_validation.h +++ b/c/validation/adbc_validation.h @@ -332,7 +332,7 @@ class StatementTest { template void TestSqlIngestNumericType(ArrowType type); - template + template void TestSqlIngestTimestampType(const char* timezone); virtual void ValidateIngestedTimestampData(struct ArrowArrayView* values, From 6a93b2391399c88ff10eae9154178297242979e6 Mon Sep 17 00:00:00 2001 From: Will Ayd Date: Wed, 6 Sep 2023 13:48:40 -0400 Subject: [PATCH 07/14] passing test --- c/validation/adbc_validation.cc | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/c/validation/adbc_validation.cc b/c/validation/adbc_validation.cc index b551dd6744..0fac81bef4 100644 --- a/c/validation/adbc_validation.cc +++ b/c/validation/adbc_validation.cc @@ -1321,7 +1321,7 @@ void StatementTest::TestSqlIngestTimestampType(const char* timezone) { ASSERT_EQ(1, reader.array->n_children); // TODO: add duration round trip testing - if (type == NANOARROW_TYPE_DURATION) { + if (type != NANOARROW_TYPE_DURATION) { ValidateIngestedTimestampData(reader.array_view->children[0], TU, timezone); } From 52f55ab8f5089971f0c47ffb0c268500c9807083 Mon Sep 17 00:00:00 2001 From: Will Ayd Date: Wed, 6 Sep 2023 14:09:16 -0400 Subject: [PATCH 08/14] better test structure --- c/driver/postgresql/postgresql_test.cc | 39 ++++++++++++------- c/driver/sqlite/sqlite_test.cc | 54 ++++++++++++++++---------- c/validation/adbc_validation.cc | 7 +--- c/validation/adbc_validation.h | 2 +- 4 files changed, 60 insertions(+), 42 deletions(-) diff --git a/c/driver/postgresql/postgresql_test.cc b/c/driver/postgresql/postgresql_test.cc index 4a43cfcb1b..11cfae9712 100644 --- a/c/driver/postgresql/postgresql_test.cc +++ b/c/driver/postgresql/postgresql_test.cc @@ -798,26 +798,35 @@ class PostgresStatementTest : public ::testing::Test, } protected: - void ValidateIngestedTimestampData(struct ArrowArrayView* values, + void ValidateIngestedTimestampData(struct ArrowArrayView* values, ArrowType type, enum ArrowTimeUnit unit, const char* timezone) override { std::vector> expected; - switch (unit) { - case (NANOARROW_TIME_UNIT_SECOND): - expected.insert(expected.end(), {std::nullopt, -42000000, 0, 42000000}); - break; - case (NANOARROW_TIME_UNIT_MILLI): - expected.insert(expected.end(), {std::nullopt, -42000, 0, 42000}); - break; - case (NANOARROW_TIME_UNIT_MICRO): - expected.insert(expected.end(), {std::nullopt, -42, 0, 42}); - break; - case (NANOARROW_TIME_UNIT_NANO): - expected.insert(expected.end(), {std::nullopt, 0, 0, 0}); + switch (type) { + case NANOARROW_TYPE_DATE32: + case NANOARROW_TYPE_DATE64: { + switch (unit) { + case (NANOARROW_TIME_UNIT_SECOND): + expected.insert(expected.end(), {std::nullopt, -42000000, 0, 42000000}); + break; + case (NANOARROW_TIME_UNIT_MILLI): + expected.insert(expected.end(), {std::nullopt, -42000, 0, 42000}); + break; + case (NANOARROW_TIME_UNIT_MICRO): + expected.insert(expected.end(), {std::nullopt, -42, 0, 42}); + break; + case (NANOARROW_TIME_UNIT_NANO): + expected.insert(expected.end(), {std::nullopt, 0, 0, 0}); + break; + } + ASSERT_NO_FATAL_FAILURE( + adbc_validation::CompareArray(values, expected)); break; + } + default: + ASSERT_TRUE(false) << "ValidateIngestedTimestampData not implemented for type " + << type; } - ASSERT_NO_FATAL_FAILURE( - adbc_validation::CompareArray(values, expected)); } PostgresQuirks quirks_; diff --git a/c/driver/sqlite/sqlite_test.cc b/c/driver/sqlite/sqlite_test.cc index e2e85dbb12..dc00af4430 100644 --- a/c/driver/sqlite/sqlite_test.cc +++ b/c/driver/sqlite/sqlite_test.cc @@ -224,32 +224,44 @@ class SqliteStatementTest : public ::testing::Test, } protected: - void ValidateIngestedTimestampData(struct ArrowArrayView* values, + void ValidateIngestedTimestampData(struct ArrowArrayView* values, ArrowType type, enum ArrowTimeUnit unit, const char* timezone) override { std::vector> expected; - switch (unit) { - case (NANOARROW_TIME_UNIT_SECOND): - expected.insert(expected.end(), {std::nullopt, "1969-12-31T23:59:18", - "1970-01-01T00:00:00", "1970-01-01T00:00:42"}); - break; - case (NANOARROW_TIME_UNIT_MILLI): - expected.insert(expected.end(), - {std::nullopt, "1969-12-31T23:59:59.958", - "1970-01-01T00:00:00.000", "1970-01-01T00:00:00.042"}); - break; - case (NANOARROW_TIME_UNIT_MICRO): - expected.insert(expected.end(), - {std::nullopt, "1969-12-31T23:59:59.999958", - "1970-01-01T00:00:00.000000", "1970-01-01T00:00:00.000042"}); - break; - case (NANOARROW_TIME_UNIT_NANO): - expected.insert(expected.end(), {std::nullopt, "1969-12-31T23:59:59.999999958", - "1970-01-01T00:00:00.000000000", - "1970-01-01T00:00:00.000000042"}); + switch (type) { + case NANOARROW_TYPE_DATE32: + case NANOARROW_TYPE_DATE64: { + switch (unit) { + case (NANOARROW_TIME_UNIT_SECOND): + expected.insert(expected.end(), + {std::nullopt, "1969-12-31T23:59:18", "1970-01-01T00:00:00", + "1970-01-01T00:00:42"}); + break; + case (NANOARROW_TIME_UNIT_MILLI): + expected.insert(expected.end(), + {std::nullopt, "1969-12-31T23:59:59.958", + "1970-01-01T00:00:00.000", "1970-01-01T00:00:00.042"}); + break; + case (NANOARROW_TIME_UNIT_MICRO): + expected.insert(expected.end(), + {std::nullopt, "1969-12-31T23:59:59.999958", + "1970-01-01T00:00:00.000000", "1970-01-01T00:00:00.000042"}); + break; + case (NANOARROW_TIME_UNIT_NANO): + expected.insert( + expected.end(), + {std::nullopt, "1969-12-31T23:59:59.999999958", + "1970-01-01T00:00:00.000000000", "1970-01-01T00:00:00.000000042"}); + break; + } + ASSERT_NO_FATAL_FAILURE( + adbc_validation::CompareArray(values, expected)); break; + } + default: + ASSERT_TRUE(false) << "ValidateIngestedTimestampData not implemented for type " + << type; } - ASSERT_NO_FATAL_FAILURE(adbc_validation::CompareArray(values, expected)); } SqliteQuirks quirks_; diff --git a/c/validation/adbc_validation.cc b/c/validation/adbc_validation.cc index 0fac81bef4..0b00ec2017 100644 --- a/c/validation/adbc_validation.cc +++ b/c/validation/adbc_validation.cc @@ -1320,10 +1320,7 @@ void StatementTest::TestSqlIngestTimestampType(const char* timezone) { ASSERT_EQ(values.size(), reader.array->length); ASSERT_EQ(1, reader.array->n_children); - // TODO: add duration round trip testing - if (type != NANOARROW_TYPE_DURATION) { - ValidateIngestedTimestampData(reader.array_view->children[0], TU, timezone); - } + ValidateIngestedTimestampData(reader.array_view->children[0], type, TU, timezone); ASSERT_NO_FATAL_FAILURE(reader.Next()); ASSERT_EQ(nullptr, reader.array->release); @@ -1333,7 +1330,7 @@ void StatementTest::TestSqlIngestTimestampType(const char* timezone) { } void StatementTest::ValidateIngestedTimestampData(struct ArrowArrayView* values, - enum ArrowTimeUnit unit, + ArrowType type, enum ArrowTimeUnit unit, const char* timezone) { FAIL() << "ValidateIngestedTimestampData is not implemented in the base class"; } diff --git a/c/validation/adbc_validation.h b/c/validation/adbc_validation.h index 89c1929997..76d8082f51 100644 --- a/c/validation/adbc_validation.h +++ b/c/validation/adbc_validation.h @@ -336,7 +336,7 @@ class StatementTest { void TestSqlIngestTimestampType(const char* timezone); virtual void ValidateIngestedTimestampData(struct ArrowArrayView* values, - enum ArrowTimeUnit unit, + ArrowType type, enum ArrowTimeUnit unit, const char* timezone); }; From bd39fc9d4525a02ef8e77c6d2d678ed4fa3c2fe1 Mon Sep 17 00:00:00 2001 From: Will Ayd Date: Wed, 6 Sep 2023 14:22:28 -0400 Subject: [PATCH 09/14] improved test coverage --- c/driver/postgresql/postgresql_test.cc | 46 +++++++++++++++++++++++++- c/driver/sqlite/sqlite_test.cc | 2 +- 2 files changed, 46 insertions(+), 2 deletions(-) diff --git a/c/driver/postgresql/postgresql_test.cc b/c/driver/postgresql/postgresql_test.cc index 11cfae9712..e2073d9d60 100644 --- a/c/driver/postgresql/postgresql_test.cc +++ b/c/driver/postgresql/postgresql_test.cc @@ -801,10 +801,10 @@ class PostgresStatementTest : public ::testing::Test, void ValidateIngestedTimestampData(struct ArrowArrayView* values, ArrowType type, enum ArrowTimeUnit unit, const char* timezone) override { - std::vector> expected; switch (type) { case NANOARROW_TYPE_DATE32: case NANOARROW_TYPE_DATE64: { + std::vector> expected; switch (unit) { case (NANOARROW_TIME_UNIT_SECOND): expected.insert(expected.end(), {std::nullopt, -42000000, 0, 42000000}); @@ -823,6 +823,50 @@ class PostgresStatementTest : public ::testing::Test, adbc_validation::CompareArray(values, expected)); break; } + case NANOARROW_TYPE_DURATION: { + struct ArrowInterval neg_interval; + struct ArrowInterval zero_interval; + struct ArrowInterval pos_interval; + + ArrowIntervalInit(&neg_interval, type); + ArrowIntervalInit(&zero_interval, type); + ArrowIntervalInit(&pos_interval, type); + + neg_interval.months = 0; + neg_interval.days = 0; + zero_interval.months = 0; + zero_interval.days = 0; + pos_interval.months = 0; + pos_interval.days = 0; + + switch (unit) { + case (NANOARROW_TIME_UNIT_SECOND): + neg_interval.ns = -42000000; + zero_interval.ns = 0; + pos_interval.ns = 42000000; + break; + case (NANOARROW_TIME_UNIT_MILLI): + neg_interval.ns = -42000; + zero_interval.ns = 0; + pos_interval.ns = 42000; + break; + case (NANOARROW_TIME_UNIT_MICRO): + neg_interval.ns = -42; + zero_interval.ns = 0; + pos_interval.ns = 42; + break; + case (NANOARROW_TIME_UNIT_NANO): + neg_interval.ns = 0; + zero_interval.ns = 0; + pos_interval.ns = 0; + break; + } + const std::vector> expected = { + std::nullopt, &neg_interval, &zero_interval, &pos_interval}; + ASSERT_NO_FATAL_FAILURE( + adbc_validation::CompareArray(values, expected)); + break; + } default: ASSERT_TRUE(false) << "ValidateIngestedTimestampData not implemented for type " << type; diff --git a/c/driver/sqlite/sqlite_test.cc b/c/driver/sqlite/sqlite_test.cc index dc00af4430..c16d79f61b 100644 --- a/c/driver/sqlite/sqlite_test.cc +++ b/c/driver/sqlite/sqlite_test.cc @@ -227,10 +227,10 @@ class SqliteStatementTest : public ::testing::Test, void ValidateIngestedTimestampData(struct ArrowArrayView* values, ArrowType type, enum ArrowTimeUnit unit, const char* timezone) override { - std::vector> expected; switch (type) { case NANOARROW_TYPE_DATE32: case NANOARROW_TYPE_DATE64: { + std::vector> expected; switch (unit) { case (NANOARROW_TIME_UNIT_SECOND): expected.insert(expected.end(), From 9b662a894426018695e22495d390bee893ffde46 Mon Sep 17 00:00:00 2001 From: Will Ayd Date: Wed, 6 Sep 2023 18:13:09 -0400 Subject: [PATCH 10/14] passing test --- c/driver/postgresql/postgresql_test.cc | 13 +++++++------ 1 file changed, 7 insertions(+), 6 deletions(-) diff --git a/c/driver/postgresql/postgresql_test.cc b/c/driver/postgresql/postgresql_test.cc index e2073d9d60..b7b31fed7b 100644 --- a/c/driver/postgresql/postgresql_test.cc +++ b/c/driver/postgresql/postgresql_test.cc @@ -841,21 +841,22 @@ class PostgresStatementTest : public ::testing::Test, switch (unit) { case (NANOARROW_TIME_UNIT_SECOND): - neg_interval.ns = -42000000; + neg_interval.ns = -42000000000; zero_interval.ns = 0; - pos_interval.ns = 42000000; + pos_interval.ns = 42000000000; break; case (NANOARROW_TIME_UNIT_MILLI): - neg_interval.ns = -42000; + neg_interval.ns = -42000000; zero_interval.ns = 0; - pos_interval.ns = 42000; + pos_interval.ns = 42000000; break; case (NANOARROW_TIME_UNIT_MICRO): - neg_interval.ns = -42; + neg_interval.ns = -42000; zero_interval.ns = 0; - pos_interval.ns = 42; + pos_interval.ns = 42000; break; case (NANOARROW_TIME_UNIT_NANO): + // lower than us precision is lost neg_interval.ns = 0; zero_interval.ns = 0; pos_interval.ns = 0; From 22936f233b9422587ccb231f975010e8b58f7757 Mon Sep 17 00:00:00 2001 From: Will Ayd Date: Wed, 6 Sep 2023 18:17:48 -0400 Subject: [PATCH 11/14] reorder tests --- c/validation/adbc_validation.cc | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/c/validation/adbc_validation.cc b/c/validation/adbc_validation.cc index 0b00ec2017..9f07faec86 100644 --- a/c/validation/adbc_validation.cc +++ b/c/validation/adbc_validation.cc @@ -1340,10 +1340,10 @@ void StatementTest::TestSqlIngestDuration() { (TestSqlIngestTimestampType( nullptr))); ASSERT_NO_FATAL_FAILURE( - (TestSqlIngestTimestampType( + (TestSqlIngestTimestampType( nullptr))); ASSERT_NO_FATAL_FAILURE( - (TestSqlIngestTimestampType( + (TestSqlIngestTimestampType( nullptr))); ASSERT_NO_FATAL_FAILURE( (TestSqlIngestTimestampType( From 233132ce829500ae4cff487e4a3ad67c1f0c668a Mon Sep 17 00:00:00 2001 From: Will Ayd Date: Wed, 6 Sep 2023 18:47:59 -0400 Subject: [PATCH 12/14] naming cleanup --- c/driver/postgresql/postgresql_test.cc | 11 +++---- c/driver/sqlite/sqlite_test.cc | 11 +++---- c/validation/adbc_validation.cc | 44 +++++++++++++------------- c/validation/adbc_validation.h | 8 ++--- 4 files changed, 36 insertions(+), 38 deletions(-) diff --git a/c/driver/postgresql/postgresql_test.cc b/c/driver/postgresql/postgresql_test.cc index b7b31fed7b..fa95e9177c 100644 --- a/c/driver/postgresql/postgresql_test.cc +++ b/c/driver/postgresql/postgresql_test.cc @@ -798,12 +798,11 @@ class PostgresStatementTest : public ::testing::Test, } protected: - void ValidateIngestedTimestampData(struct ArrowArrayView* values, ArrowType type, - enum ArrowTimeUnit unit, - const char* timezone) override { + void ValidateIngestedTemporalData(struct ArrowArrayView* values, ArrowType type, + enum ArrowTimeUnit unit, + const char* timezone) override { switch (type) { - case NANOARROW_TYPE_DATE32: - case NANOARROW_TYPE_DATE64: { + case NANOARROW_TYPE_TIMESTAMP: { std::vector> expected; switch (unit) { case (NANOARROW_TIME_UNIT_SECOND): @@ -869,7 +868,7 @@ class PostgresStatementTest : public ::testing::Test, break; } default: - ASSERT_TRUE(false) << "ValidateIngestedTimestampData not implemented for type " + ASSERT_TRUE(false) << "ValidateIngestedTemporalData not implemented for type " << type; } } diff --git a/c/driver/sqlite/sqlite_test.cc b/c/driver/sqlite/sqlite_test.cc index c16d79f61b..b9eac4ace6 100644 --- a/c/driver/sqlite/sqlite_test.cc +++ b/c/driver/sqlite/sqlite_test.cc @@ -224,12 +224,11 @@ class SqliteStatementTest : public ::testing::Test, } protected: - void ValidateIngestedTimestampData(struct ArrowArrayView* values, ArrowType type, - enum ArrowTimeUnit unit, - const char* timezone) override { + void ValidateIngestedTemporalData(struct ArrowArrayView* values, ArrowType type, + enum ArrowTimeUnit unit, + const char* timezone) override { switch (type) { - case NANOARROW_TYPE_DATE32: - case NANOARROW_TYPE_DATE64: { + case NANOARROW_TYPE_TIMESTAMP: { std::vector> expected; switch (unit) { case (NANOARROW_TIME_UNIT_SECOND): @@ -259,7 +258,7 @@ class SqliteStatementTest : public ::testing::Test, break; } default: - ASSERT_TRUE(false) << "ValidateIngestedTimestampData not implemented for type " + ASSERT_TRUE(false) << "ValidateIngestedTemporalData not implemented for type " << type; } } diff --git a/c/validation/adbc_validation.cc b/c/validation/adbc_validation.cc index 9f07faec86..0833aa2d82 100644 --- a/c/validation/adbc_validation.cc +++ b/c/validation/adbc_validation.cc @@ -1262,7 +1262,7 @@ void StatementTest::TestSqlIngestDate32() { } template -void StatementTest::TestSqlIngestTimestampType(const char* timezone) { +void StatementTest::TestSqlIngestTemporalType(const char* timezone) { if (!quirks()->supports_bulk_ingest(ADBC_INGEST_OPTION_MODE_CREATE)) { GTEST_SKIP(); } @@ -1320,7 +1320,7 @@ void StatementTest::TestSqlIngestTimestampType(const char* timezone) { ASSERT_EQ(values.size(), reader.array->length); ASSERT_EQ(1, reader.array->n_children); - ValidateIngestedTimestampData(reader.array_view->children[0], type, TU, timezone); + ValidateIngestedTemporalData(reader.array_view->children[0], type, TU, timezone); ASSERT_NO_FATAL_FAILURE(reader.Next()); ASSERT_EQ(nullptr, reader.array->release); @@ -1329,67 +1329,67 @@ void StatementTest::TestSqlIngestTimestampType(const char* timezone) { ASSERT_THAT(AdbcStatementRelease(&statement, &error), IsOkStatus(&error)); } -void StatementTest::ValidateIngestedTimestampData(struct ArrowArrayView* values, - ArrowType type, enum ArrowTimeUnit unit, - const char* timezone) { - FAIL() << "ValidateIngestedTimestampData is not implemented in the base class"; +void StatementTest::ValidateIngestedTemporalData(struct ArrowArrayView* values, + ArrowType type, enum ArrowTimeUnit unit, + const char* timezone) { + FAIL() << "ValidateIngestedTemporalData is not implemented in the base class"; } void StatementTest::TestSqlIngestDuration() { ASSERT_NO_FATAL_FAILURE( - (TestSqlIngestTimestampType( + (TestSqlIngestTemporalType( nullptr))); ASSERT_NO_FATAL_FAILURE( - (TestSqlIngestTimestampType( + (TestSqlIngestTemporalType( nullptr))); ASSERT_NO_FATAL_FAILURE( - (TestSqlIngestTimestampType( + (TestSqlIngestTemporalType( nullptr))); ASSERT_NO_FATAL_FAILURE( - (TestSqlIngestTimestampType( + (TestSqlIngestTemporalType( nullptr))); } void StatementTest::TestSqlIngestTimestamp() { ASSERT_NO_FATAL_FAILURE( - (TestSqlIngestTimestampType( + (TestSqlIngestTemporalType( nullptr))); ASSERT_NO_FATAL_FAILURE( - (TestSqlIngestTimestampType( + (TestSqlIngestTemporalType( nullptr))); ASSERT_NO_FATAL_FAILURE( - (TestSqlIngestTimestampType( + (TestSqlIngestTemporalType( nullptr))); ASSERT_NO_FATAL_FAILURE( - (TestSqlIngestTimestampType( + (TestSqlIngestTemporalType( nullptr))); } void StatementTest::TestSqlIngestTimestampTz() { ASSERT_NO_FATAL_FAILURE( - (TestSqlIngestTimestampType( + (TestSqlIngestTemporalType( "UTC"))); ASSERT_NO_FATAL_FAILURE( - (TestSqlIngestTimestampType( + (TestSqlIngestTemporalType( "UTC"))); ASSERT_NO_FATAL_FAILURE( - (TestSqlIngestTimestampType( + (TestSqlIngestTemporalType( "UTC"))); ASSERT_NO_FATAL_FAILURE( - (TestSqlIngestTimestampType( + (TestSqlIngestTemporalType( "UTC"))); ASSERT_NO_FATAL_FAILURE( - (TestSqlIngestTimestampType( + (TestSqlIngestTemporalType( "America/Los_Angeles"))); ASSERT_NO_FATAL_FAILURE( - (TestSqlIngestTimestampType( + (TestSqlIngestTemporalType( "America/Los_Angeles"))); ASSERT_NO_FATAL_FAILURE( - (TestSqlIngestTimestampType( + (TestSqlIngestTemporalType( "America/Los_Angeles"))); ASSERT_NO_FATAL_FAILURE( - (TestSqlIngestTimestampType( + (TestSqlIngestTemporalType( "America/Los_Angeles"))); } diff --git a/c/validation/adbc_validation.h b/c/validation/adbc_validation.h index 76d8082f51..dda52eb25a 100644 --- a/c/validation/adbc_validation.h +++ b/c/validation/adbc_validation.h @@ -333,11 +333,11 @@ class StatementTest { void TestSqlIngestNumericType(ArrowType type); template - void TestSqlIngestTimestampType(const char* timezone); + void TestSqlIngestTemporalType(const char* timezone); - virtual void ValidateIngestedTimestampData(struct ArrowArrayView* values, - ArrowType type, enum ArrowTimeUnit unit, - const char* timezone); + virtual void ValidateIngestedTemporalData(struct ArrowArrayView* values, ArrowType type, + enum ArrowTimeUnit unit, + const char* timezone); }; #define ADBCV_TEST_STATEMENT(FIXTURE) \ From c74f2f0ada85b9c117b3f4f47b92eea7886bc3bb Mon Sep 17 00:00:00 2001 From: Will Ayd Date: Wed, 6 Sep 2023 21:01:09 -0400 Subject: [PATCH 13/14] snowflake test skip --- c/driver/snowflake/snowflake_test.cc | 45 +++++++++++++++++----------- 1 file changed, 27 insertions(+), 18 deletions(-) diff --git a/c/driver/snowflake/snowflake_test.cc b/c/driver/snowflake/snowflake_test.cc index 8c3cd72c8b..1d9f29f3ce 100644 --- a/c/driver/snowflake/snowflake_test.cc +++ b/c/driver/snowflake/snowflake_test.cc @@ -184,28 +184,37 @@ class SnowflakeStatementTest : public ::testing::Test, } void TestSqlIngestInterval() { GTEST_SKIP(); } + void TestSqlIngestDuration() { GTEST_SKIP(); } protected: - void ValidateIngestedTimestampData(struct ArrowArrayView* values, - enum ArrowTimeUnit unit, - const char* timezone) override { - std::vector> 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}; + void ValidateIngestedTemporalData(struct ArrowArrayView* values, ArrowType type, + enum ArrowTimeUnit unit, + const char* timezone) override { + switch (type) { + case NANOARROW_TYPE_TIMESTAMP: { + std::vector> 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(values, expected)); break; + } + default: + ASSERT_TRUE(false) << "ValidateIngestedTemporalData not implemented for type " + << type; } - ASSERT_NO_FATAL_FAILURE( - adbc_validation::CompareArray(values, expected)); } SnowflakeQuirks quirks_; From dac8ff849dc74e9c3208ef7adf1e7aeba2dce98d Mon Sep 17 00:00:00 2001 From: Will Ayd Date: Thu, 7 Sep 2023 11:12:12 -0400 Subject: [PATCH 14/14] use FAIL() macro --- c/driver/postgresql/postgresql_test.cc | 3 +-- c/driver/snowflake/snowflake_test.cc | 3 +-- c/driver/sqlite/sqlite_test.cc | 3 +-- 3 files changed, 3 insertions(+), 6 deletions(-) diff --git a/c/driver/postgresql/postgresql_test.cc b/c/driver/postgresql/postgresql_test.cc index fa95e9177c..e7cd4b1f86 100644 --- a/c/driver/postgresql/postgresql_test.cc +++ b/c/driver/postgresql/postgresql_test.cc @@ -868,8 +868,7 @@ class PostgresStatementTest : public ::testing::Test, break; } default: - ASSERT_TRUE(false) << "ValidateIngestedTemporalData not implemented for type " - << type; + FAIL() << "ValidateIngestedTemporalData not implemented for type " << type; } } diff --git a/c/driver/snowflake/snowflake_test.cc b/c/driver/snowflake/snowflake_test.cc index 1d9f29f3ce..7b5861cbf5 100644 --- a/c/driver/snowflake/snowflake_test.cc +++ b/c/driver/snowflake/snowflake_test.cc @@ -212,8 +212,7 @@ class SnowflakeStatementTest : public ::testing::Test, break; } default: - ASSERT_TRUE(false) << "ValidateIngestedTemporalData not implemented for type " - << type; + FAIL() << "ValidateIngestedTemporalData not implemented for type " << type; } } diff --git a/c/driver/sqlite/sqlite_test.cc b/c/driver/sqlite/sqlite_test.cc index b9eac4ace6..2d68cc34ec 100644 --- a/c/driver/sqlite/sqlite_test.cc +++ b/c/driver/sqlite/sqlite_test.cc @@ -258,8 +258,7 @@ class SqliteStatementTest : public ::testing::Test, break; } default: - ASSERT_TRUE(false) << "ValidateIngestedTemporalData not implemented for type " - << type; + FAIL() << "ValidateIngestedTemporalData not implemented for type " << type; } }