From 8aa5d491c958113a2b747c20222d5bf78d961a58 Mon Sep 17 00:00:00 2001 From: Will Ayd Date: Wed, 28 Jun 2023 10:06:05 -0700 Subject: [PATCH 01/10] initial timestamp hacks --- c/driver/postgresql/statement.cc | 32 ++++++++++++++++++++++++++++ c/validation/adbc_validation.cc | 4 ++++ c/validation/adbc_validation.h | 4 ++++ c/validation/adbc_validation_util.cc | 11 +++++++++- 4 files changed, 50 insertions(+), 1 deletion(-) diff --git a/c/driver/postgresql/statement.cc b/c/driver/postgresql/statement.cc index 30926b4fea..5d19d4c5da 100644 --- a/c/driver/postgresql/statement.cc +++ b/c/driver/postgresql/statement.cc @@ -216,6 +216,10 @@ struct BindStream { type_id = PostgresTypeId::kBytea; param_lengths[i] = 0; break; + case ArrowType::NANOARROW_TYPE_TIMESTAMP: + type_id = PostgresTypeId::kTimestamp; + param_lengths[i] = 8; + break; default: SetError(error, "%s%" PRIu64 "%s%s%s%s", "[libpq] Field #", static_cast(i + 1), " ('", bind_schema->children[i]->name, @@ -337,6 +341,31 @@ struct BindStream { param_values[col] = const_cast(view.data.as_char); break; } + case ArrowType::NANOARROW_TYPE_TIMESTAMP: { + int64_t val = array_view->children[col]->buffer_views[1].data.as_int64[row]; + auto unit = bind_schema_fields[col].time_unit; + + // TODO: maybe upstream to nanoarrow as ArrowTimeUnitGetMultiplier + switch (unit) { + case NANOARROW_TIME_UNIT_SECOND: + val *= 1000000; + break; + case NANOARROW_TIME_UNIT_MILLI: + val *= 1000; + break; + case NANOARROW_TIME_UNIT_MICRO: + break; + case NANOARROW_TIME_UNIT_NANO: + val /= 1000; + break; + } + + // 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)); + break; + } default: SetError(error, "%s%" PRId64 "%s%s%s%s", "[libpq] Field #", col + 1, " ('", bind_schema->children[col]->name, @@ -605,6 +634,9 @@ AdbcStatusCode PostgresStatement::CreateBulkTable( case ArrowType::NANOARROW_TYPE_BINARY: create += " BYTEA"; break; + case ArrowType::NANOARROW_TYPE_TIMESTAMP: + create += " TIMESTAMP"; + 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 d73b556470..d0ac195b95 100644 --- a/c/validation/adbc_validation.cc +++ b/c/validation/adbc_validation.cc @@ -1094,6 +1094,10 @@ void StatementTest::TestSqlIngestBinary() { NANOARROW_TYPE_BINARY, {std::nullopt, "", "\x00\x01\x02\x04", "\xFE\xFF"})); } +void StatementTest::TestSqlIngestTimestamp() { + ASSERT_NO_FATAL_FAILURE(TestSqlIngestType(NANOARROW_TYPE_TIMESTAMP, {0, 42})); +} + void StatementTest::TestSqlIngestAppend() { if (!quirks()->supports_bulk_ingest()) { GTEST_SKIP(); diff --git a/c/validation/adbc_validation.h b/c/validation/adbc_validation.h index 4e4251bf94..4ce5d714bf 100644 --- a/c/validation/adbc_validation.h +++ b/c/validation/adbc_validation.h @@ -229,6 +229,9 @@ class StatementTest { void TestSqlIngestString(); void TestSqlIngestBinary(); + // Temporal + void TestSqlIngestTimestamp(); + // ---- End Type-specific tests ---------------- void TestSqlIngestAppend(); @@ -288,6 +291,7 @@ class StatementTest { TEST_F(FIXTURE, SqlIngestFloat64) { TestSqlIngestFloat64(); } \ TEST_F(FIXTURE, SqlIngestString) { TestSqlIngestString(); } \ TEST_F(FIXTURE, SqlIngestBinary) { TestSqlIngestBinary(); } \ + TEST_F(FIXTURE, SqlIngestTimestamp) { TestSqlIngestTimestamp(); } \ TEST_F(FIXTURE, SqlIngestAppend) { TestSqlIngestAppend(); } \ TEST_F(FIXTURE, SqlIngestErrors) { TestSqlIngestErrors(); } \ TEST_F(FIXTURE, SqlIngestMultipleConnections) { TestSqlIngestMultipleConnections(); } \ diff --git a/c/validation/adbc_validation_util.cc b/c/validation/adbc_validation_util.cc index 7978947f2c..073178b466 100644 --- a/c/validation/adbc_validation_util.cc +++ b/c/validation/adbc_validation_util.cc @@ -141,7 +141,16 @@ int MakeSchema(struct ArrowSchema* schema, const std::vector& field CHECK_ERRNO(ArrowSchemaSetTypeStruct(schema, fields.size())); size_t i = 0; for (const SchemaField& field : fields) { - CHECK_ERRNO(ArrowSchemaSetType(schema->children[i], field.type)); + switch (field.type) { + case NANOARROW_TYPE_TIMESTAMP: + // TODO: don't hardcode unit here + CHECK_ERRNO(ArrowSchemaSetTypeDateTime(schema->children[i], field.type, + NANOARROW_TIME_UNIT_MICRO, + /*timezone=*/nullptr)); + break; + default: + CHECK_ERRNO(ArrowSchemaSetType(schema->children[i], field.type)); + } CHECK_ERRNO(ArrowSchemaSetName(schema->children[i], field.name.c_str())); if (!field.nullable) { schema->children[i]->flags &= ~ARROW_FLAG_NULLABLE; From e6309860034fd8e4a333e173ebbd2a41394a588d Mon Sep 17 00:00:00 2001 From: Will Ayd Date: Wed, 28 Jun 2023 10:12:19 -0700 Subject: [PATCH 02/10] sqlite skip --- c/driver/sqlite/sqlite_test.cc | 3 +++ 1 file changed, 3 insertions(+) diff --git a/c/driver/sqlite/sqlite_test.cc b/c/driver/sqlite/sqlite_test.cc index 8a580cdbc8..d245c1da7d 100644 --- a/c/driver/sqlite/sqlite_test.cc +++ b/c/driver/sqlite/sqlite_test.cc @@ -171,6 +171,9 @@ 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 TestSqlIngestTimestamp() { + GTEST_SKIP() << "Cannot ingest TIMESTAMP (not implemented)"; + } protected: SqliteQuirks quirks_; From 289606cd41709abea7a02037c7ad276bebc40be8 Mon Sep 17 00:00:00 2001 From: Will Ayd Date: Wed, 28 Jun 2023 13:22:57 -0700 Subject: [PATCH 03/10] feedback pt 1 --- c/driver/postgresql/statement.cc | 36 ++++++++++++++++++++++++++++---- 1 file changed, 32 insertions(+), 4 deletions(-) diff --git a/c/driver/postgresql/statement.cc b/c/driver/postgresql/statement.cc index 5d19d4c5da..0d9002c079 100644 --- a/c/driver/postgresql/statement.cc +++ b/c/driver/postgresql/statement.cc @@ -217,6 +217,12 @@ struct BindStream { param_lengths[i] = 0; break; case ArrowType::NANOARROW_TYPE_TIMESTAMP: + if (!(bind_schema_fields[i].timezone == nullptr)) { + SetError(error, "%s%" PRIi64 "%s%s%s", "[libpq] Field #", i + 1, " (\"", + bind_schema->children[i]->name, + "\") has unsupported type code timestamp with timezone"); + return ADBC_STATUS_NOT_IMPLEMENTED; + } type_id = PostgresTypeId::kTimestamp; param_lengths[i] = 8; break; @@ -343,14 +349,38 @@ struct BindStream { } case ArrowType::NANOARROW_TYPE_TIMESTAMP: { int64_t val = array_view->children[col]->buffer_views[1].data.as_int64[row]; - auto unit = bind_schema_fields[col].time_unit; + if (!(bind_schema_fields[col].timezone == nullptr)) { + SetError(error, "%s%" PRIi64 "%s%s%s", "[libpq] Column #", col + 1, + " (\"", PQfname(result, col), + "\") has unsupported type code timestamp with timezone"); + return ADBC_STATUS_NOT_IMPLEMENTED; + } - // TODO: maybe upstream to nanoarrow as ArrowTimeUnitGetMultiplier + // 2000-01-01 00:00:00.000000 in microseconds + constexpr int64_t kPostgresTimestampEpoch = 946684800000000; + constexpr int64_t kSecOverflowLimit = 9223372036854; + constexpr int64_t kmSecOverflowLimit = 9223372036854775; + + auto unit = bind_schema_fields[col].time_unit; switch (unit) { case NANOARROW_TIME_UNIT_SECOND: + if (abs(val) > kSecOverflowLimit) { + SetError(error, "%s%" PRId64 "%s%s%s%" PRId64 "%s", "[libpq] Field #", + col + 1, "('", bind_schema->children[col]->name, "') Row #", + row + 1, + "has value which exceeds postgres timestamp limits"); + return ADBC_STATUS_INVALID_ARGUMENT; + } val *= 1000000; break; case NANOARROW_TIME_UNIT_MILLI: + if (abs(val) > kmSecOverflowLimit) { + SetError(error, "%s%" PRId64 "%s%s%s%" PRId64 "%s", "[libpq] Field #", + col + 1, "('", bind_schema->children[col]->name, "') Row #", + row + 1, + "has value which exceeds postgres timestamp limits"); + return ADBC_STATUS_INVALID_ARGUMENT; + } val *= 1000; break; case NANOARROW_TIME_UNIT_MICRO: @@ -360,8 +390,6 @@ struct BindStream { break; } - // 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)); break; From 78b817dabf92fa59f87033bb9345a943c364f54c Mon Sep 17 00:00:00 2001 From: Will Ayd Date: Wed, 28 Jun 2023 14:34:38 -0700 Subject: [PATCH 04/10] implemented custom test for timestamp --- c/driver/postgresql/statement.cc | 4 +- c/validation/adbc_validation.cc | 63 +++++++++++++++++++++++++++++++- 2 files changed, 64 insertions(+), 3 deletions(-) diff --git a/c/driver/postgresql/statement.cc b/c/driver/postgresql/statement.cc index 0d9002c079..089dd9a393 100644 --- a/c/driver/postgresql/statement.cc +++ b/c/driver/postgresql/statement.cc @@ -217,7 +217,7 @@ struct BindStream { param_lengths[i] = 0; break; case ArrowType::NANOARROW_TYPE_TIMESTAMP: - if (!(bind_schema_fields[i].timezone == nullptr)) { + if (strcmp("", bind_schema_fields[i].timezone)) { SetError(error, "%s%" PRIi64 "%s%s%s", "[libpq] Field #", i + 1, " (\"", bind_schema->children[i]->name, "\") has unsupported type code timestamp with timezone"); @@ -349,7 +349,7 @@ struct BindStream { } case ArrowType::NANOARROW_TYPE_TIMESTAMP: { int64_t val = array_view->children[col]->buffer_views[1].data.as_int64[row]; - if (!(bind_schema_fields[col].timezone == nullptr)) { + if (strcmp("", bind_schema_fields[col].timezone)) { SetError(error, "%s%" PRIi64 "%s%s%s", "[libpq] Column #", col + 1, " (\"", PQfname(result, col), "\") has unsupported type code timestamp with timezone"); diff --git a/c/validation/adbc_validation.cc b/c/validation/adbc_validation.cc index d0ac195b95..34f312655a 100644 --- a/c/validation/adbc_validation.cc +++ b/c/validation/adbc_validation.cc @@ -1095,7 +1095,68 @@ void StatementTest::TestSqlIngestBinary() { } void StatementTest::TestSqlIngestTimestamp() { - ASSERT_NO_FATAL_FAILURE(TestSqlIngestType(NANOARROW_TYPE_TIMESTAMP, {0, 42})); + if (!quirks()->supports_bulk_ingest()) { + GTEST_SKIP(); + } + + ASSERT_THAT(quirks()->DropTable(&connection, "bulk_ingest", &error), + IsOkStatus(&error)); + + Handle schema; + Handle array; + struct ArrowError na_error; + const std::vector> values = {0, 42}; + + // TODO: 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], NANOARROW_TYPE_TIMESTAMP, + NANOARROW_TIME_UNIT_MICRO, /*timezone=*/nullptr); + ArrowSchemaSetName(schema->children[0], "col"); + ASSERT_THAT(MakeBatch(&schema.value, &array.value, &na_error, values), + IsOkErrno()); + + ASSERT_THAT(AdbcStatementNew(&connection, &statement, &error), IsOkStatus(&error)); + ASSERT_THAT(AdbcStatementSetOption(&statement, ADBC_INGEST_OPTION_TARGET_TABLE, + "bulk_ingest", &error), + IsOkStatus(&error)); + ASSERT_THAT(AdbcStatementBind(&statement, &array.value, &schema.value, &error), + IsOkStatus(&error)); + + int64_t rows_affected = 0; + ASSERT_THAT(AdbcStatementExecuteQuery(&statement, nullptr, &rows_affected, &error), + 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), + IsOkStatus(&error)); + { + StreamReader reader; + ASSERT_THAT(AdbcStatementExecuteQuery(&statement, &reader.stream.value, + &reader.rows_affected, &error), + IsOkStatus(&error)); + ASSERT_THAT(reader.rows_affected, + ::testing::AnyOf(::testing::Eq(values.size()), ::testing::Eq(-1))); + + ASSERT_NO_FATAL_FAILURE(reader.GetSchema()); + ASSERT_NO_FATAL_FAILURE(CompareSchema(&reader.schema.value, + {{"col", NANOARROW_TYPE_TIMESTAMP, NULLABLE}})); + + 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); + + ASSERT_NO_FATAL_FAILURE( + CompareArray(reader.array_view->children[0], values)); + + ASSERT_NO_FATAL_FAILURE(reader.Next()); + ASSERT_EQ(nullptr, reader.array->release); + } } void StatementTest::TestSqlIngestAppend() { From 776119db187c25e8053bc8efa0f22355c223ee6e Mon Sep 17 00:00:00 2001 From: Will Ayd Date: Wed, 28 Jun 2023 14:45:51 -0700 Subject: [PATCH 05/10] variable unit support --- c/validation/adbc_validation.cc | 25 ++++++++++++++++++------- c/validation/adbc_validation.h | 1 + 2 files changed, 19 insertions(+), 7 deletions(-) diff --git a/c/validation/adbc_validation.cc b/c/validation/adbc_validation.cc index 34f312655a..43d8511923 100644 --- a/c/validation/adbc_validation.cc +++ b/c/validation/adbc_validation.cc @@ -1094,7 +1094,7 @@ void StatementTest::TestSqlIngestBinary() { NANOARROW_TYPE_BINARY, {std::nullopt, "", "\x00\x01\x02\x04", "\xFE\xFF"})); } -void StatementTest::TestSqlIngestTimestamp() { +void StatementTest::TestSqlIngestTimestampWithUnit(enum ArrowTimeUnit unit) { if (!quirks()->supports_bulk_ingest()) { GTEST_SKIP(); } @@ -1105,14 +1105,14 @@ void StatementTest::TestSqlIngestTimestamp() { Handle schema; Handle array; struct ArrowError na_error; - const std::vector> values = {0, 42}; + const std::vector> values = {std::nullopt, 0, 42}; - // TODO: much of this code is shared with TestSqlIngestType with minor + // 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], NANOARROW_TYPE_TIMESTAMP, - NANOARROW_TIME_UNIT_MICRO, /*timezone=*/nullptr); + ArrowSchemaSetTypeDateTime(schema->children[0], NANOARROW_TYPE_TIMESTAMP, unit, + /*timezone=*/nullptr); ArrowSchemaSetName(schema->children[0], "col"); ASSERT_THAT(MakeBatch(&schema.value, &array.value, &na_error, values), IsOkErrno()); @@ -1151,14 +1151,25 @@ void StatementTest::TestSqlIngestTimestamp() { ASSERT_EQ(values.size(), reader.array->length); ASSERT_EQ(1, reader.array->n_children); - ASSERT_NO_FATAL_FAILURE( - CompareArray(reader.array_view->children[0], values)); + if (unit == NANOARROW_TIME_UNIT_MICRO) { + // Similar to the TestSqlIngestType implementation we are only now + // testing values if the unit round trips + ASSERT_NO_FATAL_FAILURE( + CompareArray(reader.array_view->children[0], values)); + } ASSERT_NO_FATAL_FAILURE(reader.Next()); ASSERT_EQ(nullptr, reader.array->release); } } +void StatementTest::TestSqlIngestTimestamp() { + ASSERT_NO_FATAL_FAILURE(TestSqlIngestTimestampWithUnit(NANOARROW_TIME_UNIT_SECOND)); + ASSERT_NO_FATAL_FAILURE(TestSqlIngestTimestampWithUnit(NANOARROW_TIME_UNIT_MICRO)); + ASSERT_NO_FATAL_FAILURE(TestSqlIngestTimestampWithUnit(NANOARROW_TIME_UNIT_MILLI)); + ASSERT_NO_FATAL_FAILURE(TestSqlIngestTimestampWithUnit(NANOARROW_TIME_UNIT_NANO)); +} + void StatementTest::TestSqlIngestAppend() { if (!quirks()->supports_bulk_ingest()) { GTEST_SKIP(); diff --git a/c/validation/adbc_validation.h b/c/validation/adbc_validation.h index 4ce5d714bf..9f3a84c1bd 100644 --- a/c/validation/adbc_validation.h +++ b/c/validation/adbc_validation.h @@ -231,6 +231,7 @@ class StatementTest { // Temporal void TestSqlIngestTimestamp(); + void TestSqlIngestTimestampWithUnit(enum ArrowTimeUnit unit); // ---- End Type-specific tests ---------------- From 80b21e5943a899f535ea5912e495972fee6c3c00 Mon Sep 17 00:00:00 2001 From: Will Ayd Date: Wed, 28 Jun 2023 14:52:16 -0700 Subject: [PATCH 06/10] templated time unit test --- c/validation/adbc_validation.cc | 15 ++++++++------- c/validation/adbc_validation.h | 4 +++- 2 files changed, 11 insertions(+), 8 deletions(-) diff --git a/c/validation/adbc_validation.cc b/c/validation/adbc_validation.cc index 43d8511923..61f23fffa6 100644 --- a/c/validation/adbc_validation.cc +++ b/c/validation/adbc_validation.cc @@ -1094,7 +1094,8 @@ void StatementTest::TestSqlIngestBinary() { NANOARROW_TYPE_BINARY, {std::nullopt, "", "\x00\x01\x02\x04", "\xFE\xFF"})); } -void StatementTest::TestSqlIngestTimestampWithUnit(enum ArrowTimeUnit unit) { +template +void StatementTest::TestSqlIngestTemporalType() { if (!quirks()->supports_bulk_ingest()) { GTEST_SKIP(); } @@ -1111,7 +1112,7 @@ void StatementTest::TestSqlIngestTimestampWithUnit(enum ArrowTimeUnit unit) { // changes to allow for various time units to be tested ArrowSchemaInit(&schema.value); ArrowSchemaSetTypeStruct(&schema.value, 1); - ArrowSchemaSetTypeDateTime(schema->children[0], NANOARROW_TYPE_TIMESTAMP, unit, + ArrowSchemaSetTypeDateTime(schema->children[0], NANOARROW_TYPE_TIMESTAMP, TU, /*timezone=*/nullptr); ArrowSchemaSetName(schema->children[0], "col"); ASSERT_THAT(MakeBatch(&schema.value, &array.value, &na_error, values), @@ -1151,7 +1152,7 @@ void StatementTest::TestSqlIngestTimestampWithUnit(enum ArrowTimeUnit unit) { ASSERT_EQ(values.size(), reader.array->length); ASSERT_EQ(1, reader.array->n_children); - if (unit == NANOARROW_TIME_UNIT_MICRO) { + if (TU == NANOARROW_TIME_UNIT_MICRO) { // Similar to the TestSqlIngestType implementation we are only now // testing values if the unit round trips ASSERT_NO_FATAL_FAILURE( @@ -1164,10 +1165,10 @@ void StatementTest::TestSqlIngestTimestampWithUnit(enum ArrowTimeUnit unit) { } void StatementTest::TestSqlIngestTimestamp() { - ASSERT_NO_FATAL_FAILURE(TestSqlIngestTimestampWithUnit(NANOARROW_TIME_UNIT_SECOND)); - ASSERT_NO_FATAL_FAILURE(TestSqlIngestTimestampWithUnit(NANOARROW_TIME_UNIT_MICRO)); - ASSERT_NO_FATAL_FAILURE(TestSqlIngestTimestampWithUnit(NANOARROW_TIME_UNIT_MILLI)); - ASSERT_NO_FATAL_FAILURE(TestSqlIngestTimestampWithUnit(NANOARROW_TIME_UNIT_NANO)); + ASSERT_NO_FATAL_FAILURE(TestSqlIngestTemporalType()); + ASSERT_NO_FATAL_FAILURE(TestSqlIngestTemporalType()); + ASSERT_NO_FATAL_FAILURE(TestSqlIngestTemporalType()); + ASSERT_NO_FATAL_FAILURE(TestSqlIngestTemporalType()); } void StatementTest::TestSqlIngestAppend() { diff --git a/c/validation/adbc_validation.h b/c/validation/adbc_validation.h index 9f3a84c1bd..1650d064e0 100644 --- a/c/validation/adbc_validation.h +++ b/c/validation/adbc_validation.h @@ -231,7 +231,6 @@ class StatementTest { // Temporal void TestSqlIngestTimestamp(); - void TestSqlIngestTimestampWithUnit(enum ArrowTimeUnit unit); // ---- End Type-specific tests ---------------- @@ -273,6 +272,9 @@ class StatementTest { template void TestSqlIngestNumericType(ArrowType type); + + template + void TestSqlIngestTemporalType(); }; #define ADBCV_TEST_STATEMENT(FIXTURE) \ From bcd344b7e408766dfef2fa229bc60e809bea382b Mon Sep 17 00:00:00 2001 From: Will Ayd Date: Wed, 28 Jun 2023 15:03:22 -0700 Subject: [PATCH 07/10] Fixed error printing --- c/driver/postgresql/statement.cc | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/c/driver/postgresql/statement.cc b/c/driver/postgresql/statement.cc index 089dd9a393..f58bef893c 100644 --- a/c/driver/postgresql/statement.cc +++ b/c/driver/postgresql/statement.cc @@ -218,8 +218,8 @@ struct BindStream { break; case ArrowType::NANOARROW_TYPE_TIMESTAMP: if (strcmp("", bind_schema_fields[i].timezone)) { - SetError(error, "%s%" PRIi64 "%s%s%s", "[libpq] Field #", i + 1, " (\"", - bind_schema->children[i]->name, + SetError(error, "%s%" PRIi64 "%s%s%s", "[libpq] Field #", + static_cast(i + 1), " (\"", bind_schema->children[i]->name, "\") has unsupported type code timestamp with timezone"); return ADBC_STATUS_NOT_IMPLEMENTED; } From bfbf2429ea709834a6ec085a686c409011a2fa2d Mon Sep 17 00:00:00 2001 From: Will Ayd Date: Wed, 28 Jun 2023 16:23:17 -0700 Subject: [PATCH 08/10] driver manager test fix --- c/driver_manager/adbc_driver_manager_test.cc | 3 +++ 1 file changed, 3 insertions(+) diff --git a/c/driver_manager/adbc_driver_manager_test.cc b/c/driver_manager/adbc_driver_manager_test.cc index 99fa477bfa..d33114ee3f 100644 --- a/c/driver_manager/adbc_driver_manager_test.cc +++ b/c/driver_manager/adbc_driver_manager_test.cc @@ -226,6 +226,9 @@ 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 TestSqlIngestTimestamp() { + GTEST_SKIP() << "Cannot ingest TIMESTAMP (not implemented)"; + } protected: SqliteQuirks quirks_; From c8f7a6b4070848bf24c581dbc57d343cabc66533 Mon Sep 17 00:00:00 2001 From: Will Ayd Date: Thu, 29 Jun 2023 18:37:28 -0700 Subject: [PATCH 09/10] simplify SetError --- c/driver/postgresql/statement.cc | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/c/driver/postgresql/statement.cc b/c/driver/postgresql/statement.cc index f58bef893c..db9af1619b 100644 --- a/c/driver/postgresql/statement.cc +++ b/c/driver/postgresql/statement.cc @@ -218,7 +218,7 @@ struct BindStream { break; case ArrowType::NANOARROW_TYPE_TIMESTAMP: if (strcmp("", bind_schema_fields[i].timezone)) { - SetError(error, "%s%" PRIi64 "%s%s%s", "[libpq] Field #", + SetError(error, "[libpq] Field #%" PRIi64 "%s%s%s", static_cast(i + 1), " (\"", bind_schema->children[i]->name, "\") has unsupported type code timestamp with timezone"); return ADBC_STATUS_NOT_IMPLEMENTED; @@ -350,8 +350,8 @@ struct BindStream { case ArrowType::NANOARROW_TYPE_TIMESTAMP: { int64_t val = array_view->children[col]->buffer_views[1].data.as_int64[row]; if (strcmp("", bind_schema_fields[col].timezone)) { - SetError(error, "%s%" PRIi64 "%s%s%s", "[libpq] Column #", col + 1, - " (\"", PQfname(result, col), + SetError(error, "[libpq] Column #%" PRIi64 "%s%s%s", col + 1, " (\"", + PQfname(result, col), "\") has unsupported type code timestamp with timezone"); return ADBC_STATUS_NOT_IMPLEMENTED; } @@ -365,7 +365,7 @@ struct BindStream { switch (unit) { case NANOARROW_TIME_UNIT_SECOND: if (abs(val) > kSecOverflowLimit) { - SetError(error, "%s%" PRId64 "%s%s%s%" PRId64 "%s", "[libpq] Field #", + 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"); @@ -375,7 +375,7 @@ struct BindStream { break; case NANOARROW_TIME_UNIT_MILLI: if (abs(val) > kmSecOverflowLimit) { - SetError(error, "%s%" PRId64 "%s%s%s%" PRId64 "%s", "[libpq] Field #", + 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"); From 60ef31d438624fdb287925d82d8e4e5b40f0f978 Mon Sep 17 00:00:00 2001 From: Will Ayd Date: Fri, 30 Jun 2023 08:56:32 -0700 Subject: [PATCH 10/10] feedback --- c/driver/postgresql/statement.cc | 6 ++++++ c/validation/adbc_validation_util.cc | 11 +---------- 2 files changed, 7 insertions(+), 10 deletions(-) diff --git a/c/driver/postgresql/statement.cc b/c/driver/postgresql/statement.cc index db9af1619b..8bd80a3c17 100644 --- a/c/driver/postgresql/statement.cc +++ b/c/driver/postgresql/statement.cc @@ -663,6 +663,12 @@ AdbcStatusCode PostgresStatement::CreateBulkTable( create += " BYTEA"; break; case ArrowType::NANOARROW_TYPE_TIMESTAMP: + if (strcmp("", source_schema_fields[i].timezone)) { + SetError(error, "[libpq] Field #%" PRIi64 "%s%s%s", static_cast(i + 1), + " (\"", source_schema.children[i]->name, + "\") has unsupported type for ingestion timestamp with timezone"); + return ADBC_STATUS_NOT_IMPLEMENTED; + } create += " TIMESTAMP"; break; default: diff --git a/c/validation/adbc_validation_util.cc b/c/validation/adbc_validation_util.cc index 073178b466..7978947f2c 100644 --- a/c/validation/adbc_validation_util.cc +++ b/c/validation/adbc_validation_util.cc @@ -141,16 +141,7 @@ int MakeSchema(struct ArrowSchema* schema, const std::vector& field CHECK_ERRNO(ArrowSchemaSetTypeStruct(schema, fields.size())); size_t i = 0; for (const SchemaField& field : fields) { - switch (field.type) { - case NANOARROW_TYPE_TIMESTAMP: - // TODO: don't hardcode unit here - CHECK_ERRNO(ArrowSchemaSetTypeDateTime(schema->children[i], field.type, - NANOARROW_TIME_UNIT_MICRO, - /*timezone=*/nullptr)); - break; - default: - CHECK_ERRNO(ArrowSchemaSetType(schema->children[i], field.type)); - } + CHECK_ERRNO(ArrowSchemaSetType(schema->children[i], field.type)); CHECK_ERRNO(ArrowSchemaSetName(schema->children[i], field.name.c_str())); if (!field.nullable) { schema->children[i]->flags &= ~ARROW_FLAG_NULLABLE;