From 84095f53f1198933893c37ae8bd9892eab2687e9 Mon Sep 17 00:00:00 2001 From: Sutou Kouhei Date: Sun, 14 Jan 2024 08:54:06 +0900 Subject: [PATCH 1/3] fix(ci): remove invalid --version=14 clang-format argument Fixes #1461. --- .pre-commit-config.yaml | 2 +- .../copy/postgres_copy_reader_test.cc | 15 +- .../copy/postgres_copy_test_common.h | 42 ++-- .../copy/postgres_copy_writer_test.cc | 164 +++++++-------- c/driver/postgresql/copy/reader.h | 2 +- c/driver/postgresql/copy/writer.h | 43 ++-- c/driver/postgresql/postgres_util.h | 2 +- c/driver/postgresql/postgresql_benchmark.cc | 195 ++++++++---------- c/driver/postgresql/postgresql_test.cc | 155 +++++++------- c/driver/postgresql/statement.cc | 2 +- c/validation/adbc_validation.cc | 28 ++- c/validation/adbc_validation.h | 9 +- 12 files changed, 299 insertions(+), 360 deletions(-) diff --git a/.pre-commit-config.yaml b/.pre-commit-config.yaml index 194924ae0f..c13ad36209 100644 --- a/.pre-commit-config.yaml +++ b/.pre-commit-config.yaml @@ -43,7 +43,7 @@ repos: rev: v1.3.5 hooks: - id: clang-format - args: [-i, --version=14] + args: [-i] types_or: [c, c++] - repo: https://github.com/cheshirekow/cmake-format-precommit rev: v0.6.13 diff --git a/c/driver/postgresql/copy/postgres_copy_reader_test.cc b/c/driver/postgresql/copy/postgres_copy_reader_test.cc index 55a61b27ed..366ad7ddf1 100644 --- a/c/driver/postgresql/copy/postgres_copy_reader_test.cc +++ b/c/driver/postgresql/copy/postgres_copy_reader_test.cc @@ -18,8 +18,8 @@ #include #include -#include "postgresql/copy/reader.h" #include "postgres_copy_test_common.h" +#include "postgresql/copy/reader.h" namespace adbcpq { @@ -53,7 +53,6 @@ class PostgresCopyStreamTester { PostgresCopyStreamReader reader_; }; - TEST(PostgresCopyUtilsTest, PostgresCopyReadBoolean) { ArrowBufferView data; data.data.as_uint8 = kTestPgCopyBoolean; @@ -91,7 +90,6 @@ TEST(PostgresCopyUtilsTest, PostgresCopyReadBoolean) { ASSERT_FALSE(ArrowBitGet(data_buffer, 2)); } - TEST(PostgresCopyUtilsTest, PostgresCopyReadSmallInt) { ArrowBufferView data; data.data.as_uint8 = kTestPgCopySmallInt; @@ -130,7 +128,6 @@ TEST(PostgresCopyUtilsTest, PostgresCopyReadSmallInt) { ASSERT_EQ(data_buffer[4], 0); } - TEST(PostgresCopyUtilsTest, PostgresCopyReadInteger) { ArrowBufferView data; data.data.as_uint8 = kTestPgCopyInteger; @@ -169,7 +166,6 @@ TEST(PostgresCopyUtilsTest, PostgresCopyReadInteger) { ASSERT_EQ(data_buffer[4], 0); } - TEST(PostgresCopyUtilsTest, PostgresCopyReadBigInt) { ArrowBufferView data; data.data.as_uint8 = kTestPgCopyBigInt; @@ -208,7 +204,6 @@ TEST(PostgresCopyUtilsTest, PostgresCopyReadBigInt) { ASSERT_EQ(data_buffer[4], 0); } - TEST(PostgresCopyUtilsTest, PostgresCopyReadReal) { ArrowBufferView data; data.data.as_uint8 = kTestPgCopyReal; @@ -247,7 +242,6 @@ TEST(PostgresCopyUtilsTest, PostgresCopyReadReal) { ASSERT_EQ(data_buffer[4], 0); } - TEST(PostgresCopyUtilsTest, PostgresCopyReadDoublePrecision) { ArrowBufferView data; data.data.as_uint8 = kTestPgCopyDoublePrecision; @@ -287,7 +281,6 @@ TEST(PostgresCopyUtilsTest, PostgresCopyReadDoublePrecision) { ASSERT_EQ(data_buffer[4], 0); } - TEST(PostgresCopyUtilsTest, PostgresCopyReadDate) { ArrowBufferView data; data.data.as_uint8 = kTestPgCopyDate; @@ -321,7 +314,6 @@ TEST(PostgresCopyUtilsTest, PostgresCopyReadDate) { ASSERT_EQ(data_buffer[1], 47482); } - // For full coverage, ensure that this contains NUMERIC examples that: // - Have >= four zeroes to the left of the decimal point // - Have >= four zeroes to the right of the decimal point @@ -404,7 +396,6 @@ TEST(PostgresCopyUtilsTest, PostgresCopyReadNumeric) { EXPECT_EQ(std::string(item.data, item.size_bytes), "inf"); } - TEST(PostgresCopyUtilsTest, PostgresCopyReadTimestamp) { ArrowBufferView data; data.data.as_uint8 = kTestPgCopyTimestamp; @@ -438,7 +429,6 @@ TEST(PostgresCopyUtilsTest, PostgresCopyReadTimestamp) { ASSERT_EQ(data_buffer[1], 4102490096000000); } - TEST(PostgresCopyUtilsTest, PostgresCopyReadInterval) { ArrowBufferView data; data.data.as_uint8 = kTestPgCopyInterval; @@ -486,7 +476,6 @@ TEST(PostgresCopyUtilsTest, PostgresCopyReadInterval) { ASSERT_EQ(interval.ns, 4000000000); } - TEST(PostgresCopyUtilsTest, PostgresCopyReadText) { ArrowBufferView data; data.data.as_uint8 = kTestPgCopyText; @@ -526,7 +515,6 @@ TEST(PostgresCopyUtilsTest, PostgresCopyReadText) { ASSERT_EQ(std::string(data_buffer + 3, 4), "1234"); } - TEST(PostgresCopyUtilsTest, PostgresCopyReadBinary) { ArrowBufferView data; data.data.as_uint8 = kTestPgCopyBinary; @@ -576,7 +564,6 @@ TEST(PostgresCopyUtilsTest, PostgresCopyReadBinary) { ASSERT_EQ(data_buffer[7], 0xff); } - // COPY (SELECT CAST("col" AS INTEGER ARRAY) AS "col" FROM ( VALUES ('{-123, -1}'), ('{0, // 1, 123}'), (NULL)) AS drvd("col")) TO STDOUT WITH (FORMAT binary); static uint8_t kTestPgCopyIntegerArray[] = { diff --git a/c/driver/postgresql/copy/postgres_copy_test_common.h b/c/driver/postgresql/copy/postgres_copy_test_common.h index cdc256d77a..7f1e14872c 100644 --- a/c/driver/postgresql/copy/postgres_copy_test_common.h +++ b/c/driver/postgresql/copy/postgres_copy_test_common.h @@ -28,7 +28,6 @@ static uint8_t kTestPgCopyBoolean[] = { 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x01, 0x00, 0x00, 0x00, 0x01, 0x01, 0x00, 0x01, 0x00, 0x00, 0x00, 0x01, 0x00, 0x00, 0x01, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff}; - // COPY (SELECT CAST("col" AS SMALLINT) AS "col" FROM ( VALUES (-123), (-1), (1), (123), // (NULL)) AS drvd("col")) TO STDOUT WITH (FORMAT binary); static uint8_t kTestPgCopySmallInt[] = { @@ -47,7 +46,6 @@ static uint8_t kTestPgCopyInteger[] = { 0x00, 0x00, 0x04, 0x00, 0x00, 0x00, 0x01, 0x00, 0x01, 0x00, 0x00, 0x00, 0x04, 0x00, 0x00, 0x00, 0x7b, 0x00, 0x01, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff}; - // COPY (SELECT CAST("col" AS BIGINT) AS "col" FROM ( VALUES (-123), (-1), (1), (123), // (NULL)) AS drvd("col")) TO STDOUT WITH (FORMAT binary); static uint8_t kTestPgCopyBigInt[] = { @@ -78,29 +76,28 @@ static uint8_t kTestPgCopyDoublePrecision[] = { 0x2f, 0x1a, 0x9f, 0xbe, 0x77, 0x00, 0x01, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff}; static uint8_t kTestPgCopyDate[] = { - 0x50, 0x47, 0x43, 0x4f, 0x50, 0x59, 0x0a, 0xff, 0x0d, 0x0a, 0x00, 0x00, 0x00, 0x00, - 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x01, 0x00, 0x00, 0x00, 0x04, 0xff, 0xff, - 0x71, 0x54, 0x00, 0x01, 0x00, 0x00, 0x00, 0x04, 0x00, 0x00, 0x8e, 0xad, 0x00, 0x01, - 0xff, 0xff, 0xff, 0xff, 0xff, 0xff}; + 0x50, 0x47, 0x43, 0x4f, 0x50, 0x59, 0x0a, 0xff, 0x0d, 0x0a, 0x00, 0x00, + 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x01, 0x00, 0x00, 0x00, + 0x04, 0xff, 0xff, 0x71, 0x54, 0x00, 0x01, 0x00, 0x00, 0x00, 0x04, 0x00, + 0x00, 0x8e, 0xad, 0x00, 0x01, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff}; // COPY (SELECT CAST(col AS TIMESTAMP) FROM ( VALUES ('1900-01-01 12:34:56'), // ('2100-01-01 12:34:56'), (NULL)) AS drvd("col")) TO STDOUT WITH (FORMAT BINARY); static uint8_t kTestPgCopyTimestamp[] = { - 0x50, 0x47, 0x43, 0x4f, 0x50, 0x59, 0x0a, 0xff, 0x0d, 0x0a, 0x00, 0x00, - 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x01, 0x00, 0x00, - 0x00, 0x08, 0xff, 0xf4, 0xc9, 0xf9, 0x07, 0xe5, 0x9c, 0x00, 0x00, - 0x01, 0x00, 0x00, 0x00, 0x08, 0x00, 0x0b, 0x36, 0x30, 0x2d, 0xa5, - 0xfc, 0x00, 0x00, 0x01, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff}; + 0x50, 0x47, 0x43, 0x4f, 0x50, 0x59, 0x0a, 0xff, 0x0d, 0x0a, 0x00, 0x00, 0x00, 0x00, + 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x01, 0x00, 0x00, 0x00, 0x08, 0xff, 0xf4, 0xc9, + 0xf9, 0x07, 0xe5, 0x9c, 0x00, 0x00, 0x01, 0x00, 0x00, 0x00, 0x08, 0x00, 0x0b, 0x36, + 0x30, 0x2d, 0xa5, 0xfc, 0x00, 0x00, 0x01, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff}; // COPY (SELECT CAST(col AS INTERVAL) FROM ( VALUES ('-1 months -2 days -4 seconds'), // ('1 months 2 days 4 seconds'), (NULL)) AS drvd("col")) TO STDOUT WITH (FORMAT BINARY); static uint8_t kTestPgCopyInterval[] = { - 0x50, 0x47, 0x43, 0x4f, 0x50, 0x59, 0x0a, 0xff, 0x0d, 0x0a, 0x00, 0x00, 0x00, 0x00, - 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x01, 0x00, 0x00, 0x00, 0x10, 0xff, - 0xff, 0xff, 0xff, 0xff, 0xc2, 0xf7, 0x00, 0xff, 0xff, 0xff, 0xfe, 0xff, 0xff, 0xff, - 0xff, 0x00, 0x01, 0x00, 0x00, 0x00, 0x10, 0x00, 0x00, 0x00, 0x00, 0x00, 0x3d, 0x09, - 0x00, 0x00, 0x00, 0x00, 0x02, 0x00, 0x00, 0x00, 0x01, 0x00, 0x01, 0xff, 0xff, 0xff, - 0xff, 0xff, 0xff}; + 0x50, 0x47, 0x43, 0x4f, 0x50, 0x59, 0x0a, 0xff, 0x0d, 0x0a, 0x00, 0x00, + 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x01, 0x00, 0x00, 0x00, + 0x10, 0xff, 0xff, 0xff, 0xff, 0xff, 0xc2, 0xf7, 0x00, 0xff, 0xff, 0xff, + 0xfe, 0xff, 0xff, 0xff, 0xff, 0x00, 0x01, 0x00, 0x00, 0x00, 0x10, 0x00, + 0x00, 0x00, 0x00, 0x00, 0x3d, 0x09, 0x00, 0x00, 0x00, 0x00, 0x02, 0x00, + 0x00, 0x00, 0x01, 0x00, 0x01, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff}; // COPY (SELECT CAST("col" AS TEXT) AS "col" FROM ( VALUES ('abc'), ('1234'), // (NULL::text)) AS drvd("col")) TO STDOUT WITH (FORMAT binary); @@ -114,11 +111,10 @@ static uint8_t kTestPgCopyText[] = { // ('\x01020304'), ('\xFEFF'), (NULL)) AS drvd("col")) TO STDOUT // WITH (FORMAT binary); static uint8_t kTestPgCopyBinary[] = { - 0x50, 0x47, 0x43, 0x4f, 0x50, 0x59, 0x0a, 0xff, 0x0d, 0x0a, 0x00, 0x00, 0x00, - 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x01, 0x00, 0x00, 0x00, 0x00, - 0x00, 0x01, 0x00, 0x00, 0x00, 0x02, 0x00, 0x01, 0x00, 0x01, 0x00, 0x00, 0x00, 0x04, - 0x01, 0x02, 0x03, 0x04, 0x00, 0x01, 0x00, 0x00, 0x00, 0x02, 0xfe, 0xff, - 0x00, 0x01, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff}; - + 0x50, 0x47, 0x43, 0x4f, 0x50, 0x59, 0x0a, 0xff, 0x0d, 0x0a, 0x00, 0x00, + 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x01, 0x00, 0x00, 0x00, + 0x00, 0x00, 0x01, 0x00, 0x00, 0x00, 0x02, 0x00, 0x01, 0x00, 0x01, 0x00, + 0x00, 0x00, 0x04, 0x01, 0x02, 0x03, 0x04, 0x00, 0x01, 0x00, 0x00, 0x00, + 0x02, 0xfe, 0xff, 0x00, 0x01, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff}; } // namespace adbcpq diff --git a/c/driver/postgresql/copy/postgres_copy_writer_test.cc b/c/driver/postgresql/copy/postgres_copy_writer_test.cc index 2a33b47776..f701a7239a 100644 --- a/c/driver/postgresql/copy/postgres_copy_writer_test.cc +++ b/c/driver/postgresql/copy/postgres_copy_writer_test.cc @@ -22,9 +22,9 @@ #include #include +#include "postgres_copy_test_common.h" #include "postgresql/copy/writer.h" #include "validation/adbc_validation_util.h" -#include "postgres_copy_test_common.h" namespace adbcpq { @@ -67,7 +67,6 @@ class PostgresCopyStreamWriteTester { PostgresCopyStreamWriter writer_; }; - TEST(PostgresCopyUtilsTest, PostgresCopyWriteBoolean) { adbc_validation::Handle schema; adbc_validation::Handle array; @@ -100,7 +99,7 @@ TEST(PostgresCopyUtilsTest, PostgresCopyWriteInt8) { ASSERT_EQ(adbc_validation::MakeSchema(&schema.value, {{"col", NANOARROW_TYPE_INT8}}), ADBC_STATUS_OK); ASSERT_EQ(adbc_validation::MakeBatch(&schema.value, &array.value, &na_error, - {-123, -1, 1, 123, std::nullopt}), + {-123, -1, 1, 123, std::nullopt}), ADBC_STATUS_OK); PostgresCopyStreamWriteTester tester; @@ -141,7 +140,6 @@ TEST(PostgresCopyUtilsTest, PostgresCopyWriteInt16) { } } - TEST(PostgresCopyUtilsTest, PostgresCopyWriteInt32) { adbc_validation::Handle schema; adbc_validation::Handle array; @@ -166,7 +164,6 @@ TEST(PostgresCopyUtilsTest, PostgresCopyWriteInt32) { } } - TEST(PostgresCopyUtilsTest, PostgresCopyWriteInt64) { adbc_validation::Handle schema; adbc_validation::Handle array; @@ -191,7 +188,6 @@ TEST(PostgresCopyUtilsTest, PostgresCopyWriteInt64) { } } - TEST(PostgresCopyUtilsTest, PostgresCopyWriteReal) { adbc_validation::Handle schema; adbc_validation::Handle array; @@ -216,7 +212,6 @@ TEST(PostgresCopyUtilsTest, PostgresCopyWriteReal) { } } - TEST(PostgresCopyUtilsTest, PostgresCopyWriteDoublePrecision) { adbc_validation::Handle schema; adbc_validation::Handle array; @@ -307,15 +302,17 @@ TEST(PostgresCopyUtilsTest, PostgresCopyWriteNumeric) { ArrowDecimalSetInt(&decimal5, 100000000000000); const std::vector> values = { - std::nullopt, &decimal1, &decimal2, &decimal3, &decimal4, &decimal5}; + std::nullopt, &decimal1, &decimal2, &decimal3, &decimal4, &decimal5}; ArrowSchemaInit(&schema.value); ASSERT_EQ(ArrowSchemaSetTypeStruct(&schema.value, 1), 0); - ASSERT_EQ(AdbcNsArrowSchemaSetTypeDecimal(schema.value.children[0], - type, precision, scale), 0); + ASSERT_EQ( + AdbcNsArrowSchemaSetTypeDecimal(schema.value.children[0], type, precision, scale), + 0); ASSERT_EQ(ArrowSchemaSetName(schema.value.children[0], "col"), 0); ASSERT_EQ(adbc_validation::MakeBatch(&schema.value, &array.value, - &na_error, values), ADBC_STATUS_OK); + &na_error, values), + ADBC_STATUS_OK); PostgresCopyStreamWriteTester tester; ASSERT_EQ(tester.Init(&schema.value, &array.value), NANOARROW_OK); @@ -331,13 +328,11 @@ TEST(PostgresCopyUtilsTest, PostgresCopyWriteNumeric) { } } -using TimestampTestParamType = std::tuple>>; +using TimestampTestParamType = + std::tuple>>; -class PostgresCopyWriteTimestampTest : public testing::TestWithParam< - TimestampTestParamType> { -}; +class PostgresCopyWriteTimestampTest + : public testing::TestWithParam {}; TEST_P(PostgresCopyWriteTimestampTest, WritesProperBufferValues) { adbc_validation::Handle schema; @@ -352,16 +347,12 @@ TEST_P(PostgresCopyWriteTimestampTest, WritesProperBufferValues) { ArrowSchemaInit(&schema.value); ArrowSchemaSetTypeStruct(&schema.value, 1); - ArrowSchemaSetTypeDateTime(schema->children[0], - NANOARROW_TYPE_TIMESTAMP, - unit, + ArrowSchemaSetTypeDateTime(schema->children[0], NANOARROW_TYPE_TIMESTAMP, unit, timezone); ArrowSchemaSetName(schema->children[0], "col"); - ASSERT_EQ(adbc_validation::MakeBatch(&schema.value, - &array.value, - &na_error, - values), - ADBC_STATUS_OK); + ASSERT_EQ( + adbc_validation::MakeBatch(&schema.value, &array.value, &na_error, values), + ADBC_STATUS_OK); PostgresCopyStreamWriteTester tester; ASSERT_EQ(tester.Init(&schema.value, &array.value), NANOARROW_OK); @@ -377,35 +368,38 @@ TEST_P(PostgresCopyWriteTimestampTest, WritesProperBufferValues) { } } -static const std::vector ts_values { - {NANOARROW_TIME_UNIT_SECOND, nullptr, - {-2208943504, 4102490096, std::nullopt}}, - {NANOARROW_TIME_UNIT_MILLI, nullptr, - {-2208943504000, 4102490096000, std::nullopt}}, - {NANOARROW_TIME_UNIT_MICRO, nullptr, - {-2208943504000000, 4102490096000000, std::nullopt}}, - {NANOARROW_TIME_UNIT_NANO, nullptr, - {-2208943504000000000, 4102490096000000000, std::nullopt}}, - {NANOARROW_TIME_UNIT_SECOND, "UTC", - {-2208943504, 4102490096, std::nullopt}}, - {NANOARROW_TIME_UNIT_MILLI, "UTC", - {-2208943504000, 4102490096000, std::nullopt}}, - {NANOARROW_TIME_UNIT_MICRO, "UTC", - {-2208943504000000, 4102490096000000, std::nullopt}}, - {NANOARROW_TIME_UNIT_NANO, "UTC", - {-2208943504000000000, 4102490096000000000, std::nullopt}}, - {NANOARROW_TIME_UNIT_SECOND, "America/New_York", - {-2208943504, 4102490096, std::nullopt}}, - {NANOARROW_TIME_UNIT_MILLI, "America/New_York", - {-2208943504000, 4102490096000, std::nullopt}}, - {NANOARROW_TIME_UNIT_MICRO, "America/New_York", - {-2208943504000000, 4102490096000000, std::nullopt}}, - {NANOARROW_TIME_UNIT_NANO, "America/New_York", - {-2208943504000000000, 4102490096000000000, std::nullopt}}, +static const std::vector ts_values{ + {NANOARROW_TIME_UNIT_SECOND, nullptr, {-2208943504, 4102490096, std::nullopt}}, + {NANOARROW_TIME_UNIT_MILLI, nullptr, {-2208943504000, 4102490096000, std::nullopt}}, + {NANOARROW_TIME_UNIT_MICRO, + nullptr, + {-2208943504000000, 4102490096000000, std::nullopt}}, + {NANOARROW_TIME_UNIT_NANO, + nullptr, + {-2208943504000000000, 4102490096000000000, std::nullopt}}, + {NANOARROW_TIME_UNIT_SECOND, "UTC", {-2208943504, 4102490096, std::nullopt}}, + {NANOARROW_TIME_UNIT_MILLI, "UTC", {-2208943504000, 4102490096000, std::nullopt}}, + {NANOARROW_TIME_UNIT_MICRO, + "UTC", + {-2208943504000000, 4102490096000000, std::nullopt}}, + {NANOARROW_TIME_UNIT_NANO, + "UTC", + {-2208943504000000000, 4102490096000000000, std::nullopt}}, + {NANOARROW_TIME_UNIT_SECOND, + "America/New_York", + {-2208943504, 4102490096, std::nullopt}}, + {NANOARROW_TIME_UNIT_MILLI, + "America/New_York", + {-2208943504000, 4102490096000, std::nullopt}}, + {NANOARROW_TIME_UNIT_MICRO, + "America/New_York", + {-2208943504000000, 4102490096000000, std::nullopt}}, + {NANOARROW_TIME_UNIT_NANO, + "America/New_York", + {-2208943504000000000, 4102490096000000000, std::nullopt}}, }; -INSTANTIATE_TEST_SUITE_P(PostgresCopyWriteTimestamp, - PostgresCopyWriteTimestampTest, +INSTANTIATE_TEST_SUITE_P(PostgresCopyWriteTimestamp, PostgresCopyWriteTimestampTest, testing::ValuesIn(ts_values)); TEST(PostgresCopyUtilsTest, PostgresCopyWriteInterval) { @@ -428,13 +422,14 @@ TEST(PostgresCopyUtilsTest, PostgresCopyWriteInterval) { pos_interval.days = 2; pos_interval.ns = 4000000000; - const std::vector> values = { - &neg_interval, &pos_interval, std::nullopt}; + const std::vector> values = {&neg_interval, &pos_interval, + std::nullopt}; ASSERT_EQ(adbc_validation::MakeSchema(&schema.value, {{"col", type}}), ADBC_STATUS_OK); - ASSERT_EQ(adbc_validation::MakeBatch( - &schema.value, &array.value, &na_error, values), ADBC_STATUS_OK); + ASSERT_EQ(adbc_validation::MakeBatch(&schema.value, &array.value, + &na_error, values), + ADBC_STATUS_OK); PostgresCopyStreamWriteTester tester; ASSERT_EQ(tester.Init(&schema.value, &array.value), NANOARROW_OK); @@ -454,17 +449,17 @@ TEST(PostgresCopyUtilsTest, PostgresCopyWriteInterval) { // COPY (SELECT CAST(col AS INTERVAL) FROM ( VALUES ('-4 seconds'), // ('4 seconds'), (NULL)) AS drvd("col")) TO STDOUT WITH (FORMAT BINARY); static uint8_t kTestPgCopyDuration[] = { - 0x50, 0x47, 0x43, 0x4f, 0x50, 0x59, 0x0a, 0xff, 0x0d, 0x0a, 0x00, 0x00, 0x00, 0x00, - 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x01, 0x00, 0x00, 0x00, 0x10, 0xff, - 0xff, 0xff, 0xff, 0xff, 0xc2, 0xf7, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, - 0x00, 0x00, 0x01, 0x00, 0x00, 0x00, 0x10, 0x00, 0x00, 0x00, 0x00, 0x00, 0x3d, 0x09, - 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x01, 0xff, 0xff, 0xff, - 0xff, 0xff, 0xff}; -using DurationTestParamType = std::tuple>>; - -class PostgresCopyWriteDurationTest : public testing::TestWithParam< - DurationTestParamType> {}; + 0x50, 0x47, 0x43, 0x4f, 0x50, 0x59, 0x0a, 0xff, 0x0d, 0x0a, 0x00, 0x00, + 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x01, 0x00, 0x00, 0x00, + 0x10, 0xff, 0xff, 0xff, 0xff, 0xff, 0xc2, 0xf7, 0x00, 0x00, 0x00, 0x00, + 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x01, 0x00, 0x00, 0x00, 0x10, 0x00, + 0x00, 0x00, 0x00, 0x00, 0x3d, 0x09, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, + 0x00, 0x00, 0x00, 0x00, 0x01, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff}; +using DurationTestParamType = + std::tuple>>; + +class PostgresCopyWriteDurationTest + : public testing::TestWithParam {}; TEST_P(PostgresCopyWriteDurationTest, WritesProperBufferValues) { adbc_validation::Handle schema; @@ -480,8 +475,9 @@ TEST_P(PostgresCopyWriteDurationTest, WritesProperBufferValues) { ArrowSchemaSetTypeStruct(&schema.value, 1); ArrowSchemaSetTypeDateTime(schema->children[0], type, unit, nullptr); ArrowSchemaSetName(schema->children[0], "col"); - ASSERT_EQ(adbc_validation::MakeBatch( - &schema.value, &array.value, &na_error, values), ADBC_STATUS_OK); + ASSERT_EQ( + adbc_validation::MakeBatch(&schema.value, &array.value, &na_error, values), + ADBC_STATUS_OK); PostgresCopyStreamWriteTester tester; ASSERT_EQ(tester.Init(&schema.value, &array.value), NANOARROW_OK); @@ -497,18 +493,16 @@ TEST_P(PostgresCopyWriteDurationTest, WritesProperBufferValues) { } } -static const std::vector duration_params { - {NANOARROW_TIME_UNIT_SECOND, {-4, 4, std::nullopt}}, - {NANOARROW_TIME_UNIT_MILLI, {-4000, 4000, std::nullopt}}, - {NANOARROW_TIME_UNIT_MICRO, {-4000000, 4000000, std::nullopt}}, - {NANOARROW_TIME_UNIT_NANO, {-4000000000, 4000000000, std::nullopt}}, +static const std::vector duration_params{ + {NANOARROW_TIME_UNIT_SECOND, {-4, 4, std::nullopt}}, + {NANOARROW_TIME_UNIT_MILLI, {-4000, 4000, std::nullopt}}, + {NANOARROW_TIME_UNIT_MICRO, {-4000000, 4000000, std::nullopt}}, + {NANOARROW_TIME_UNIT_NANO, {-4000000000, 4000000000, std::nullopt}}, }; -INSTANTIATE_TEST_SUITE_P(PostgresCopyWriteDuration, - PostgresCopyWriteDurationTest, +INSTANTIATE_TEST_SUITE_P(PostgresCopyWriteDuration, PostgresCopyWriteDurationTest, testing::ValuesIn(duration_params)); - TEST(PostgresCopyUtilsTest, PostgresCopyWriteString) { adbc_validation::Handle schema; adbc_validation::Handle array; @@ -565,15 +559,12 @@ TEST(PostgresCopyUtilsTest, PostgresCopyWriteBinary) { ASSERT_EQ(adbc_validation::MakeSchema(&schema.value, {{"col", NANOARROW_TYPE_BINARY}}), ADBC_STATUS_OK); ASSERT_EQ(adbc_validation::MakeBatch>( - &schema.value, &array.value, &na_error, - { - std::vector{}, - std::vector{std::byte{0x00}, std::byte{0x01}}, - std::vector{ - std::byte{0x01}, std::byte{0x02}, std::byte{0x03}, std::byte{0x04} - }, - std::vector{std::byte{0xfe}, std::byte{0xff}}, - std::nullopt}), + &schema.value, &array.value, &na_error, + {std::vector{}, + std::vector{std::byte{0x00}, std::byte{0x01}}, + std::vector{std::byte{0x01}, std::byte{0x02}, std::byte{0x03}, + std::byte{0x04}}, + std::vector{std::byte{0xfe}, std::byte{0xff}}, std::nullopt}), ADBC_STATUS_OK); PostgresCopyStreamWriteTester tester; @@ -590,7 +581,6 @@ TEST(PostgresCopyUtilsTest, PostgresCopyWriteBinary) { } } - TEST(PostgresCopyUtilsTest, PostgresCopyWriteMultiBatch) { // Regression test for https://github.com/apache/arrow-adbc/issues/1310 adbc_validation::Handle schema; diff --git a/c/driver/postgresql/copy/reader.h b/c/driver/postgresql/copy/reader.h index 8ba0568ad9..0ab3f95e65 100644 --- a/c/driver/postgresql/copy/reader.h +++ b/c/driver/postgresql/copy/reader.h @@ -26,9 +26,9 @@ #include -#include "copy_common.h" #include "../postgres_type.h" #include "../postgres_util.h" +#include "copy_common.h" namespace adbcpq { diff --git a/c/driver/postgresql/copy/writer.h b/c/driver/postgresql/copy/writer.h index b04f370f77..320b89e6c7 100644 --- a/c/driver/postgresql/copy/writer.h +++ b/c/driver/postgresql/copy/writer.h @@ -26,8 +26,8 @@ #include -#include "copy_common.h" #include "../postgres_util.h" +#include "copy_common.h" namespace adbcpq { @@ -47,7 +47,6 @@ constexpr int64_t kMaxSafeMillisToMicros = 9223372036854775L; // without overflow constexpr int64_t kMinSafeMillisToMicros = -9223372036854775L; - // 2000-01-01 00:00:00.000000 in microseconds constexpr int64_t kPostgresTimestampEpoch = 946684800000000L; @@ -211,11 +210,11 @@ class PostgresCopyIntervalFieldWriter : public PostgresCopyFieldWriter { // Inspiration for this taken from get_str_from_var in the pg source // src/backend/utils/adt/numeric.c -template +template class PostgresCopyNumericFieldWriter : public PostgresCopyFieldWriter { -public: - PostgresCopyNumericFieldWriter(int32_t precision, int32_t scale) : - precision_{precision}, scale_{scale} {} + public: + PostgresCopyNumericFieldWriter(int32_t precision, int32_t scale) + : precision_{precision}, scale_{scale} {} ArrowErrorCode Write(ArrowBuffer* buffer, int64_t index, ArrowError* error) override { struct ArrowDecimal decimal; @@ -235,8 +234,8 @@ class PostgresCopyNumericFieldWriter : public PostgresCopyFieldWriter { char decimal_string[max_decimal_digits_ + 1]; int digits_remaining = DecimalToString(&decimal, decimal_string); do { - const int start_pos = digits_remaining < kDecDigits ? - 0 : digits_remaining - kDecDigits; + const int start_pos = + digits_remaining < kDecDigits ? 0 : digits_remaining - kDecDigits; const size_t len = digits_remaining < 4 ? digits_remaining : kDecDigits; char substr[kDecDigits + 1]; std::memcpy(substr, decimal_string + start_pos, len); @@ -272,11 +271,8 @@ class PostgresCopyNumericFieldWriter : public PostgresCopyFieldWriter { } while (true); int16_t ndigits = pg_digits.size(); - int32_t field_size_bytes = sizeof(ndigits) - + sizeof(weight) - + sizeof(sign) - + sizeof(dscale) - + ndigits * sizeof(int16_t); + int32_t field_size_bytes = sizeof(ndigits) + sizeof(weight) + sizeof(sign) + + sizeof(dscale) + ndigits * sizeof(int16_t); NANOARROW_RETURN_NOT_OK(WriteChecked(buffer, field_size_bytes, error)); NANOARROW_RETURN_NOT_OK(WriteChecked(buffer, ndigits, error)); @@ -293,7 +289,7 @@ class PostgresCopyNumericFieldWriter : public PostgresCopyFieldWriter { return ADBC_STATUS_OK; } -private: + private: // returns the length of the string template int DecimalToString(struct ArrowDecimal* decimal, char* out) { @@ -321,11 +317,12 @@ class PostgresCopyNumericFieldWriter : public PostgresCopyFieldWriter { carry = (buf[nwords - 1] >= 0x7FFFFFFFFFFFFFFF); for (size_t j = nwords - 1; j > 0; j--) { - buf[j] = ((buf[j] << 1) & 0xFFFFFFFFFFFFFFFF) + (buf[j-1] >= 0x7FFFFFFFFFFFFFFF); + buf[j] = + ((buf[j] << 1) & 0xFFFFFFFFFFFFFFFF) + (buf[j - 1] >= 0x7FFFFFFFFFFFFFFF); } buf[0] = ((buf[0] << 1) & 0xFFFFFFFFFFFFFFFF); - for (int j = sizeof(s) - 2; j>= 0; j--) { + for (int j = sizeof(s) - 2; j >= 0; j--) { s[j] += s[j] - '0' + carry; carry = (s[j] > '9'); if (carry) { @@ -350,7 +347,7 @@ class PostgresCopyNumericFieldWriter : public PostgresCopyFieldWriter { static constexpr uint16_t kNumericNeg = 0x4000; static constexpr int32_t bitwidth_ = (T == NANOARROW_TYPE_DECIMAL128) ? 128 : 256; static constexpr size_t max_decimal_digits_ = - (T == NANOARROW_TYPE_DECIMAL128) ? 39 : 78; + (T == NANOARROW_TYPE_DECIMAL128) ? 39 : 78; const int32_t precision_; const int32_t scale_; }; @@ -477,7 +474,7 @@ class PostgresCopyTimestampFieldWriter : public PostgresCopyFieldWriter { return ADBC_STATUS_INVALID_ARGUMENT; } - if (value < std::numeric_limits::min() + kPostgresTimestampEpoch) { + if (value < (std::numeric_limits::min)() + kPostgresTimestampEpoch) { ArrowErrorSet(error, "[libpq] Row %" PRId64 " timestamp value %" PRId64 " with unit %d would underflow", @@ -526,15 +523,15 @@ static inline ArrowErrorCode MakeCopyFieldWriter(struct ArrowSchema* schema, case NANOARROW_TYPE_DECIMAL128: { const auto precision = schema_view.decimal_precision; const auto scale = schema_view.decimal_scale; - *out = new PostgresCopyNumericFieldWriter< - NANOARROW_TYPE_DECIMAL128>(precision, scale); + *out = + new PostgresCopyNumericFieldWriter(precision, scale); return NANOARROW_OK; } case NANOARROW_TYPE_DECIMAL256: { const auto precision = schema_view.decimal_precision; const auto scale = schema_view.decimal_scale; - *out = new PostgresCopyNumericFieldWriter< - NANOARROW_TYPE_DECIMAL256>(precision, scale); + *out = + new PostgresCopyNumericFieldWriter(precision, scale); return NANOARROW_OK; } case NANOARROW_TYPE_BINARY: @@ -670,4 +667,4 @@ class PostgresCopyStreamWriter { int64_t records_written_ = 0; }; - } // namespace adbcpq +} // namespace adbcpq diff --git a/c/driver/postgresql/postgres_util.h b/c/driver/postgresql/postgres_util.h index 95e2619f10..6d42f85a2c 100644 --- a/c/driver/postgresql/postgres_util.h +++ b/c/driver/postgresql/postgres_util.h @@ -170,7 +170,7 @@ struct Handle { Resource* operator->() { return &value; } - void reset() { Releaser::Release(&value); } + void reset() { Releaser::Release(&value); } }; } // namespace adbcpq diff --git a/c/driver/postgresql/postgresql_benchmark.cc b/c/driver/postgresql/postgresql_benchmark.cc index 908269966e..fc22429b0d 100644 --- a/c/driver/postgresql/postgresql_benchmark.cc +++ b/c/driver/postgresql/postgresql_benchmark.cc @@ -33,10 +33,9 @@ } \ } while (0) -#define ADBC_BENCHMARK_RETURN_NOT_OK(EXPR) \ - _ADBC_BENCHMARK_RETURN_NOT_OK_IMPL(_NANOARROW_MAKE_NAME(errno_status_, \ - __COUNTER__), EXPR) - +#define ADBC_BENCHMARK_RETURN_NOT_OK(EXPR) \ + _ADBC_BENCHMARK_RETURN_NOT_OK_IMPL(_NANOARROW_MAKE_NAME(errno_status_, __COUNTER__), \ + EXPR) static void BM_PostgresqlExecute(benchmark::State& state) { const char* uri = std::getenv("ADBC_POSTGRESQL_TEST_URI"); @@ -48,45 +47,39 @@ static void BM_PostgresqlExecute(benchmark::State& state) { struct AdbcError error; ADBC_BENCHMARK_RETURN_NOT_OK(AdbcDatabaseNew(&database.value, &error)); - ADBC_BENCHMARK_RETURN_NOT_OK(AdbcDatabaseSetOption(&database.value, - "uri", - uri, - &error)); + ADBC_BENCHMARK_RETURN_NOT_OK( + AdbcDatabaseSetOption(&database.value, "uri", uri, &error)); ADBC_BENCHMARK_RETURN_NOT_OK(AdbcDatabaseInit(&database.value, &error)); adbc_validation::Handle connection; ADBC_BENCHMARK_RETURN_NOT_OK(AdbcConnectionNew(&connection.value, &error)); - ADBC_BENCHMARK_RETURN_NOT_OK(AdbcConnectionInit(&connection.value, - &database.value, - &error)); + ADBC_BENCHMARK_RETURN_NOT_OK( + AdbcConnectionInit(&connection.value, &database.value, &error)); adbc_validation::Handle statement; - ADBC_BENCHMARK_RETURN_NOT_OK(AdbcStatementNew(&connection.value, - &statement.value, - &error)); + ADBC_BENCHMARK_RETURN_NOT_OK( + AdbcStatementNew(&connection.value, &statement.value, &error)); const char* drop_query = "DROP TABLE IF EXISTS adbc_postgresql_ingest_benchmark"; - ADBC_BENCHMARK_RETURN_NOT_OK(AdbcStatementSetSqlQuery(&statement.value, - drop_query, - &error)); + ADBC_BENCHMARK_RETURN_NOT_OK( + AdbcStatementSetSqlQuery(&statement.value, drop_query, &error)); - ADBC_BENCHMARK_RETURN_NOT_OK(AdbcStatementExecuteQuery(&statement.value, - nullptr, - nullptr, - &error)); + ADBC_BENCHMARK_RETURN_NOT_OK( + AdbcStatementExecuteQuery(&statement.value, nullptr, nullptr, &error)); adbc_validation::Handle schema; adbc_validation::Handle array; struct ArrowError na_error; - ADBC_BENCHMARK_RETURN_NOT_OK(adbc_validation::MakeSchema(&schema.value, { - {"bools", NANOARROW_TYPE_BOOL}, - {"int16s", NANOARROW_TYPE_INT16}, - {"int32s", NANOARROW_TYPE_INT32}, - {"int64s", NANOARROW_TYPE_INT64}, - {"floats", NANOARROW_TYPE_FLOAT}, - {"doubles", NANOARROW_TYPE_DOUBLE}, - })); + ADBC_BENCHMARK_RETURN_NOT_OK( + adbc_validation::MakeSchema(&schema.value, { + {"bools", NANOARROW_TYPE_BOOL}, + {"int16s", NANOARROW_TYPE_INT16}, + {"int32s", NANOARROW_TYPE_INT32}, + {"int64s", NANOARROW_TYPE_INT64}, + {"floats", NANOARROW_TYPE_FLOAT}, + {"doubles", NANOARROW_TYPE_DOUBLE}, + })); if (ArrowArrayInitFromSchema(&array.value, &schema.value, &na_error) != NANOARROW_OK) { state.SkipWithError("Call to ArrowArrayInitFromSchema failed!"); @@ -134,46 +127,37 @@ static void BM_PostgresqlExecute(benchmark::State& state) { } const char* create_query = - "CREATE TABLE adbc_postgresql_ingest_benchmark (bools BOOLEAN, int16s SMALLINT, " - "int32s INTEGER, int64s BIGINT, floats REAL, doubles DOUBLE PRECISION)"; + "CREATE TABLE adbc_postgresql_ingest_benchmark (bools BOOLEAN, int16s SMALLINT, " + "int32s INTEGER, int64s BIGINT, floats REAL, doubles DOUBLE PRECISION)"; - ADBC_BENCHMARK_RETURN_NOT_OK(AdbcStatementSetSqlQuery(&statement.value, - create_query, - &error)); + ADBC_BENCHMARK_RETURN_NOT_OK( + AdbcStatementSetSqlQuery(&statement.value, create_query, &error)); - ADBC_BENCHMARK_RETURN_NOT_OK(AdbcStatementExecuteQuery(&statement.value, - nullptr, - nullptr, - &error)); + ADBC_BENCHMARK_RETURN_NOT_OK( + AdbcStatementExecuteQuery(&statement.value, nullptr, nullptr, &error)); adbc_validation::Handle insert_stmt; - ADBC_BENCHMARK_RETURN_NOT_OK(AdbcStatementNew(&connection.value, - &insert_stmt.value, - &error)); + ADBC_BENCHMARK_RETURN_NOT_OK( + AdbcStatementNew(&connection.value, &insert_stmt.value, &error)); - ADBC_BENCHMARK_RETURN_NOT_OK(AdbcStatementSetOption(&insert_stmt.value, - ADBC_INGEST_OPTION_TARGET_TABLE, - "adbc_postgresql_ingest_benchmark", - &error)); + ADBC_BENCHMARK_RETURN_NOT_OK( + AdbcStatementSetOption(&insert_stmt.value, ADBC_INGEST_OPTION_TARGET_TABLE, + "adbc_postgresql_ingest_benchmark", &error)); - ADBC_BENCHMARK_RETURN_NOT_OK(AdbcStatementSetOption(&insert_stmt.value, - ADBC_INGEST_OPTION_MODE, - ADBC_INGEST_OPTION_MODE_APPEND, - &error)); + ADBC_BENCHMARK_RETURN_NOT_OK( + AdbcStatementSetOption(&insert_stmt.value, ADBC_INGEST_OPTION_MODE, + ADBC_INGEST_OPTION_MODE_APPEND, &error)); for (auto _ : state) { AdbcStatementBind(&insert_stmt.value, &array.value, &schema.value, &error); AdbcStatementExecuteQuery(&insert_stmt.value, nullptr, nullptr, &error); } - ADBC_BENCHMARK_RETURN_NOT_OK(AdbcStatementSetSqlQuery(&statement.value, - drop_query, - &error)); + ADBC_BENCHMARK_RETURN_NOT_OK( + AdbcStatementSetSqlQuery(&statement.value, drop_query, &error)); - ADBC_BENCHMARK_RETURN_NOT_OK(AdbcStatementExecuteQuery(&statement.value, - nullptr, - nullptr, - &error)); + ADBC_BENCHMARK_RETURN_NOT_OK( + AdbcStatementExecuteQuery(&statement.value, nullptr, nullptr, &error)); } static void BM_PostgresqlDecimalWrite(benchmark::State& state) { @@ -186,32 +170,25 @@ static void BM_PostgresqlDecimalWrite(benchmark::State& state) { struct AdbcError error; ADBC_BENCHMARK_RETURN_NOT_OK(AdbcDatabaseNew(&database.value, &error)); - ADBC_BENCHMARK_RETURN_NOT_OK(AdbcDatabaseSetOption(&database.value, - "uri", - uri, - &error)); + ADBC_BENCHMARK_RETURN_NOT_OK( + AdbcDatabaseSetOption(&database.value, "uri", uri, &error)); ADBC_BENCHMARK_RETURN_NOT_OK(AdbcDatabaseInit(&database.value, &error)); adbc_validation::Handle connection; ADBC_BENCHMARK_RETURN_NOT_OK(AdbcConnectionNew(&connection.value, &error)); - ADBC_BENCHMARK_RETURN_NOT_OK(AdbcConnectionInit(&connection.value, - &database.value, - &error)); + ADBC_BENCHMARK_RETURN_NOT_OK( + AdbcConnectionInit(&connection.value, &database.value, &error)); adbc_validation::Handle statement; - ADBC_BENCHMARK_RETURN_NOT_OK(AdbcStatementNew(&connection.value, - &statement.value, - &error)); + ADBC_BENCHMARK_RETURN_NOT_OK( + AdbcStatementNew(&connection.value, &statement.value, &error)); const char* drop_query = "DROP TABLE IF EXISTS adbc_postgresql_ingest_benchmark"; - ADBC_BENCHMARK_RETURN_NOT_OK(AdbcStatementSetSqlQuery(&statement.value, - drop_query, - &error)); + ADBC_BENCHMARK_RETURN_NOT_OK( + AdbcStatementSetSqlQuery(&statement.value, drop_query, &error)); - ADBC_BENCHMARK_RETURN_NOT_OK(AdbcStatementExecuteQuery(&statement.value, - nullptr, - nullptr, - &error)); + ADBC_BENCHMARK_RETURN_NOT_OK( + AdbcStatementExecuteQuery(&statement.value, nullptr, nullptr, &error)); adbc_validation::Handle schema; adbc_validation::Handle array; @@ -230,19 +207,19 @@ static void BM_PostgresqlDecimalWrite(benchmark::State& state) { } for (size_t i = 0; i < ncols; i++) { - if (AdbcNsArrowSchemaSetTypeDecimal(schema.value.children[i], - type, precision, scale) != NANOARROW_OK) { - state.SkipWithError("Call to ArrowSchemaSetTypeDecimal failed!"); - error.release(&error); - return; - } - - std::string colname = "col" + std::to_string(i); - if (ArrowSchemaSetName(schema.value.children[i], colname.c_str()) != NANOARROW_OK) { - state.SkipWithError("Call to ArrowSchemaSetName failed!"); - error.release(&error); - return; - } + if (AdbcNsArrowSchemaSetTypeDecimal(schema.value.children[i], type, precision, + scale) != NANOARROW_OK) { + state.SkipWithError("Call to ArrowSchemaSetTypeDecimal failed!"); + error.release(&error); + return; + } + + std::string colname = "col" + std::to_string(i); + if (ArrowSchemaSetName(schema.value.children[i], colname.c_str()) != NANOARROW_OK) { + state.SkipWithError("Call to ArrowSchemaSetName failed!"); + error.release(&error); + return; + } } if (ArrowArrayInitFromSchema(&array.value, &schema.value, &na_error) != NANOARROW_OK) { state.SkipWithError("Call to ArrowArrayInitFromSchema failed!"); @@ -282,46 +259,38 @@ static void BM_PostgresqlDecimalWrite(benchmark::State& state) { } const char* create_query = - "CREATE TABLE adbc_postgresql_ingest_benchmark (col0 DECIMAL(38, 8), " - "col1 DECIMAL(38, 8), col2 DECIMAL(38, 8), col3 DECIMAL(38, 8), col4 DECIMAL(38, 8))"; + "CREATE TABLE adbc_postgresql_ingest_benchmark (col0 DECIMAL(38, 8), " + "col1 DECIMAL(38, 8), col2 DECIMAL(38, 8), col3 DECIMAL(38, 8), col4 DECIMAL(38, " + "8))"; - ADBC_BENCHMARK_RETURN_NOT_OK(AdbcStatementSetSqlQuery(&statement.value, - create_query, - &error)); + ADBC_BENCHMARK_RETURN_NOT_OK( + AdbcStatementSetSqlQuery(&statement.value, create_query, &error)); - ADBC_BENCHMARK_RETURN_NOT_OK(AdbcStatementExecuteQuery(&statement.value, - nullptr, - nullptr, - &error)); + ADBC_BENCHMARK_RETURN_NOT_OK( + AdbcStatementExecuteQuery(&statement.value, nullptr, nullptr, &error)); adbc_validation::Handle insert_stmt; - ADBC_BENCHMARK_RETURN_NOT_OK(AdbcStatementNew(&connection.value, - &insert_stmt.value, - &error)); + ADBC_BENCHMARK_RETURN_NOT_OK( + AdbcStatementNew(&connection.value, &insert_stmt.value, &error)); - ADBC_BENCHMARK_RETURN_NOT_OK(AdbcStatementSetOption(&insert_stmt.value, - ADBC_INGEST_OPTION_TARGET_TABLE, - "adbc_postgresql_ingest_benchmark", - &error)); + ADBC_BENCHMARK_RETURN_NOT_OK( + AdbcStatementSetOption(&insert_stmt.value, ADBC_INGEST_OPTION_TARGET_TABLE, + "adbc_postgresql_ingest_benchmark", &error)); - ADBC_BENCHMARK_RETURN_NOT_OK(AdbcStatementSetOption(&insert_stmt.value, - ADBC_INGEST_OPTION_MODE, - ADBC_INGEST_OPTION_MODE_APPEND, - &error)); + ADBC_BENCHMARK_RETURN_NOT_OK( + AdbcStatementSetOption(&insert_stmt.value, ADBC_INGEST_OPTION_MODE, + ADBC_INGEST_OPTION_MODE_APPEND, &error)); for (auto _ : state) { AdbcStatementBind(&insert_stmt.value, &array.value, &schema.value, &error); AdbcStatementExecuteQuery(&insert_stmt.value, nullptr, nullptr, &error); } - ADBC_BENCHMARK_RETURN_NOT_OK(AdbcStatementSetSqlQuery(&statement.value, - drop_query, - &error)); + ADBC_BENCHMARK_RETURN_NOT_OK( + AdbcStatementSetSqlQuery(&statement.value, drop_query, &error)); - ADBC_BENCHMARK_RETURN_NOT_OK(AdbcStatementExecuteQuery(&statement.value, - nullptr, - nullptr, - &error)); + ADBC_BENCHMARK_RETURN_NOT_OK( + AdbcStatementExecuteQuery(&statement.value, nullptr, nullptr, &error)); } // TODO: we are limited to only 1 iteration as AdbcStatementBind is part of diff --git a/c/driver/postgresql/postgresql_test.cc b/c/driver/postgresql/postgresql_test.cc index 8ac841d8a0..6013e7e60c 100644 --- a/c/driver/postgresql/postgresql_test.cc +++ b/c/driver/postgresql/postgresql_test.cc @@ -1659,7 +1659,7 @@ struct DecimalTestCase { }; class PostgresDecimalTest : public ::testing::TestWithParam { -public: + public: void SetUp() override { ASSERT_THAT(AdbcDatabaseNew(&database_, &error_), IsOkStatus(&error_)); ASSERT_THAT(quirks_.SetupDatabase(&database_, &error_), IsOkStatus(&error_)); @@ -1690,7 +1690,7 @@ class PostgresDecimalTest : public ::testing::TestWithParam { if (error_.release) error_.release(&error_); } -protected: + protected: PostgresQuirks quirks_; struct AdbcError error_ = {}; struct AdbcDatabase database_ = {}; @@ -1712,14 +1712,14 @@ TEST_P(PostgresDecimalTest, SelectValue) { int32_t bitwidth; switch (type) { - case NANOARROW_TYPE_DECIMAL128: - bitwidth = 128; - break; - case NANOARROW_TYPE_DECIMAL256: - bitwidth = 256; - break; - default: - FAIL(); + case NANOARROW_TYPE_DECIMAL128: + bitwidth = 128; + break; + case NANOARROW_TYPE_DECIMAL256: + bitwidth = 256; + break; + default: + FAIL(); } // this is a bit of a hack to make std::vector play nicely with @@ -1727,8 +1727,7 @@ TEST_P(PostgresDecimalTest, SelectValue) { constexpr size_t max_decimals = 10; struct ArrowDecimal decimals[max_decimals]; if (nrecords > max_decimals) { - FAIL() << - " max_decimals exceeded for test case - please change parametrization"; + FAIL() << " max_decimals exceeded for test case - please change parametrization"; } std::vector> values; @@ -1747,16 +1746,16 @@ TEST_P(PostgresDecimalTest, SelectValue) { ArrowSchemaInit(&schema.value); ASSERT_EQ(ArrowSchemaSetTypeStruct(&schema.value, 1), 0); - ASSERT_EQ(AdbcNsArrowSchemaSetTypeDecimal(schema.value.children[0], - type, precision, scale), 0); + ASSERT_EQ( + AdbcNsArrowSchemaSetTypeDecimal(schema.value.children[0], type, precision, scale), + 0); ASSERT_EQ(ArrowSchemaSetName(schema.value.children[0], "col"), 0); ASSERT_THAT(adbc_validation::MakeBatch(&schema.value, &array.value, &na_error, values), adbc_validation::IsOkErrno()); - ASSERT_THAT(AdbcStatementSetOption(&statement_, - ADBC_INGEST_OPTION_TARGET_TABLE, + ASSERT_THAT(AdbcStatementSetOption(&statement_, ADBC_INGEST_OPTION_TARGET_TABLE, "bulk_ingest", &error_), IsOkStatus(&error_)); ASSERT_THAT(AdbcStatementBind(&statement_, &array.value, &schema.value, &error_), @@ -1769,8 +1768,8 @@ TEST_P(PostgresDecimalTest, SelectValue) { ::testing::AnyOf(::testing::Eq(values.size()), ::testing::Eq(-1))); ASSERT_THAT(AdbcStatementSetSqlQuery( - &statement_, - "SELECT * FROM bulk_ingest ORDER BY \"col\" ASC NULLS FIRST", &error_), + &statement_, + "SELECT * FROM bulk_ingest ORDER BY \"col\" ASC NULLS FIRST", &error_), IsOkStatus(&error_)); { @@ -1783,18 +1782,16 @@ TEST_P(PostgresDecimalTest, SelectValue) { ASSERT_NO_FATAL_FAILURE(reader.GetSchema()); ArrowType round_trip_type = quirks_.IngestSelectRoundTripType(type); - ASSERT_NO_FATAL_FAILURE(adbc_validation::CompareSchema(&reader.schema.value, - {{"col", - round_trip_type, true}})); + ASSERT_NO_FATAL_FAILURE(adbc_validation::CompareSchema( + &reader.schema.value, {{"col", round_trip_type, true}})); ASSERT_NO_FATAL_FAILURE(reader.Next()); ASSERT_NE(nullptr, reader.array->release); ASSERT_EQ(values.size(), reader.array->length); ASSERT_EQ(1, reader.array->n_children); - ASSERT_NO_FATAL_FAILURE(adbc_validation::CompareArray< - std::string>(reader.array_view->children[0], - expected_with_null)); + ASSERT_NO_FATAL_FAILURE(adbc_validation::CompareArray( + reader.array_view->children[0], expected_with_null)); ASSERT_NO_FATAL_FAILURE(reader.Next()); ASSERT_EQ(nullptr, reader.array->release); @@ -1802,74 +1799,80 @@ TEST_P(PostgresDecimalTest, SelectValue) { } static std::vector> kDecimalData = { - // -12345600000 - {18446744061363951616ULL, 18446744073709551615ULL, 0, 0}, - // 1234 - {1234ULL, 0, 0, 0}, - // 100000000 - {100000000ULL, 0, 0, 0}, - // 12345600000 - {12345600000ULL, 0, 0, 0}, - // 100000000000000 - {100000000000000ULL, 0, 0, 0}, - // 2342394230592232349023094 - {8221368519775271798ULL, 126981ULL, 0, 0}, + // -12345600000 + {18446744061363951616ULL, 18446744073709551615ULL, 0, 0}, + // 1234 + {1234ULL, 0, 0, 0}, + // 100000000 + {100000000ULL, 0, 0, 0}, + // 12345600000 + {12345600000ULL, 0, 0, 0}, + // 100000000000000 + {100000000000000ULL, 0, 0, 0}, + // 2342394230592232349023094 + {8221368519775271798ULL, 126981ULL, 0, 0}, }; static std::vector> kDecimal256Data = { - // 1234567890123456789012345678901234567890123456789012345678901234567890123456 - {17877984925544397504ULL, 5352188884907840935ULL, 234631617561833724ULL, - 196678011949953713ULL}, - // -1234567890123456789012345678901234567890123456789012345678901234567890123456 - {568759148165154112ULL, 13094555188801710680ULL, 18212112456147717891ULL, - 18250066061759597902ULL}, + // 1234567890123456789012345678901234567890123456789012345678901234567890123456 + {17877984925544397504ULL, 5352188884907840935ULL, 234631617561833724ULL, + 196678011949953713ULL}, + // -1234567890123456789012345678901234567890123456789012345678901234567890123456 + {568759148165154112ULL, 13094555188801710680ULL, 18212112456147717891ULL, + 18250066061759597902ULL}, }; static std::initializer_list kDecimal128Cases = { - { - NANOARROW_TYPE_DECIMAL128, 38, 8, kDecimalData, - {"-123.456", "0.00001234", "1", "123.456", "1000000", - "23423942305922323.49023094"} - }}; + {NANOARROW_TYPE_DECIMAL128, + 38, + 8, + kDecimalData, + {"-123.456", "0.00001234", "1", "123.456", "1000000", + "23423942305922323.49023094"}}}; static std::initializer_list kDecimal128NoScaleCases = { - { - NANOARROW_TYPE_DECIMAL128, 38, 0, kDecimalData, - {"-12345600000", "1234", "100000000", "12345600000", "100000000000000", - "2342394230592232349023094"} - }}; + {NANOARROW_TYPE_DECIMAL128, + 38, + 0, + kDecimalData, + {"-12345600000", "1234", "100000000", "12345600000", "100000000000000", + "2342394230592232349023094"}}}; static std::initializer_list kDecimal256Cases = { - { - NANOARROW_TYPE_DECIMAL256, 38, 8, kDecimalData, - {"-123.456", "0.00001234", "1", "123.456", "1000000", - "23423942305922323.49023094"} - }}; + {NANOARROW_TYPE_DECIMAL256, + 38, + 8, + kDecimalData, + {"-123.456", "0.00001234", "1", "123.456", "1000000", + "23423942305922323.49023094"}}}; static std::initializer_list kDecimal256NoScaleCases = { - { - NANOARROW_TYPE_DECIMAL256, 38, 0, kDecimalData, - {"-12345600000", "1234", "100000000", "12345600000", "100000000000000", - "2342394230592232349023094"} - }}; + {NANOARROW_TYPE_DECIMAL256, + 38, + 0, + kDecimalData, + {"-12345600000", "1234", "100000000", "12345600000", "100000000000000", + "2342394230592232349023094"}}}; static std::initializer_list kDecimal256LargeCases = { - { - NANOARROW_TYPE_DECIMAL256, 76, 8, kDecimal256Data, - { - "-12345678901234567890123456789012345678901234567890123456789012345678.90123456", - "12345678901234567890123456789012345678901234567890123456789012345678.90123456", - } - }}; + {NANOARROW_TYPE_DECIMAL256, + 76, + 8, + kDecimal256Data, + { + "-12345678901234567890123456789012345678901234567890123456789012345678.90123456", + "12345678901234567890123456789012345678901234567890123456789012345678.90123456", + }}}; static std::initializer_list kDecimal256LargeNoScaleCases = { - { - NANOARROW_TYPE_DECIMAL256, 76, 0, kDecimal256Data, - { - "-1234567890123456789012345678901234567890123456789012345678901234567890123456", - "1234567890123456789012345678901234567890123456789012345678901234567890123456", - } - }}; + {NANOARROW_TYPE_DECIMAL256, + 76, + 0, + kDecimal256Data, + { + "-1234567890123456789012345678901234567890123456789012345678901234567890123456", + "1234567890123456789012345678901234567890123456789012345678901234567890123456", + }}}; INSTANTIATE_TEST_SUITE_P(Decimal128Tests, PostgresDecimalTest, testing::ValuesIn(kDecimal128Cases)); diff --git a/c/driver/postgresql/statement.cc b/c/driver/postgresql/statement.cc index 601716b163..4a86a2893c 100644 --- a/c/driver/postgresql/statement.cc +++ b/c/driver/postgresql/statement.cc @@ -38,8 +38,8 @@ #include "common/options.h" #include "common/utils.h" #include "connection.h" -#include "error.h" #include "copy/writer.h" +#include "error.h" #include "postgres_type.h" #include "postgres_util.h" #include "result_helper.h" diff --git a/c/validation/adbc_validation.cc b/c/validation/adbc_validation.cc index 97d12be169..aec945e696 100644 --- a/c/validation/adbc_validation.cc +++ b/c/validation/adbc_validation.cc @@ -3629,14 +3629,15 @@ void StatementTest::TestSqlQueryRowsAffectedDelete() { IsOkStatus(&error)); ASSERT_THAT(AdbcStatementNew(&connection, &statement, &error), IsOkStatus(&error)); - ASSERT_THAT(AdbcStatementSetSqlQuery(&statement, - "CREATE TABLE delete_test (foo INT)", &error), - IsOkStatus(&error)); + ASSERT_THAT( + AdbcStatementSetSqlQuery(&statement, "CREATE TABLE delete_test (foo INT)", &error), + IsOkStatus(&error)); ASSERT_THAT(AdbcStatementExecuteQuery(&statement, nullptr, nullptr, &error), IsOkStatus(&error)); - ASSERT_THAT(AdbcStatementSetSqlQuery(&statement, - "INSERT INTO delete_test (foo) VALUES (1), (2), (3), (4), (5)", &error), + ASSERT_THAT(AdbcStatementSetSqlQuery( + &statement, + "INSERT INTO delete_test (foo) VALUES (1), (2), (3), (4), (5)", &error), IsOkStatus(&error)); ASSERT_THAT(AdbcStatementExecuteQuery(&statement, nullptr, nullptr, &error), IsOkStatus(&error)); @@ -3648,8 +3649,7 @@ void StatementTest::TestSqlQueryRowsAffectedDelete() { int64_t rows_affected = 0; ASSERT_THAT(AdbcStatementExecuteQuery(&statement, nullptr, &rows_affected, &error), IsOkStatus(&error)); - ASSERT_THAT(rows_affected, - ::testing::AnyOf(::testing::Eq(3), ::testing::Eq(-1))); + ASSERT_THAT(rows_affected, ::testing::AnyOf(::testing::Eq(3), ::testing::Eq(-1))); } void StatementTest::TestSqlQueryRowsAffectedDeleteStream() { @@ -3657,14 +3657,15 @@ void StatementTest::TestSqlQueryRowsAffectedDeleteStream() { IsOkStatus(&error)); ASSERT_THAT(AdbcStatementNew(&connection, &statement, &error), IsOkStatus(&error)); - ASSERT_THAT(AdbcStatementSetSqlQuery(&statement, - "CREATE TABLE delete_test (foo INT)", &error), - IsOkStatus(&error)); + ASSERT_THAT( + AdbcStatementSetSqlQuery(&statement, "CREATE TABLE delete_test (foo INT)", &error), + IsOkStatus(&error)); ASSERT_THAT(AdbcStatementExecuteQuery(&statement, nullptr, nullptr, &error), IsOkStatus(&error)); - ASSERT_THAT(AdbcStatementSetSqlQuery(&statement, - "INSERT INTO delete_test (foo) VALUES (1), (2), (3), (4), (5)", &error), + ASSERT_THAT(AdbcStatementSetSqlQuery( + &statement, + "INSERT INTO delete_test (foo) VALUES (1), (2), (3), (4), (5)", &error), IsOkStatus(&error)); ASSERT_THAT(AdbcStatementExecuteQuery(&statement, nullptr, nullptr, &error), IsOkStatus(&error)); @@ -3681,9 +3682,6 @@ void StatementTest::TestSqlQueryRowsAffectedDeleteStream() { ::testing::AnyOf(::testing::Eq(5), ::testing::Eq(-1))); } - - - void StatementTest::TestTransactions() { if (!quirks()->supports_transactions() || quirks()->ddl_implicit_commit_txn()) { GTEST_SKIP(); diff --git a/c/validation/adbc_validation.h b/c/validation/adbc_validation.h index 30a20491eb..fcb4a5c286 100644 --- a/c/validation/adbc_validation.h +++ b/c/validation/adbc_validation.h @@ -51,8 +51,7 @@ class DriverQuirks { } virtual AdbcStatusCode DropTable(struct AdbcConnection* connection, - const std::string& name, - const std::string& db_schema, + const std::string& name, const std::string& db_schema, struct AdbcError* error) const { return ADBC_STATUS_NOT_IMPLEMENTED; } @@ -77,8 +76,8 @@ class DriverQuirks { /// \brief Create a schema for testing. virtual AdbcStatusCode EnsureDbSchema(struct AdbcConnection* connection, - const std::string& name, - struct AdbcError* error) const { + const std::string& name, + struct AdbcError* error) const { return ADBC_STATUS_NOT_IMPLEMENTED; } @@ -489,7 +488,7 @@ class StatementTest { TEST_F(FIXTURE, SqlIngestTemporaryAppend) { TestSqlIngestTemporaryAppend(); } \ TEST_F(FIXTURE, SqlIngestTemporaryReplace) { TestSqlIngestTemporaryReplace(); } \ TEST_F(FIXTURE, SqlIngestTemporaryExclusive) { TestSqlIngestTemporaryExclusive(); } \ - TEST_F(FIXTURE, SqlIngestPrimaryKey) { TestSqlIngestPrimaryKey(); } \ + TEST_F(FIXTURE, SqlIngestPrimaryKey) { TestSqlIngestPrimaryKey(); } \ TEST_F(FIXTURE, SqlPartitionedInts) { TestSqlPartitionedInts(); } \ TEST_F(FIXTURE, SqlPrepareGetParameterSchema) { TestSqlPrepareGetParameterSchema(); } \ TEST_F(FIXTURE, SqlPrepareSelectNoParams) { TestSqlPrepareSelectNoParams(); } \ From 9becbef68f44e6e5884eddf60aa613eeb67e8def Mon Sep 17 00:00:00 2001 From: Sutou Kouhei Date: Sun, 14 Jan 2024 17:26:24 +0900 Subject: [PATCH 2/3] Use pre-commit/mirrors-clang-format --- .pre-commit-config.yaml | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/.pre-commit-config.yaml b/.pre-commit-config.yaml index c13ad36209..480ca074f5 100644 --- a/.pre-commit-config.yaml +++ b/.pre-commit-config.yaml @@ -39,11 +39,10 @@ repos: files: '\.(bat|sln)$' - id: trailing-whitespace exclude: "^r/.*?/_snaps/.*?.md$" - - repo: https://github.com/pocc/pre-commit-hooks - rev: v1.3.5 + - repo: https://github.com/pre-commit/mirrors-clang-format + rev: "v14.0.6" hooks: - id: clang-format - args: [-i] types_or: [c, c++] - repo: https://github.com/cheshirekow/cmake-format-precommit rev: v0.6.13 From 9c18e32f7c7220fc831783b03823f67298ca77df Mon Sep 17 00:00:00 2001 From: Sutou Kouhei Date: Sun, 14 Jan 2024 20:44:55 +0900 Subject: [PATCH 3/3] Use parenthesis for min() macro --- c/driver/postgresql/statement.cc | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/c/driver/postgresql/statement.cc b/c/driver/postgresql/statement.cc index 4a86a2893c..c599832904 100644 --- a/c/driver/postgresql/statement.cc +++ b/c/driver/postgresql/statement.cc @@ -476,7 +476,7 @@ struct BindStream { return ADBC_STATUS_INVALID_ARGUMENT; } - if (val < std::numeric_limits::min() + kPostgresTimestampEpoch) { + if (val < (std::numeric_limits::min)() + kPostgresTimestampEpoch) { SetError(error, "[libpq] Field #%" PRId64 " ('%s') Row #%" PRId64 " has value '%" PRIi64 "' which would underflow",