From eefacad39bf4df7698a34fcb5aaca05b3e7151ea Mon Sep 17 00:00:00 2001 From: Will Ayd Date: Fri, 28 Jul 2023 14:40:49 -0400 Subject: [PATCH 01/18] postgres date32 implementation --- c/driver/postgresql/statement.cc | 23 +++++++++++++++++++++++ c/validation/adbc_validation.cc | 9 +++++++++ c/validation/adbc_validation.h | 2 ++ 3 files changed, 34 insertions(+) diff --git a/c/driver/postgresql/statement.cc b/c/driver/postgresql/statement.cc index dd3fd82c8a..3452411db5 100644 --- a/c/driver/postgresql/statement.cc +++ b/c/driver/postgresql/statement.cc @@ -219,6 +219,10 @@ struct BindStream { type_id = PostgresTypeId::kBytea; param_lengths[i] = 0; break; + case ArrowType::NANOARROW_TYPE_DATE32: + type_id = PostgresTypeId::kDate; + param_lengths[i] = 4; + break; case ArrowType::NANOARROW_TYPE_TIMESTAMP: type_id = PostgresTypeId::kTimestamp; param_lengths[i] = 8; @@ -389,6 +393,22 @@ struct BindStream { param_values[col] = const_cast(view.data.as_char); break; } + case ArrowType::NANOARROW_TYPE_DATE32: { + // 2000-01-01 + constexpr int32_t kPostgresDateEpoch = 10957; + const int32_t raw_value = + array_view->children[col]->buffer_views[1].data.as_int32[row]; + if (raw_value < INT32_MIN + kPostgresDateEpoch) { + 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 date limits"); + return ADBC_STATUS_INVALID_ARGUMENT; + } + + const uint32_t value = ToNetworkInt32(raw_value - kPostgresDateEpoch); + std::memcpy(param_values[col], &value, sizeof(int32_t)); + break; + } case ArrowType::NANOARROW_TYPE_TIMESTAMP: { int64_t val = array_view->children[col]->buffer_views[1].data.as_int64[row]; @@ -801,6 +821,9 @@ AdbcStatusCode PostgresStatement::CreateBulkTable( case ArrowType::NANOARROW_TYPE_BINARY: create += " BYTEA"; break; + case ArrowType::NANOARROW_TYPE_DATE32: + create += " DATE"; + break; case ArrowType::NANOARROW_TYPE_TIMESTAMP: if (strcmp("", source_schema_fields[i].timezone)) { create += " TIMESTAMPTZ"; diff --git a/c/validation/adbc_validation.cc b/c/validation/adbc_validation.cc index 54f3981cce..609e05defd 100644 --- a/c/validation/adbc_validation.cc +++ b/c/validation/adbc_validation.cc @@ -1037,6 +1037,11 @@ void StatementTest::TestSqlIngestNumericType(ArrowType type) { // values. Likely a bug on our side, but for now, avoid them. values.push_back(static_cast(-1.5)); values.push_back(static_cast(1.5)); + } else if (type == ArrowType::NANOARROW_TYPE_DATE32) { + // Different databases may choose different epochs, so providing + // max values for DATE types is likely to cause overflows + values.push_back(static_cast(-20000)); + values.push_back(static_cast(20000)); } else { values.push_back(std::numeric_limits::lowest()); values.push_back(std::numeric_limits::max()); @@ -1095,6 +1100,10 @@ void StatementTest::TestSqlIngestBinary() { NANOARROW_TYPE_BINARY, {std::nullopt, "", "\x00\x01\x02\x04", "\xFE\xFF"})); } +void StatementTest::TestSqlIngestDate32() { + ASSERT_NO_FATAL_FAILURE(TestSqlIngestNumericType(NANOARROW_TYPE_DATE32)); +} + template void StatementTest::TestSqlIngestTemporalType(const char* timezone) { if (!quirks()->supports_bulk_ingest()) { diff --git a/c/validation/adbc_validation.h b/c/validation/adbc_validation.h index dc5d69c27d..bbe371195e 100644 --- a/c/validation/adbc_validation.h +++ b/c/validation/adbc_validation.h @@ -230,6 +230,7 @@ class StatementTest { void TestSqlIngestBinary(); // Temporal + void TestSqlIngestDate32(); void TestSqlIngestTimestamp(); void TestSqlIngestTimestampTz(); void TestSqlIngestInterval(); @@ -301,6 +302,7 @@ class StatementTest { TEST_F(FIXTURE, SqlIngestFloat64) { TestSqlIngestFloat64(); } \ TEST_F(FIXTURE, SqlIngestString) { TestSqlIngestString(); } \ TEST_F(FIXTURE, SqlIngestBinary) { TestSqlIngestBinary(); } \ + TEST_F(FIXTURE, SqlIngestDate32) { TestSqlIngestDate32(); } \ TEST_F(FIXTURE, SqlIngestTimestamp) { TestSqlIngestTimestamp(); } \ TEST_F(FIXTURE, SqlIngestTimestampTz) { TestSqlIngestTimestampTz(); } \ TEST_F(FIXTURE, SqlIngestInterval) { TestSqlIngestInterval(); } \ From be370557715a85ec9aa5f9739305bbc97034f838 Mon Sep 17 00:00:00 2001 From: Will Ayd Date: Fri, 28 Jul 2023 15:22:35 -0400 Subject: [PATCH 02/18] initial sqlite build pass --- c/driver/sqlite/sqlite_test.cc | 1 + c/driver/sqlite/statement_reader.c | 75 ++++++++++++++++++++++++++++++ 2 files changed, 76 insertions(+) diff --git a/c/driver/sqlite/sqlite_test.cc b/c/driver/sqlite/sqlite_test.cc index 00332066ad..3c5f9caf68 100644 --- a/c/driver/sqlite/sqlite_test.cc +++ b/c/driver/sqlite/sqlite_test.cc @@ -77,6 +77,7 @@ class SqliteQuirks : public adbc_validation::DriverQuirks { case NANOARROW_TYPE_FLOAT: case NANOARROW_TYPE_DOUBLE: return NANOARROW_TYPE_DOUBLE; + case NANOARROW_TYPE_DATE32: case NANOARROW_TYPE_TIMESTAMP: return NANOARROW_TYPE_STRING; default: diff --git a/c/driver/sqlite/statement_reader.c b/c/driver/sqlite/statement_reader.c index 366c0fa1f3..5ffb7e340f 100644 --- a/c/driver/sqlite/statement_reader.c +++ b/c/driver/sqlite/statement_reader.c @@ -93,6 +93,59 @@ AdbcStatusCode AdbcSqliteBinderSetArrayStream(struct AdbcSqliteBinder* binder, return AdbcSqliteBinderSet(binder, error); } +#define MILLISECONDS_PER_DAY 86400000; + +/* + Allocates to buf on success. Caller is responsible for freeing. + On failure sets error and contents of buf are undefined. +*/ +static AdbcStatusCode ArrowDate32ToIsoString(int32_t value, char** buf, + struct AdbcError* error) { + int strlen = 10; + +#if SIZEOF_TIME_T < 8 + if ((seconds > INT32_MAX / MILLISECONDS_PER_DAY) || + (seconds < INT32_MIN / MILLISECONDS_PER_DAY)) { + SetError(error, "Date %" PRId32 " exceeds platform time_t bounds", value); + + return ADBC_STATUS_INVALID_ARGUMENT; + } + const time_t time = (time_t)(value * MILLISECONDS_PER_DAY); +#else + const time_t time = value * MILLISECONDS_PER_DAY; +#endif + + struct tm broken_down_time; + +#if defined(_WIN32) + if (gmtime_s(&broken_down_time, &time) != 0) { + SetError(error, "Could not convert date %" PRId32 " with to broken down time", value); + + return ADBC_STATUS_INVALID_ARGUMENT; + } +#else + if (gmtime_r(&time, &broken_down_time) != &broken_down_time) { + SetError(error, "Could not convert date %" PRId32 " with to broken down time", value); + + return ADBC_STATUS_INVALID_ARGUMENT; + } +#endif + + char* tsstr = malloc(strlen + 1); + if (tsstr == NULL) { + return ADBC_STATUS_IO; + } + + if (strftime(tsstr, strlen + 1, "%Y-%m-%d", &broken_down_time) == 0) { + SetError(error, "Call to strftime for date %" PRId32 " with failed", value); + free(tsstr); + return ADBC_STATUS_INVALID_ARGUMENT; + } + + *buf = tsstr; + return ADBC_STATUS_OK; +} + /* Allocates to buf on success. Caller is responsible for freeing. On failure sets error and contents of buf are undefined. @@ -300,6 +353,28 @@ AdbcStatusCode AdbcSqliteBinderBindNext(struct AdbcSqliteBinder* binder, sqlite3 SQLITE_STATIC); break; } + case NANOARROW_TYPE_DATE32: { + int64_t value = + ArrowArrayViewGetIntUnsafe(binder->batch.children[col], binder->next_row); + char* tsstr; + + if ((value > INT32_MAX) || (value < INT32_MIN)) { + SetError(error, + "Column %d has value %" PRId64 + " which exceeds the expected range " + "for an Arrow DATE32 type", + col, value); + return ADBC_STATUS_INVALID_DATA; + } + + RAISE_ADBC(ArrowDate32ToIsoString((int32_t)value, &tsstr, error)); + // SQLITE_TRANSIENT ensures the value is copied during bind + status = + sqlite3_bind_text(stmt, col + 1, tsstr, strlen(tsstr), SQLITE_TRANSIENT); + + free(tsstr); + break; + } case NANOARROW_TYPE_TIMESTAMP: { struct ArrowSchemaView bind_schema_view; RAISE_ADBC(ArrowSchemaViewInit(&bind_schema_view, binder->schema.children[col], From 4c642143cd84534384d8c8e0e55398dc594a4d6d Mon Sep 17 00:00:00 2001 From: Will Ayd Date: Fri, 28 Jul 2023 15:27:14 -0400 Subject: [PATCH 03/18] Temporal -> Timestamp --- c/driver/postgresql/postgresql_test.cc | 6 ++--- c/driver/sqlite/sqlite_test.cc | 6 ++--- c/validation/adbc_validation.cc | 37 +++++++++++++------------- c/validation/adbc_validation.h | 8 +++--- 4 files changed, 29 insertions(+), 28 deletions(-) diff --git a/c/driver/postgresql/postgresql_test.cc b/c/driver/postgresql/postgresql_test.cc index 33115bcf8b..a826e17267 100644 --- a/c/driver/postgresql/postgresql_test.cc +++ b/c/driver/postgresql/postgresql_test.cc @@ -576,9 +576,9 @@ class PostgresStatementTest : public ::testing::Test, } protected: - void ValidateIngestedTemporalData(struct ArrowArrayView* values, - enum ArrowTimeUnit unit, - const char* timezone) override { + void ValidateIngestedTimestampData(struct ArrowArrayView* values, + enum ArrowTimeUnit unit, + const char* timezone) override { std::vector> expected; switch (unit) { case (NANOARROW_TIME_UNIT_SECOND): diff --git a/c/driver/sqlite/sqlite_test.cc b/c/driver/sqlite/sqlite_test.cc index 3c5f9caf68..b03158cca6 100644 --- a/c/driver/sqlite/sqlite_test.cc +++ b/c/driver/sqlite/sqlite_test.cc @@ -201,9 +201,9 @@ class SqliteStatementTest : public ::testing::Test, } protected: - void ValidateIngestedTemporalData(struct ArrowArrayView* values, - enum ArrowTimeUnit unit, - const char* timezone) override { + void ValidateIngestedTimestampData(struct ArrowArrayView* values, + enum ArrowTimeUnit unit, + const char* timezone) override { std::vector> expected; switch (unit) { case (NANOARROW_TIME_UNIT_SECOND): diff --git a/c/validation/adbc_validation.cc b/c/validation/adbc_validation.cc index 609e05defd..e84cc518f5 100644 --- a/c/validation/adbc_validation.cc +++ b/c/validation/adbc_validation.cc @@ -1105,7 +1105,7 @@ void StatementTest::TestSqlIngestDate32() { } template -void StatementTest::TestSqlIngestTemporalType(const char* timezone) { +void StatementTest::TestSqlIngestTimestampType(const char* timezone) { if (!quirks()->supports_bulk_ingest()) { GTEST_SKIP(); } @@ -1164,7 +1164,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); + ValidateIngestedTimestampData(reader.array_view->children[0], TU, timezone); ASSERT_NO_FATAL_FAILURE(reader.Next()); ASSERT_EQ(nullptr, reader.array->release); @@ -1173,33 +1173,34 @@ void StatementTest::TestSqlIngestTemporalType(const char* timezone) { ASSERT_THAT(AdbcStatementRelease(&statement, &error), IsOkStatus(&error)); } -void StatementTest::ValidateIngestedTemporalData(struct ArrowArrayView* values, - enum ArrowTimeUnit unit, - const char* timezone) { - FAIL() << "ValidateIngestedTemporalData is not implemented in the base class"; +void StatementTest::ValidateIngestedTimestampData(struct ArrowArrayView* values, + enum ArrowTimeUnit unit, + const char* timezone) { + FAIL() << "ValidateIngestedTimestampData is not implemented in the base class"; } 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( + 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(TestSqlIngestTemporalType("UTC")); - ASSERT_NO_FATAL_FAILURE(TestSqlIngestTemporalType("UTC")); - ASSERT_NO_FATAL_FAILURE(TestSqlIngestTemporalType("UTC")); - ASSERT_NO_FATAL_FAILURE(TestSqlIngestTemporalType("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( - TestSqlIngestTemporalType("America/Los_Angeles")); + TestSqlIngestTimestampType("America/Los_Angeles")); ASSERT_NO_FATAL_FAILURE( - TestSqlIngestTemporalType("America/Los_Angeles")); + TestSqlIngestTimestampType("America/Los_Angeles")); ASSERT_NO_FATAL_FAILURE( - TestSqlIngestTemporalType("America/Los_Angeles")); + TestSqlIngestTimestampType("America/Los_Angeles")); ASSERT_NO_FATAL_FAILURE( - TestSqlIngestTemporalType("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 bbe371195e..23dacb7f4b 100644 --- a/c/validation/adbc_validation.h +++ b/c/validation/adbc_validation.h @@ -278,11 +278,11 @@ class StatementTest { void TestSqlIngestNumericType(ArrowType type); template - void TestSqlIngestTemporalType(const char* timezone); + void TestSqlIngestTimestampType(const char* timezone); - virtual void ValidateIngestedTemporalData(struct ArrowArrayView* values, - enum ArrowTimeUnit unit, - const char* timezone); + virtual void ValidateIngestedTimestampData(struct ArrowArrayView* values, + enum ArrowTimeUnit unit, + const char* timezone); }; #define ADBCV_TEST_STATEMENT(FIXTURE) \ From 84b500f58dd9fb778f32bb721d7a81ab54e511ac Mon Sep 17 00:00:00 2001 From: Will Ayd Date: Fri, 28 Jul 2023 15:38:19 -0400 Subject: [PATCH 04/18] fix sqlite conversion --- c/driver/sqlite/statement_reader.c | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/c/driver/sqlite/statement_reader.c b/c/driver/sqlite/statement_reader.c index 5ffb7e340f..5c24e5246d 100644 --- a/c/driver/sqlite/statement_reader.c +++ b/c/driver/sqlite/statement_reader.c @@ -93,7 +93,7 @@ AdbcStatusCode AdbcSqliteBinderSetArrayStream(struct AdbcSqliteBinder* binder, return AdbcSqliteBinderSet(binder, error); } -#define MILLISECONDS_PER_DAY 86400000; +#define SECONDS_PER_DAY 86400; /* Allocates to buf on success. Caller is responsible for freeing. @@ -104,15 +104,15 @@ static AdbcStatusCode ArrowDate32ToIsoString(int32_t value, char** buf, int strlen = 10; #if SIZEOF_TIME_T < 8 - if ((seconds > INT32_MAX / MILLISECONDS_PER_DAY) || - (seconds < INT32_MIN / MILLISECONDS_PER_DAY)) { + if ((seconds > INT32_MAX / SECONDS_PER_DAY) || + (seconds < INT32_MIN / SECONDS_PER_DAY)) { SetError(error, "Date %" PRId32 " exceeds platform time_t bounds", value); return ADBC_STATUS_INVALID_ARGUMENT; } - const time_t time = (time_t)(value * MILLISECONDS_PER_DAY); + const time_t time = (time_t)(value * SECONDS_PER_DAY); #else - const time_t time = value * MILLISECONDS_PER_DAY; + const time_t time = value * SECONDS_PER_DAY; #endif struct tm broken_down_time; From b4f9b6204ea01e571c710360f5e7706e2b0a10fc Mon Sep 17 00:00:00 2001 From: Will Ayd Date: Fri, 28 Jul 2023 16:11:00 -0400 Subject: [PATCH 05/18] fixes --- c/driver/sqlite/statement_reader.c | 4 ++-- c/driver_manager/adbc_driver_manager_test.cc | 1 + 2 files changed, 3 insertions(+), 2 deletions(-) diff --git a/c/driver/sqlite/statement_reader.c b/c/driver/sqlite/statement_reader.c index 5c24e5246d..8b4655644f 100644 --- a/c/driver/sqlite/statement_reader.c +++ b/c/driver/sqlite/statement_reader.c @@ -119,13 +119,13 @@ static AdbcStatusCode ArrowDate32ToIsoString(int32_t value, char** buf, #if defined(_WIN32) if (gmtime_s(&broken_down_time, &time) != 0) { - SetError(error, "Could not convert date %" PRId32 " with to broken down time", value); + SetError(error, "Could not convert date %" PRId32 " to broken down time", value); return ADBC_STATUS_INVALID_ARGUMENT; } #else if (gmtime_r(&time, &broken_down_time) != &broken_down_time) { - SetError(error, "Could not convert date %" PRId32 " with to broken down time", value); + SetError(error, "Could not convert date %" PRId32 " to broken down time", value); return ADBC_STATUS_INVALID_ARGUMENT; } diff --git a/c/driver_manager/adbc_driver_manager_test.cc b/c/driver_manager/adbc_driver_manager_test.cc index 149da7c665..d3ff6f58e1 100644 --- a/c/driver_manager/adbc_driver_manager_test.cc +++ b/c/driver_manager/adbc_driver_manager_test.cc @@ -226,6 +226,7 @@ class SqliteStatementTest : public ::testing::Test, void TestSqlIngestUInt64() { GTEST_SKIP() << "Cannot ingest UINT64 (out of range)"; } void TestSqlIngestBinary() { GTEST_SKIP() << "Cannot ingest BINARY (not implemented)"; } + void TestSqlIngestDate32() { GTEST_SKIP() << "Cannot ingest DATE (not implemented)"; } void TestSqlIngestTimestamp() { GTEST_SKIP() << "Cannot ingest TIMESTAMP (not implemented)"; } From b585b83d56c6f528ebc0d7803e5c5de4307b48d6 Mon Sep 17 00:00:00 2001 From: Will Ayd Date: Fri, 28 Jul 2023 16:42:17 -0400 Subject: [PATCH 06/18] try smaller date range for windows --- 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 e84cc518f5..5ad390aee1 100644 --- a/c/validation/adbc_validation.cc +++ b/c/validation/adbc_validation.cc @@ -1040,8 +1040,8 @@ void StatementTest::TestSqlIngestNumericType(ArrowType type) { } else if (type == ArrowType::NANOARROW_TYPE_DATE32) { // Different databases may choose different epochs, so providing // max values for DATE types is likely to cause overflows - values.push_back(static_cast(-20000)); - values.push_back(static_cast(20000)); + values.push_back(static_cast(-200)); + values.push_back(static_cast(200)); } else { values.push_back(std::numeric_limits::lowest()); values.push_back(std::numeric_limits::max()); From 12530a913361b4ba8d1461043e243d673f09c456 Mon Sep 17 00:00:00 2001 From: Will Ayd Date: Fri, 28 Jul 2023 17:10:55 -0400 Subject: [PATCH 07/18] revert --- 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 5ad390aee1..e84cc518f5 100644 --- a/c/validation/adbc_validation.cc +++ b/c/validation/adbc_validation.cc @@ -1040,8 +1040,8 @@ void StatementTest::TestSqlIngestNumericType(ArrowType type) { } else if (type == ArrowType::NANOARROW_TYPE_DATE32) { // Different databases may choose different epochs, so providing // max values for DATE types is likely to cause overflows - values.push_back(static_cast(-200)); - values.push_back(static_cast(200)); + values.push_back(static_cast(-20000)); + values.push_back(static_cast(20000)); } else { values.push_back(std::numeric_limits::lowest()); values.push_back(std::numeric_limits::max()); From 4f2e068ed3bc73637ed4e6823ab058757f78aff0 Mon Sep 17 00:00:00 2001 From: Will Ayd Date: Mon, 31 Jul 2023 11:36:18 -0400 Subject: [PATCH 08/18] macro typo --- c/driver/sqlite/statement_reader.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/c/driver/sqlite/statement_reader.c b/c/driver/sqlite/statement_reader.c index 8b4655644f..15c31d8a81 100644 --- a/c/driver/sqlite/statement_reader.c +++ b/c/driver/sqlite/statement_reader.c @@ -93,7 +93,7 @@ AdbcStatusCode AdbcSqliteBinderSetArrayStream(struct AdbcSqliteBinder* binder, return AdbcSqliteBinderSet(binder, error); } -#define SECONDS_PER_DAY 86400; +#define SECONDS_PER_DAY 86400 /* Allocates to buf on success. Caller is responsible for freeing. From 09b45e48e41417de49da6901805cecdc7daa1fc6 Mon Sep 17 00:00:00 2001 From: Will Ayd Date: Mon, 31 Jul 2023 11:42:53 -0400 Subject: [PATCH 09/18] snowflake test fix --- c/driver/snowflake/snowflake_test.cc | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/c/driver/snowflake/snowflake_test.cc b/c/driver/snowflake/snowflake_test.cc index c8e08a568f..96edb88982 100644 --- a/c/driver/snowflake/snowflake_test.cc +++ b/c/driver/snowflake/snowflake_test.cc @@ -178,9 +178,9 @@ class SnowflakeStatementTest : public ::testing::Test, } protected: - void ValidateIngestedTemporalData(struct ArrowArrayView* values, - enum ArrowTimeUnit unit, - const char* timezone) override { + void ValidateIngestedTimestampData(struct ArrowArrayView* values, + enum ArrowTimeUnit unit, + const char* timezone) override { std::vector> expected; switch (unit) { case NANOARROW_TIME_UNIT_SECOND: From 6c09ee996d705a4c69ea9f2ed97b40dd7ce8833e Mon Sep 17 00:00:00 2001 From: Will Ayd Date: Mon, 31 Jul 2023 13:51:16 -0400 Subject: [PATCH 10/18] MSVC debug? --- c/driver/sqlite/statement_reader.c | 1 + 1 file changed, 1 insertion(+) diff --git a/c/driver/sqlite/statement_reader.c b/c/driver/sqlite/statement_reader.c index 15c31d8a81..99cc0f2712 100644 --- a/c/driver/sqlite/statement_reader.c +++ b/c/driver/sqlite/statement_reader.c @@ -119,6 +119,7 @@ static AdbcStatusCode ArrowDate32ToIsoString(int32_t value, char** buf, #if defined(_WIN32) if (gmtime_s(&broken_down_time, &time) != 0) { + printf("not able to use time value of %lld\n", time); SetError(error, "Could not convert date %" PRId32 " to broken down time", value); return ADBC_STATUS_INVALID_ARGUMENT; From adc8466781b9f6e301186d184e1051d8cf449a2b Mon Sep 17 00:00:00 2001 From: Will Ayd Date: Mon, 31 Jul 2023 14:07:55 -0400 Subject: [PATCH 11/18] smaller gmtime values --- 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 e84cc518f5..5fa25bbd98 100644 --- a/c/validation/adbc_validation.cc +++ b/c/validation/adbc_validation.cc @@ -1040,8 +1040,8 @@ void StatementTest::TestSqlIngestNumericType(ArrowType type) { } else if (type == ArrowType::NANOARROW_TYPE_DATE32) { // Different databases may choose different epochs, so providing // max values for DATE types is likely to cause overflows - values.push_back(static_cast(-20000)); - values.push_back(static_cast(20000)); + values.push_back(static_cast(-42)); + values.push_back(static_cast(42)); } else { values.push_back(std::numeric_limits::lowest()); values.push_back(std::numeric_limits::max()); From 0d7546985d8a0632989fee1653ca0c0a0bc86bd0 Mon Sep 17 00:00:00 2001 From: Will Ayd Date: Mon, 31 Jul 2023 14:20:35 -0400 Subject: [PATCH 12/18] more debug --- c/driver/sqlite/statement_reader.c | 6 +++--- c/validation/adbc_validation.cc | 4 ++-- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/c/driver/sqlite/statement_reader.c b/c/driver/sqlite/statement_reader.c index 99cc0f2712..ee93c56f2e 100644 --- a/c/driver/sqlite/statement_reader.c +++ b/c/driver/sqlite/statement_reader.c @@ -110,16 +110,16 @@ static AdbcStatusCode ArrowDate32ToIsoString(int32_t value, char** buf, return ADBC_STATUS_INVALID_ARGUMENT; } - const time_t time = (time_t)(value * SECONDS_PER_DAY); + time_t time = (time_t)(value * SECONDS_PER_DAY); #else - const time_t time = value * SECONDS_PER_DAY; + time_t time = value * SECONDS_PER_DAY; #endif struct tm broken_down_time; + time = 0; // debug #if defined(_WIN32) if (gmtime_s(&broken_down_time, &time) != 0) { - printf("not able to use time value of %lld\n", time); SetError(error, "Could not convert date %" PRId32 " to broken down time", value); return ADBC_STATUS_INVALID_ARGUMENT; diff --git a/c/validation/adbc_validation.cc b/c/validation/adbc_validation.cc index 5fa25bbd98..e84cc518f5 100644 --- a/c/validation/adbc_validation.cc +++ b/c/validation/adbc_validation.cc @@ -1040,8 +1040,8 @@ void StatementTest::TestSqlIngestNumericType(ArrowType type) { } else if (type == ArrowType::NANOARROW_TYPE_DATE32) { // Different databases may choose different epochs, so providing // max values for DATE types is likely to cause overflows - values.push_back(static_cast(-42)); - values.push_back(static_cast(42)); + values.push_back(static_cast(-20000)); + values.push_back(static_cast(20000)); } else { values.push_back(std::numeric_limits::lowest()); values.push_back(std::numeric_limits::max()); From 15deeea5a06c75ba9866642010db0b02c53960ba Mon Sep 17 00:00:00 2001 From: Will Ayd Date: Mon, 31 Jul 2023 14:32:18 -0400 Subject: [PATCH 13/18] more windows debug --- c/driver/sqlite/statement_reader.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/c/driver/sqlite/statement_reader.c b/c/driver/sqlite/statement_reader.c index ee93c56f2e..38f1182919 100644 --- a/c/driver/sqlite/statement_reader.c +++ b/c/driver/sqlite/statement_reader.c @@ -116,7 +116,7 @@ static AdbcStatusCode ArrowDate32ToIsoString(int32_t value, char** buf, #endif struct tm broken_down_time; - time = 0; // debug + time = -1; // debug #if defined(_WIN32) if (gmtime_s(&broken_down_time, &time) != 0) { From 253452fddf109af1e6650aee12f122168a4db717 Mon Sep 17 00:00:00 2001 From: Will Ayd Date: Mon, 31 Jul 2023 14:45:57 -0400 Subject: [PATCH 14/18] limit debug --- c/driver/sqlite/statement_reader.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/c/driver/sqlite/statement_reader.c b/c/driver/sqlite/statement_reader.c index 38f1182919..b50ff58c9c 100644 --- a/c/driver/sqlite/statement_reader.c +++ b/c/driver/sqlite/statement_reader.c @@ -116,7 +116,7 @@ static AdbcStatusCode ArrowDate32ToIsoString(int32_t value, char** buf, #endif struct tm broken_down_time; - time = -1; // debug + time = -20000; // debug #if defined(_WIN32) if (gmtime_s(&broken_down_time, &time) != 0) { From d303a8a1aa1f217b03d8bfdf4dcfc161df68c9f0 Mon Sep 17 00:00:00 2001 From: Will Ayd Date: Mon, 31 Jul 2023 14:46:22 -0400 Subject: [PATCH 15/18] more --- c/driver/sqlite/statement_reader.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/c/driver/sqlite/statement_reader.c b/c/driver/sqlite/statement_reader.c index b50ff58c9c..dc0e7a4f3b 100644 --- a/c/driver/sqlite/statement_reader.c +++ b/c/driver/sqlite/statement_reader.c @@ -116,7 +116,7 @@ static AdbcStatusCode ArrowDate32ToIsoString(int32_t value, char** buf, #endif struct tm broken_down_time; - time = -20000; // debug + time = -20000 * SECONDS_PER_DAY; // debug #if defined(_WIN32) if (gmtime_s(&broken_down_time, &time) != 0) { From 684e48ecf331705b5c674a6fe55f37aa1babc4f9 Mon Sep 17 00:00:00 2001 From: Will Ayd Date: Mon, 31 Jul 2023 14:58:00 -0400 Subject: [PATCH 16/18] try before epoch --- c/driver/sqlite/statement_reader.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/c/driver/sqlite/statement_reader.c b/c/driver/sqlite/statement_reader.c index dc0e7a4f3b..6013361ffe 100644 --- a/c/driver/sqlite/statement_reader.c +++ b/c/driver/sqlite/statement_reader.c @@ -116,7 +116,7 @@ static AdbcStatusCode ArrowDate32ToIsoString(int32_t value, char** buf, #endif struct tm broken_down_time; - time = -20000 * SECONDS_PER_DAY; // debug + time = -2 * SECONDS_PER_DAY; // debug #if defined(_WIN32) if (gmtime_s(&broken_down_time, &time) != 0) { From 6ff44f6744f321a46294863244e8520dd20b7ae5 Mon Sep 17 00:00:00 2001 From: Will Ayd Date: Mon, 31 Jul 2023 15:09:13 -0400 Subject: [PATCH 17/18] another guess --- c/driver/sqlite/statement_reader.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/c/driver/sqlite/statement_reader.c b/c/driver/sqlite/statement_reader.c index 6013361ffe..579d731341 100644 --- a/c/driver/sqlite/statement_reader.c +++ b/c/driver/sqlite/statement_reader.c @@ -116,7 +116,7 @@ static AdbcStatusCode ArrowDate32ToIsoString(int32_t value, char** buf, #endif struct tm broken_down_time; - time = -2 * SECONDS_PER_DAY; // debug + time = -1 * SECONDS_PER_DAY; // debug #if defined(_WIN32) if (gmtime_s(&broken_down_time, &time) != 0) { From d4bf1bb3f980691531e28683ee1398a26f497b8f Mon Sep 17 00:00:00 2001 From: Will Ayd Date: Mon, 31 Jul 2023 15:25:32 -0400 Subject: [PATCH 18/18] smaller test range --- c/driver/sqlite/statement_reader.c | 1 - c/validation/adbc_validation.cc | 7 +++---- 2 files changed, 3 insertions(+), 5 deletions(-) diff --git a/c/driver/sqlite/statement_reader.c b/c/driver/sqlite/statement_reader.c index 579d731341..f8483c537f 100644 --- a/c/driver/sqlite/statement_reader.c +++ b/c/driver/sqlite/statement_reader.c @@ -116,7 +116,6 @@ static AdbcStatusCode ArrowDate32ToIsoString(int32_t value, char** buf, #endif struct tm broken_down_time; - time = -1 * SECONDS_PER_DAY; // debug #if defined(_WIN32) if (gmtime_s(&broken_down_time, &time) != 0) { diff --git a/c/validation/adbc_validation.cc b/c/validation/adbc_validation.cc index e84cc518f5..8f519b0417 100644 --- a/c/validation/adbc_validation.cc +++ b/c/validation/adbc_validation.cc @@ -1038,10 +1038,9 @@ void StatementTest::TestSqlIngestNumericType(ArrowType type) { values.push_back(static_cast(-1.5)); values.push_back(static_cast(1.5)); } else if (type == ArrowType::NANOARROW_TYPE_DATE32) { - // Different databases may choose different epochs, so providing - // max values for DATE types is likely to cause overflows - values.push_back(static_cast(-20000)); - values.push_back(static_cast(20000)); + // Windows does not seem to support negative date values + values.push_back(static_cast(0)); + values.push_back(static_cast(42)); } else { values.push_back(std::numeric_limits::lowest()); values.push_back(std::numeric_limits::max());