WillAyd commented on code in PR #683:
URL: https://github.com/apache/arrow-adbc/pull/683#discussion_r1192837538
##########
c/driver/postgresql/connection.cc:
##########
@@ -260,39 +353,40 @@ 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) {
+ auto iter = row.begin();
Review Comment:
Not sure how idiomatic this is; wanted to make the Row iterator random
access so you can pick and choose the records you want to extract, but this may
not be very well expressed
##########
c/driver/postgresql/connection.cc:
##########
@@ -35,19 +36,116 @@ 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);
+ }
+ class iterator {
+ const PqResultRow& outer_;
+ int col_num_ = 0;
+
+ public:
+ explicit iterator(const PqResultRow& outer, int col_num = 0)
+ : outer_(outer), col_num_(col_num) {}
+ iterator& operator++() {
+ col_num_++;
+ return *this;
+ }
+
+ iterator operator++(int) {
+ iterator retval = *this;
+ ++(*this);
+ return retval;
+ }
+
+ iterator operator+=(const int& n) {
+ col_num_ += n;
+ return *this;
+ }
+
+ iterator operator+(const int& n) {
+ iterator tmp(*this);
+ tmp += n;
+ return tmp;
+ }
+
+ PqRecord operator[](const int& n) { return *(*this + n); }
+
+ bool operator==(iterator other) const { return col_num_ == other.col_num_;
}
Review Comment:
Not sure if C++ would require an equality comparison of the containing class
as well. Excluded for now to cut down on verbosity but someone with better C++
knowledge likely knows more
--
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]
