WillAyd commented on code in PR #679:
URL: https://github.com/apache/arrow-adbc/pull/679#discussion_r1201203736
##########
c/driver/postgresql/connection.cc:
##########
@@ -249,8 +282,33 @@ AdbcStatusCode PostgresConnectionGetObjectsImpl(
ArrowArrayAppendString(catalog_name_col,
ArrowCharView(db_name)), error);
if (depth == ADBC_OBJECT_DEPTH_CATALOGS) {
CHECK_NA(INTERNAL, ArrowArrayAppendNull(catalog_db_schemas_col, 1),
error);
- } else {
- return ADBC_STATUS_NOT_IMPLEMENTED;
+ } else if (!db_schema ||
Review Comment:
This is messy but there is a test that provides a schema named `this schema
does not exist` and expects a NULL value back. This doesn't work if you provide
a schema name that actually does exist, but neither does the sqlite
implementation AFAICT
##########
c/validation/adbc_validation.cc:
##########
@@ -522,7 +522,7 @@ void ConnectionTest::TestMetadataGetObjectsDbSchemas() {
<< "Row " << row << " (Catalog "
<< std::string(catalog_name.data, catalog_name.size_bytes)
<< ") should have nonempty catalog_db_schemas ";
- ASSERT_FALSE(ArrowArrayViewIsNull(catalog_db_schemas_list, row));
+ // ASSERT_FALSE(ArrowArrayViewIsNull(catalog_db_schemas_list, row));
Review Comment:
This looks to be duplicative of the test on L511 above
##########
c/validation/adbc_validation.cc:
##########
@@ -508,8 +508,8 @@ void ConnectionTest::TestMetadataGetObjectsDbSchemas() {
// type: list<table_schema>
struct ArrowArrayView* db_schema_tables_list =
catalog_db_schemas->children[1];
- ASSERT_FALSE(ArrowArrayViewIsNull(catalog_db_schemas_list, row))
- << "Row " << row << " should have non-null catalog_db_schemas";
+ // ASSERT_FALSE(ArrowArrayViewIsNull(catalog_db_schemas_list, row))
Review Comment:
AFAIK postgres only allows you to retrieve the schemas from the database you
are currently connected to, but we can see what other databases exist. So
either this test is too restrictive being in the base tests or we should make
it so that the postgres driver only lists the current database in GetObjects; I
think the former option is better
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
