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/09 20:23:00 UTC

[GitHub] [arrow-adbc] WillAyd opened a new pull request, #668: feat(c/driver/postgresql): Implement Postgres Get table types

WillAyd opened a new pull request, #668:
URL: https://github.com/apache/arrow-adbc/pull/668

   (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 pull request #668: feat(c/driver/postgresql): Implement Postgres Get table types

Posted by "WillAyd (via GitHub)" <gi...@apache.org>.
WillAyd commented on PR #668:
URL: https://github.com/apache/arrow-adbc/pull/668#issuecomment-1541018849

   I was hoping to add some more explicit testing in the base `adbc_validation.cc` implementation of the GetTableTypes test. Something like this:
   
   ```cc
     ...
   
     bool seen_table = false;
     bool seen_view = false;
     for (auto row = 0; row < reader.array->length; row++) {
       ArrowStringView val =
           ArrowArrayViewGetStringUnsafe(reader.array_view->children[0], row);
       auto str_val = std::string(val.data, val.size_bytes);
       if (str_val == "table")
         seen_table = true;
       else if (str_val == "view")
         seen_view = true;
     }
     ASSERT_TRUE(seen_table);
     ASSERT_TRUE(seen_view);
   ```
   
   However it looks like Dremio might not provide back these values(?)


-- 
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 pull request #668: feat(c/driver/postgresql): Implement Postgres Get table types

Posted by "lidavidm (via GitHub)" <gi...@apache.org>.
lidavidm commented on PR #668:
URL: https://github.com/apache/arrow-adbc/pull/668#issuecomment-1542097417

   Possibly case-sensitive?
   
   Or IIRC Dremio may not have implemented all of the metadata calls yet


-- 
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 #668: feat(c/driver/postgresql): Implement Postgres Get table types

Posted by "WillAyd (via GitHub)" <gi...@apache.org>.
WillAyd commented on code in PR #668:
URL: https://github.com/apache/arrow-adbc/pull/668#discussion_r1189096540


##########
c/validation/adbc_validation.cc:
##########
@@ -362,6 +362,20 @@ void ConnectionTest::TestMetadataGetTableTypes() {
   ASSERT_NO_FATAL_FAILURE(CompareSchema(
       &reader.schema.value, {{"table_type", NANOARROW_TYPE_STRING, NOT_NULL}}));
   ASSERT_NO_FATAL_FAILURE(reader.Next());
+
+  bool seen_table = false;

Review Comment:
   Pretty basic but figured worth validating values in the base test class. I think all databases should at least have the concept of tables and views?



-- 
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 #668: feat(c/driver/postgresql): Implement Postgres Get table types

Posted by "lidavidm (via GitHub)" <gi...@apache.org>.
lidavidm merged PR #668:
URL: https://github.com/apache/arrow-adbc/pull/668


-- 
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