You are viewing a plain text version of this content. The canonical link for it is here.
Posted to github@arrow.apache.org by "WillAyd (via GitHub)" <gi...@apache.org> on 2023/05/05 22:17:52 UTC
[GitHub] [arrow-adbc] WillAyd opened a new pull request, #658: feat(c/driver/postgresql): Implement Postgres GetInfo
WillAyd opened a new pull request, #658:
URL: https://github.com/apache/arrow-adbc/pull/658
(no comment)
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow-adbc] WillAyd commented on a diff in pull request #658: feat(c/driver/postgresql): Implement Postgres GetInfo
Posted by "WillAyd (via GitHub)" <gi...@apache.org>.
WillAyd commented on code in PR #658:
URL: https://github.com/apache/arrow-adbc/pull/658#discussion_r1187911745
##########
c/driver/postgresql/postgresql_test.cc:
##########
@@ -143,10 +143,17 @@ TEST_F(PostgresConnectionTest, GetInfoMetadata) {
EXPECT_EQ("PostgreSQL", std::string(val.data, val.size_bytes));
break;
}
- case ADBC_INFO_VENDOR_VERSION:
- // UTF8
- ASSERT_EQ(uint8_t(0),
- reader.array_view->children[1]->buffer_views[0].data.as_uint8[row]);
+ case ADBC_INFO_VENDOR_VERSION: {
+ ArrowStringView val = ArrowArrayViewGetStringUnsafe(str_child, 3);
+#ifdef __WIN32
Review Comment:
This is unfortunate but I don't think there is a cross platform way for this to work https://github.com/google/googletest/issues/3084
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow-adbc] paleolimbot commented on a diff in pull request #658: feat(c/driver/postgresql): Implement Postgres GetInfo
Posted by "paleolimbot (via GitHub)" <gi...@apache.org>.
paleolimbot commented on code in PR #658:
URL: https://github.com/apache/arrow-adbc/pull/658#discussion_r1186871397
##########
c/driver/postgresql/postgresql_test.cc:
##########
@@ -97,6 +96,63 @@ class PostgresConnectionTest : public ::testing::Test,
protected:
PostgresQuirks quirks_;
};
+
+TEST_F(PostgresConnectionTest, GetInfoMetadata) {
+ ASSERT_THAT(AdbcConnectionNew(&connection, &error), IsOkStatus(&error));
+ ASSERT_THAT(AdbcConnectionInit(&connection, &database, &error), IsOkStatus(&error));
+
+ adbc_validation::StreamReader reader;
+ std::vector<uint32_t> info = {
+ ADBC_INFO_DRIVER_NAME,
+ ADBC_INFO_DRIVER_VERSION,
+ ADBC_INFO_VENDOR_NAME,
+ ADBC_INFO_VENDOR_VERSION,
+ };
+ ASSERT_THAT(AdbcConnectionGetInfo(&connection, info.data(), info.size(),
+ &reader.stream.value, &error),
+ IsOkStatus(&error));
+ ASSERT_NO_FATAL_FAILURE(reader.GetSchema());
+
+ std::vector<uint32_t> seen;
+ while (true) {
+ ASSERT_NO_FATAL_FAILURE(reader.Next());
+ if (!reader.array->release) break;
+
+ for (int64_t row = 0; row < reader.array->length; row++) {
+ ASSERT_FALSE(ArrowArrayViewIsNull(reader.array_view->children[0], row));
+ const uint32_t code =
+ reader.array_view->children[0]->buffer_views[1].data.as_uint32[row];
+ seen.push_back(code);
+
+ switch (code) {
+ case ADBC_INFO_DRIVER_NAME: {
+ const char* expected = "ADBC PostgreSQL Driver";
Review Comment:
If I were writing this test, I would probably start with `reader.array_view->children[1]->children[0]->buffer_views[2].data.as_char, len);` too. The downside of that is that it will crash instead of give a nice error if it's not true and that it's not too readable. Maybe:
```
int child_index = 1; // Always true?
struct ArrowArrayView* child = array_view.children[child_index];
ArrowStringView val = ArrowArrayViewGetStringUnsafe(child, 0);
EXPECT_EQ("ADBC PostgreSQL Driver", std::string(val.data, val.size_bytes));"
```
is somewhere in between?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow-adbc] WillAyd commented on a diff in pull request #658: feat(c/driver/postgresql): Implement Postgres GetInfo
Posted by "WillAyd (via GitHub)" <gi...@apache.org>.
WillAyd commented on code in PR #658:
URL: https://github.com/apache/arrow-adbc/pull/658#discussion_r1186534330
##########
c/driver/postgresql/connection.cc:
##########
@@ -68,6 +75,76 @@ AdbcStatusCode PostgresConnection::Commit(struct AdbcError* error) {
return ADBC_STATUS_OK;
}
+AdbcStatusCode PostgresConnectionGetInfoImpl(const uint32_t* info_codes,
+ size_t info_codes_length,
+ struct ArrowSchema* schema,
+ struct ArrowArray* array,
+ struct AdbcError* error) {
+ RAISE_ADBC(AdbcInitConnectionGetInfoSchema(info_codes, info_codes_length, schema, array,
+ error));
+
+ for (size_t i = 0; i < info_codes_length; i++) {
+ switch (info_codes[i]) {
+ case ADBC_INFO_VENDOR_NAME:
+ RAISE_ADBC(
+ AdbcConnectionGetInfoAppendString(array, info_codes[i], "PostgreSQL", error));
+ break;
+ case ADBC_INFO_VENDOR_VERSION:
+ RAISE_ADBC(AdbcConnectionGetInfoAppendString(
+ array, info_codes[i], std::to_string(PQlibVersion()).c_str(), error));
+ break;
+ case ADBC_INFO_DRIVER_NAME:
+ RAISE_ADBC(AdbcConnectionGetInfoAppendString(array, info_codes[i],
+ "ADBC PostgreSQL Driver", error));
+ break;
+ case ADBC_INFO_DRIVER_VERSION:
+ // TODO(lidavidm): fill in driver version
Review Comment:
I'm guessing this is something we can place in utils.c whenever read and just have all the drivers pull from it. Maybe even pre-populate it in the utils.c `AdbcInitConnectionGetInfoSchema` function
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow-adbc] WillAyd commented on a diff in pull request #658: feat(c/driver/postgresql): Implement Postgres GetInfo
Posted by "WillAyd (via GitHub)" <gi...@apache.org>.
WillAyd commented on code in PR #658:
URL: https://github.com/apache/arrow-adbc/pull/658#discussion_r1187666168
##########
c/driver/postgresql/postgresql_test.cc:
##########
@@ -97,6 +96,63 @@ class PostgresConnectionTest : public ::testing::Test,
protected:
PostgresQuirks quirks_;
};
+
+TEST_F(PostgresConnectionTest, GetInfoMetadata) {
+ ASSERT_THAT(AdbcConnectionNew(&connection, &error), IsOkStatus(&error));
+ ASSERT_THAT(AdbcConnectionInit(&connection, &database, &error), IsOkStatus(&error));
+
+ adbc_validation::StreamReader reader;
+ std::vector<uint32_t> info = {
+ ADBC_INFO_DRIVER_NAME,
+ ADBC_INFO_DRIVER_VERSION,
+ ADBC_INFO_VENDOR_NAME,
+ ADBC_INFO_VENDOR_VERSION,
+ };
+ ASSERT_THAT(AdbcConnectionGetInfo(&connection, info.data(), info.size(),
+ &reader.stream.value, &error),
+ IsOkStatus(&error));
+ ASSERT_NO_FATAL_FAILURE(reader.GetSchema());
+
+ std::vector<uint32_t> seen;
+ while (true) {
+ ASSERT_NO_FATAL_FAILURE(reader.Next());
+ if (!reader.array->release) break;
+
+ for (int64_t row = 0; row < reader.array->length; row++) {
+ ASSERT_FALSE(ArrowArrayViewIsNull(reader.array_view->children[0], row));
+ const uint32_t code =
+ reader.array_view->children[0]->buffer_views[1].data.as_uint32[row];
+ seen.push_back(code);
+
+ switch (code) {
+ case ADBC_INFO_DRIVER_NAME: {
+ const char* expected = "ADBC PostgreSQL Driver";
Review Comment:
Nice thanks for the tip - that definitely helps simplify things
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow-adbc] lidavidm commented on a diff in pull request #658: feat(c/driver/postgresql): Implement Postgres GetInfo
Posted by "lidavidm (via GitHub)" <gi...@apache.org>.
lidavidm commented on code in PR #658:
URL: https://github.com/apache/arrow-adbc/pull/658#discussion_r1188545614
##########
c/validation/adbc_validation.cc:
##########
@@ -318,9 +318,6 @@ void ConnectionTest::TestMetadataGetInfo() {
case ADBC_INFO_DRIVER_VERSION:
case ADBC_INFO_VENDOR_NAME:
case ADBC_INFO_VENDOR_VERSION:
- // UTF8
- ASSERT_EQ(uint8_t(0),
- reader.array_view->children[1]->buffer_views[0].data.as_uint8[row]);
Review Comment:
Ah, I think I remember what this is now. It's checking the union type code. We should keep this, then...and probably explain it more clearly.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow-adbc] WillAyd commented on a diff in pull request #658: feat(c/driver/postgresql): Implement Postgres GetInfo
Posted by "WillAyd (via GitHub)" <gi...@apache.org>.
WillAyd commented on code in PR #658:
URL: https://github.com/apache/arrow-adbc/pull/658#discussion_r1186534330
##########
c/driver/postgresql/connection.cc:
##########
@@ -68,6 +75,76 @@ AdbcStatusCode PostgresConnection::Commit(struct AdbcError* error) {
return ADBC_STATUS_OK;
}
+AdbcStatusCode PostgresConnectionGetInfoImpl(const uint32_t* info_codes,
+ size_t info_codes_length,
+ struct ArrowSchema* schema,
+ struct ArrowArray* array,
+ struct AdbcError* error) {
+ RAISE_ADBC(AdbcInitConnectionGetInfoSchema(info_codes, info_codes_length, schema, array,
+ error));
+
+ for (size_t i = 0; i < info_codes_length; i++) {
+ switch (info_codes[i]) {
+ case ADBC_INFO_VENDOR_NAME:
+ RAISE_ADBC(
+ AdbcConnectionGetInfoAppendString(array, info_codes[i], "PostgreSQL", error));
+ break;
+ case ADBC_INFO_VENDOR_VERSION:
+ RAISE_ADBC(AdbcConnectionGetInfoAppendString(
+ array, info_codes[i], std::to_string(PQlibVersion()).c_str(), error));
+ break;
+ case ADBC_INFO_DRIVER_NAME:
+ RAISE_ADBC(AdbcConnectionGetInfoAppendString(array, info_codes[i],
+ "ADBC PostgreSQL Driver", error));
+ break;
+ case ADBC_INFO_DRIVER_VERSION:
+ // TODO(lidavidm): fill in driver version
Review Comment:
I'm guessing this is something we can place in utils.c and just have all the drivers pull from it. Maybe even pre-populate it in the utils.c `AdbcInitConnectionGetInfoSchema` function
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow-adbc] WillAyd commented on a diff in pull request #658: feat(c/driver/postgresql): Implement Postgres GetInfo
Posted by "WillAyd (via GitHub)" <gi...@apache.org>.
WillAyd commented on code in PR #658:
URL: https://github.com/apache/arrow-adbc/pull/658#discussion_r1186726173
##########
c/driver/postgresql/postgresql_test.cc:
##########
@@ -97,6 +96,63 @@ class PostgresConnectionTest : public ::testing::Test,
protected:
PostgresQuirks quirks_;
};
+
+TEST_F(PostgresConnectionTest, GetInfoMetadata) {
+ ASSERT_THAT(AdbcConnectionNew(&connection, &error), IsOkStatus(&error));
+ ASSERT_THAT(AdbcConnectionInit(&connection, &database, &error), IsOkStatus(&error));
+
+ adbc_validation::StreamReader reader;
+ std::vector<uint32_t> info = {
+ ADBC_INFO_DRIVER_NAME,
+ ADBC_INFO_DRIVER_VERSION,
+ ADBC_INFO_VENDOR_NAME,
+ ADBC_INFO_VENDOR_VERSION,
+ };
+ ASSERT_THAT(AdbcConnectionGetInfo(&connection, info.data(), info.size(),
+ &reader.stream.value, &error),
+ IsOkStatus(&error));
+ ASSERT_NO_FATAL_FAILURE(reader.GetSchema());
+
+ std::vector<uint32_t> seen;
+ while (true) {
+ ASSERT_NO_FATAL_FAILURE(reader.Next());
+ if (!reader.array->release) break;
+
+ for (int64_t row = 0; row < reader.array->length; row++) {
+ ASSERT_FALSE(ArrowArrayViewIsNull(reader.array_view->children[0], row));
+ const uint32_t code =
+ reader.array_view->children[0]->buffer_views[1].data.as_uint32[row];
+ seen.push_back(code);
+
+ switch (code) {
+ case ADBC_INFO_DRIVER_NAME: {
+ const char* expected = "ADBC PostgreSQL Driver";
Review Comment:
I see some of the great work @paleolimbot did in https://github.com/apache/arrow-nanoarrow/pull/83 but not sure that helps us here?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow-adbc] lidavidm commented on a diff in pull request #658: feat(c/driver/postgresql): Implement Postgres GetInfo
Posted by "lidavidm (via GitHub)" <gi...@apache.org>.
lidavidm commented on code in PR #658:
URL: https://github.com/apache/arrow-adbc/pull/658#discussion_r1187495974
##########
c/driver/postgresql/connection.cc:
##########
@@ -68,6 +75,76 @@ AdbcStatusCode PostgresConnection::Commit(struct AdbcError* error) {
return ADBC_STATUS_OK;
}
+AdbcStatusCode PostgresConnectionGetInfoImpl(const uint32_t* info_codes,
+ size_t info_codes_length,
+ struct ArrowSchema* schema,
+ struct ArrowArray* array,
+ struct AdbcError* error) {
+ RAISE_ADBC(AdbcInitConnectionGetInfoSchema(info_codes, info_codes_length, schema, array,
+ error));
+
+ for (size_t i = 0; i < info_codes_length; i++) {
+ switch (info_codes[i]) {
+ case ADBC_INFO_VENDOR_NAME:
+ RAISE_ADBC(
+ AdbcConnectionGetInfoAppendString(array, info_codes[i], "PostgreSQL", error));
+ break;
+ case ADBC_INFO_VENDOR_VERSION:
+ RAISE_ADBC(AdbcConnectionGetInfoAppendString(
+ array, info_codes[i], std::to_string(PQlibVersion()).c_str(), error));
+ break;
+ case ADBC_INFO_DRIVER_NAME:
+ RAISE_ADBC(AdbcConnectionGetInfoAppendString(array, info_codes[i],
+ "ADBC PostgreSQL Driver", error));
+ break;
+ case ADBC_INFO_DRIVER_VERSION:
+ // TODO(lidavidm): fill in driver version
Review Comment:
Makes sense. We should also be able to leverage CMake to inject the version as a constant.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow-adbc] WillAyd commented on a diff in pull request #658: feat(c/driver/postgresql): Implement Postgres GetInfo
Posted by "WillAyd (via GitHub)" <gi...@apache.org>.
WillAyd commented on code in PR #658:
URL: https://github.com/apache/arrow-adbc/pull/658#discussion_r1187915216
##########
c/driver/sqlite/sqlite_test.cc:
##########
@@ -90,6 +90,68 @@ class SqliteConnectionTest : public ::testing::Test,
};
ADBCV_TEST_CONNECTION(SqliteConnectionTest)
+TEST_F(SqliteConnectionTest, GetInfoMetadata) {
+ ASSERT_THAT(AdbcConnectionNew(&connection, &error),
+ adbc_validation::IsOkStatus(&error));
+ ASSERT_THAT(AdbcConnectionInit(&connection, &database, &error),
+ adbc_validation::IsOkStatus(&error));
+
+ adbc_validation::StreamReader reader;
+ std::vector<uint32_t> info = {
+ ADBC_INFO_DRIVER_NAME,
+ ADBC_INFO_DRIVER_VERSION,
+ ADBC_INFO_VENDOR_NAME,
+ ADBC_INFO_VENDOR_VERSION,
+ };
+ ASSERT_THAT(AdbcConnectionGetInfo(&connection, info.data(), info.size(),
+ &reader.stream.value, &error),
+ adbc_validation::IsOkStatus(&error));
+ ASSERT_NO_FATAL_FAILURE(reader.GetSchema());
+
+ std::vector<uint32_t> seen;
+ while (true) {
+ ASSERT_NO_FATAL_FAILURE(reader.Next());
+ if (!reader.array->release) break;
+
+ for (int64_t row = 0; row < reader.array->length; row++) {
+ ASSERT_FALSE(ArrowArrayViewIsNull(reader.array_view->children[0], row));
+ const uint32_t code =
+ reader.array_view->children[0]->buffer_views[1].data.as_uint32[row];
+ seen.push_back(code);
+
+ int str_child_index = 0;
+ struct ArrowArrayView* str_child =
+ reader.array_view->children[1]->children[str_child_index];
+ switch (code) {
+ case ADBC_INFO_DRIVER_NAME: {
+ ArrowStringView val = ArrowArrayViewGetStringUnsafe(str_child, 0);
+ EXPECT_EQ("ADBC SQLite Driver", std::string(val.data, val.size_bytes));
+ break;
+ }
+ case ADBC_INFO_DRIVER_VERSION: {
+ ArrowStringView val = ArrowArrayViewGetStringUnsafe(str_child, 1);
+ EXPECT_EQ("(unknown)", std::string(val.data, val.size_bytes));
+ break;
+ }
+ case ADBC_INFO_VENDOR_NAME: {
+ ArrowStringView val = ArrowArrayViewGetStringUnsafe(str_child, 2);
+ EXPECT_EQ("SQLite", std::string(val.data, val.size_bytes));
+ break;
+ }
+ case ADBC_INFO_VENDOR_VERSION: {
+ ArrowStringView val = ArrowArrayViewGetStringUnsafe(str_child, 3);
+ EXPECT_THAT(std::string(val.data, val.size_bytes),
+ ::testing::MatchesRegex("3\\..*"));
Review Comment:
Ideally this could be something like `3\\.\d{1,2}\\.\d{1,2}` but didn't want to go down a rabbit hole given the cross platform regex differences
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow-adbc] WillAyd commented on a diff in pull request #658: feat(c/driver/postgresql): Implement Postgres GetInfo
Posted by "WillAyd (via GitHub)" <gi...@apache.org>.
WillAyd commented on code in PR #658:
URL: https://github.com/apache/arrow-adbc/pull/658#discussion_r1186725646
##########
c/driver/postgresql/postgresql_test.cc:
##########
@@ -97,6 +96,63 @@ class PostgresConnectionTest : public ::testing::Test,
protected:
PostgresQuirks quirks_;
};
+
+TEST_F(PostgresConnectionTest, GetInfoMetadata) {
+ ASSERT_THAT(AdbcConnectionNew(&connection, &error), IsOkStatus(&error));
+ ASSERT_THAT(AdbcConnectionInit(&connection, &database, &error), IsOkStatus(&error));
+
+ adbc_validation::StreamReader reader;
+ std::vector<uint32_t> info = {
+ ADBC_INFO_DRIVER_NAME,
+ ADBC_INFO_DRIVER_VERSION,
+ ADBC_INFO_VENDOR_NAME,
+ ADBC_INFO_VENDOR_VERSION,
+ };
+ ASSERT_THAT(AdbcConnectionGetInfo(&connection, info.data(), info.size(),
+ &reader.stream.value, &error),
+ IsOkStatus(&error));
+ ASSERT_NO_FATAL_FAILURE(reader.GetSchema());
+
+ std::vector<uint32_t> seen;
+ while (true) {
+ ASSERT_NO_FATAL_FAILURE(reader.Next());
+ if (!reader.array->release) break;
+
+ for (int64_t row = 0; row < reader.array->length; row++) {
+ ASSERT_FALSE(ArrowArrayViewIsNull(reader.array_view->children[0], row));
+ const uint32_t code =
+ reader.array_view->children[0]->buffer_views[1].data.as_uint32[row];
+ seen.push_back(code);
+
+ switch (code) {
+ case ADBC_INFO_DRIVER_NAME: {
+ const char* expected = "ADBC PostgreSQL Driver";
Review Comment:
This test is way too deep in the implementation details of the dense union, but I didn't see a better way of reading values from that union with nanoarrow. Is there something I am overlooking or would this need to be implemented upstream?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow-adbc] WillAyd commented on a diff in pull request #658: feat(c/driver/postgresql): Implement Postgres GetInfo
Posted by "WillAyd (via GitHub)" <gi...@apache.org>.
WillAyd commented on code in PR #658:
URL: https://github.com/apache/arrow-adbc/pull/658#discussion_r1187699181
##########
c/driver/postgresql/postgresql_test.cc:
##########
@@ -97,6 +96,66 @@ class PostgresConnectionTest : public ::testing::Test,
protected:
PostgresQuirks quirks_;
};
+
+TEST_F(PostgresConnectionTest, GetInfoMetadata) {
+ ASSERT_THAT(AdbcConnectionNew(&connection, &error), IsOkStatus(&error));
+ ASSERT_THAT(AdbcConnectionInit(&connection, &database, &error), IsOkStatus(&error));
+
+ adbc_validation::StreamReader reader;
+ std::vector<uint32_t> info = {
+ ADBC_INFO_DRIVER_NAME,
+ ADBC_INFO_DRIVER_VERSION,
+ ADBC_INFO_VENDOR_NAME,
+ ADBC_INFO_VENDOR_VERSION,
+ };
+ ASSERT_THAT(AdbcConnectionGetInfo(&connection, info.data(), info.size(),
+ &reader.stream.value, &error),
+ IsOkStatus(&error));
+ ASSERT_NO_FATAL_FAILURE(reader.GetSchema());
+
+ std::vector<uint32_t> seen;
+ while (true) {
+ ASSERT_NO_FATAL_FAILURE(reader.Next());
+ if (!reader.array->release) break;
+
+ for (int64_t row = 0; row < reader.array->length; row++) {
+ ASSERT_FALSE(ArrowArrayViewIsNull(reader.array_view->children[0], row));
+ const uint32_t code =
+ reader.array_view->children[0]->buffer_views[1].data.as_uint32[row];
+ seen.push_back(code);
+
+ int str_child_index = 0;
+ struct ArrowArrayView* str_child =
+ reader.array_view->children[1]->children[str_child_index];
+ switch (code) {
+ case ADBC_INFO_DRIVER_NAME: {
+ ArrowStringView val = ArrowArrayViewGetStringUnsafe(str_child, 0);
+ EXPECT_EQ("ADBC PostgreSQL Driver", std::string(val.data, val.size_bytes));
+ break;
+ }
+ case ADBC_INFO_DRIVER_VERSION: {
+ ArrowStringView val = ArrowArrayViewGetStringUnsafe(str_child, 1);
+ EXPECT_EQ("(unknown)", std::string(val.data, val.size_bytes));
+ break;
+ }
+ case ADBC_INFO_VENDOR_NAME: {
+ ArrowStringView val = ArrowArrayViewGetStringUnsafe(str_child, 2);
+ EXPECT_EQ("PostgreSQL", std::string(val.data, val.size_bytes));
+ break;
+ }
+ case ADBC_INFO_VENDOR_VERSION:
+ // UTF8
+ ASSERT_EQ(uint8_t(0),
+ reader.array_view->children[1]->buffer_views[0].data.as_uint8[row]);
Review Comment:
Let me check - this was copied from the adbc_validation test so didn't look too much into it
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow-adbc] WillAyd commented on a diff in pull request #658: feat(c/driver/postgresql): Implement Postgres GetInfo
Posted by "WillAyd (via GitHub)" <gi...@apache.org>.
WillAyd commented on code in PR #658:
URL: https://github.com/apache/arrow-adbc/pull/658#discussion_r1187674706
##########
c/driver/postgresql/connection.cc:
##########
@@ -68,6 +75,76 @@ AdbcStatusCode PostgresConnection::Commit(struct AdbcError* error) {
return ADBC_STATUS_OK;
}
+AdbcStatusCode PostgresConnectionGetInfoImpl(const uint32_t* info_codes,
+ size_t info_codes_length,
+ struct ArrowSchema* schema,
+ struct ArrowArray* array,
+ struct AdbcError* error) {
+ RAISE_ADBC(AdbcInitConnectionGetInfoSchema(info_codes, info_codes_length, schema, array,
+ error));
+
+ for (size_t i = 0; i < info_codes_length; i++) {
+ switch (info_codes[i]) {
+ case ADBC_INFO_VENDOR_NAME:
+ RAISE_ADBC(
+ AdbcConnectionGetInfoAppendString(array, info_codes[i], "PostgreSQL", error));
+ break;
+ case ADBC_INFO_VENDOR_VERSION:
+ RAISE_ADBC(AdbcConnectionGetInfoAppendString(
+ array, info_codes[i], std::to_string(PQlibVersion()).c_str(), error));
+ break;
+ case ADBC_INFO_DRIVER_NAME:
+ RAISE_ADBC(AdbcConnectionGetInfoAppendString(array, info_codes[i],
+ "ADBC PostgreSQL Driver", error));
+ break;
+ case ADBC_INFO_DRIVER_VERSION:
+ // TODO(lidavidm): fill in driver version
Review Comment:
OK cool. Gave this a look but keeping for a follow up - I think will require updating the CMake targets for the driver libraries and test libraries. Probably not too difficult just didn't want to cram to much into this PR
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow-adbc] lidavidm commented on a diff in pull request #658: feat(c/driver/postgresql): Implement Postgres GetInfo
Posted by "lidavidm (via GitHub)" <gi...@apache.org>.
lidavidm commented on code in PR #658:
URL: https://github.com/apache/arrow-adbc/pull/658#discussion_r1187692385
##########
c/driver/postgresql/postgresql_test.cc:
##########
@@ -97,6 +96,66 @@ class PostgresConnectionTest : public ::testing::Test,
protected:
PostgresQuirks quirks_;
};
+
+TEST_F(PostgresConnectionTest, GetInfoMetadata) {
+ ASSERT_THAT(AdbcConnectionNew(&connection, &error), IsOkStatus(&error));
+ ASSERT_THAT(AdbcConnectionInit(&connection, &database, &error), IsOkStatus(&error));
+
+ adbc_validation::StreamReader reader;
+ std::vector<uint32_t> info = {
+ ADBC_INFO_DRIVER_NAME,
+ ADBC_INFO_DRIVER_VERSION,
+ ADBC_INFO_VENDOR_NAME,
+ ADBC_INFO_VENDOR_VERSION,
+ };
+ ASSERT_THAT(AdbcConnectionGetInfo(&connection, info.data(), info.size(),
+ &reader.stream.value, &error),
+ IsOkStatus(&error));
+ ASSERT_NO_FATAL_FAILURE(reader.GetSchema());
+
+ std::vector<uint32_t> seen;
+ while (true) {
+ ASSERT_NO_FATAL_FAILURE(reader.Next());
+ if (!reader.array->release) break;
+
+ for (int64_t row = 0; row < reader.array->length; row++) {
+ ASSERT_FALSE(ArrowArrayViewIsNull(reader.array_view->children[0], row));
+ const uint32_t code =
+ reader.array_view->children[0]->buffer_views[1].data.as_uint32[row];
+ seen.push_back(code);
+
+ int str_child_index = 0;
+ struct ArrowArrayView* str_child =
+ reader.array_view->children[1]->children[str_child_index];
+ switch (code) {
+ case ADBC_INFO_DRIVER_NAME: {
+ ArrowStringView val = ArrowArrayViewGetStringUnsafe(str_child, 0);
+ EXPECT_EQ("ADBC PostgreSQL Driver", std::string(val.data, val.size_bytes));
+ break;
+ }
+ case ADBC_INFO_DRIVER_VERSION: {
+ ArrowStringView val = ArrowArrayViewGetStringUnsafe(str_child, 1);
+ EXPECT_EQ("(unknown)", std::string(val.data, val.size_bytes));
+ break;
+ }
+ case ADBC_INFO_VENDOR_NAME: {
+ ArrowStringView val = ArrowArrayViewGetStringUnsafe(str_child, 2);
+ EXPECT_EQ("PostgreSQL", std::string(val.data, val.size_bytes));
+ break;
+ }
+ case ADBC_INFO_VENDOR_VERSION:
+ // UTF8
+ ASSERT_EQ(uint8_t(0),
+ reader.array_view->children[1]->buffer_views[0].data.as_uint8[row]);
Review Comment:
What's going on here? This is a string too
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow-adbc] lidavidm merged pull request #658: feat(c/driver/postgresql): Implement Postgres GetInfo
Posted by "lidavidm (via GitHub)" <gi...@apache.org>.
lidavidm merged PR #658:
URL: https://github.com/apache/arrow-adbc/pull/658
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org