WillAyd commented on code in PR #683:
URL: https://github.com/apache/arrow-adbc/pull/683#discussion_r1195673238


##########
c/driver/postgresql/connection.cc:
##########
@@ -260,39 +328,39 @@ AdbcStatusCode PostgresConnection::GetTableSchema(const 
char* catalog,
 
   PqResultHelper result_helper = PqResultHelper{conn_, query.buffer};
   StringBuilderReset(&query);
-  pg_result* result = result_helper.Execute();
-
-  ExecStatusType pq_status = PQresultStatus(result);
-  auto uschema = nanoarrow::UniqueSchema();
 
-  if (pq_status == PGRES_TUPLES_OK) {
-    int num_rows = PQntuples(result);
+  if (result_helper.Status() != PGRES_TUPLES_OK) {
+    SetError(error, "%s%s", "Failed to get table schema: ", 
PQerrorMessage(conn_));
+    final_status = ADBC_STATUS_IO;
+  } else {
+    auto uschema = nanoarrow::UniqueSchema();
     ArrowSchemaInit(uschema.get());
-    CHECK_NA(INTERNAL, ArrowSchemaSetTypeStruct(uschema.get(), num_rows), 
error);
+    CHECK_NA(INTERNAL, ArrowSchemaSetTypeStruct(uschema.get(), 
result_helper.NumRows()),
+             error);
 
     ArrowError na_error;
-    for (int row = 0; row < num_rows; row++) {
-      const char* colname = PQgetvalue(result, row, 0);
+    int row_counter = 0;
+    for (auto row : result_helper) {
+      const char* colname = row[0].data;
       const Oid pg_oid = static_cast<uint32_t>(
-          std::strtol(PQgetvalue(result, row, 1), /*str_end=*/nullptr, 
/*base=*/10));
+          std::strtol(row[1].data, /*str_end=*/nullptr, /*base=*/10));
 
       PostgresType pg_type;
       if (type_resolver_->Find(pg_oid, &pg_type, &na_error) != NANOARROW_OK) {
-        SetError(error, "%s%d%s%s%s%" PRIu32, "Column #", row + 1, " (\"", 
colname,
-                 "\") has unknown type code ", pg_oid);
+        SetError(error, "%s%d%s%s%s%" PRIu32, "Column #", row_counter + 1, " 
(\"",

Review Comment:
   Each row here represents a column in a table:
   
   ```sh
   column_name | type
   ------------|------
   column_a    | INTEGER
   column_b    | VARCHAR
   column_c    | UNKNOWN
   ```
   
   So while the code internally recognizes these as rows from an error 
reporting perspective it might make more sense for the user to know that the 
3rd column of the table they are querying has an unknown type



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

Reply via email to