You are viewing a plain text version of this content. The canonical link for it is here.
Posted to github@arrow.apache.org by "lidavidm (via GitHub)" <gi...@apache.org> on 2023/05/15 20:25:50 UTC
[GitHub] [arrow-adbc] lidavidm commented on a diff in pull request #683: refactor(c/driver/postgres): Implement InputIterator for ResultHelper
lidavidm commented on code in PR #683:
URL: https://github.com/apache/arrow-adbc/pull/683#discussion_r1194323872
##########
c/driver/postgresql/connection.cc:
##########
@@ -170,24 +234,17 @@ AdbcStatusCode PostgresConnectionGetObjectsImpl(
PqResultHelper result_helper = PqResultHelper{conn, query.buffer};
StringBuilderReset(&query);
- pg_result* result = result_helper.Execute();
-
- ExecStatusType pq_status = PQresultStatus(result);
- if (pq_status == PGRES_TUPLES_OK) {
- int num_rows = PQntuples(result);
- for (int row = 0; row < num_rows; row++) {
- const char* db_name = PQgetvalue(result, row, 0);
- CHECK_NA(INTERNAL,
- 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;
- }
- CHECK_NA(INTERNAL, ArrowArrayFinishElement(array), error);
+
+ for (auto row : result_helper) {
Review Comment:
Where do we check the status now?
##########
c/driver/postgresql/connection.cc:
##########
@@ -170,24 +234,17 @@ AdbcStatusCode PostgresConnectionGetObjectsImpl(
PqResultHelper result_helper = PqResultHelper{conn, query.buffer};
StringBuilderReset(&query);
- pg_result* result = result_helper.Execute();
-
- ExecStatusType pq_status = PQresultStatus(result);
- if (pq_status == PGRES_TUPLES_OK) {
- int num_rows = PQntuples(result);
- for (int row = 0; row < num_rows; row++) {
- const char* db_name = PQgetvalue(result, row, 0);
- CHECK_NA(INTERNAL,
- 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;
- }
- CHECK_NA(INTERNAL, ArrowArrayFinishElement(array), error);
+
+ for (auto row : result_helper) {
Review Comment:
might be good to spell out the autos here
##########
c/driver/postgresql/connection.cc:
##########
@@ -19,8 +19,10 @@
#include <cinttypes>
#include <cstring>
+#include <iterator>
Review Comment:
Where do we use this header?
##########
c/driver/postgresql/connection.cc:
##########
@@ -35,20 +37,82 @@ static const uint32_t kSupportedInfoCodes[] = {
ADBC_INFO_DRIVER_VERSION, ADBC_INFO_DRIVER_ARROW_VERSION,
};
+struct PqRecord {
Review Comment:
I think it's worth documenting these classes at this point
##########
c/driver/postgresql/connection.cc:
##########
@@ -35,19 +37,77 @@ static const uint32_t kSupportedInfoCodes[] = {
ADBC_INFO_DRIVER_VERSION, ADBC_INFO_DRIVER_ARROW_VERSION,
};
+struct PqRecord {
+ const char* data;
+ const int len;
+ const bool is_null;
+};
+
+class PqResultRow {
+ public:
+ PqResultRow(pg_result* result, int row_num) : result_(result), row_num_(row_num) {
+ ncols_ = PQnfields(result);
+ }
+
+ PqRecord operator[](const int& col_num) {
+ const char* data = PQgetvalue(result_, row_num_, col_num);
Review Comment:
Maybe an assert?
--
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