lidavidm commented on code in PR #714:
URL: https://github.com/apache/arrow-adbc/pull/714#discussion_r1210249221
##########
c/driver/postgresql/connection.cc:
##########
@@ -68,16 +69,54 @@ class PqResultRow {
// Helper to manager the lifecycle of a PQResult. The query argument
// will be evaluated as part of the constructor, with the desctructor handling
cleanup
-// Caller is responsible for calling the `Status()` method to ensure results
are
-// as expected prior to iterating
+// Caller must call Prepare then Execute, checking both for an OK
AdbcStatusCode
+// prior to iterating
class PqResultHelper {
public:
- PqResultHelper(PGconn* conn, const char* query) : conn_(conn) {
+ explicit PqResultHelper(PGconn* conn, const char* query, struct AdbcError*
error)
+ : conn_(conn), error_(error) {
query_ = std::string(query);
- result_ = PQexec(conn_, query_.c_str());
+ param_values_ = {};
Review Comment:
nit: technically unnecessary (it would get default-initialized)
##########
c/driver/postgresql/connection.cc:
##########
@@ -68,16 +69,54 @@ class PqResultRow {
// Helper to manager the lifecycle of a PQResult. The query argument
// will be evaluated as part of the constructor, with the desctructor handling
cleanup
-// Caller is responsible for calling the `Status()` method to ensure results
are
-// as expected prior to iterating
+// Caller must call Prepare then Execute, checking both for an OK
AdbcStatusCode
+// prior to iterating
class PqResultHelper {
public:
- PqResultHelper(PGconn* conn, const char* query) : conn_(conn) {
+ explicit PqResultHelper(PGconn* conn, const char* query, struct AdbcError*
error)
+ : conn_(conn), error_(error) {
query_ = std::string(query);
- result_ = PQexec(conn_, query_.c_str());
+ param_values_ = {};
}
- ExecStatusType Status() { return PQresultStatus(result_); }
+ explicit PqResultHelper(PGconn* conn, const char* query,
Review Comment:
I guess, why not just declare it as taking a `std::string` and then
initializing `query_` with a move?
##########
c/driver/postgresql/connection.cc:
##########
@@ -68,16 +69,54 @@ class PqResultRow {
// Helper to manager the lifecycle of a PQResult. The query argument
// will be evaluated as part of the constructor, with the desctructor handling
cleanup
-// Caller is responsible for calling the `Status()` method to ensure results
are
-// as expected prior to iterating
+// Caller must call Prepare then Execute, checking both for an OK
AdbcStatusCode
+// prior to iterating
class PqResultHelper {
public:
- PqResultHelper(PGconn* conn, const char* query) : conn_(conn) {
+ explicit PqResultHelper(PGconn* conn, const char* query, struct AdbcError*
error)
+ : conn_(conn), error_(error) {
query_ = std::string(query);
- result_ = PQexec(conn_, query_.c_str());
+ param_values_ = {};
}
- ExecStatusType Status() { return PQresultStatus(result_); }
+ explicit PqResultHelper(PGconn* conn, const char* query,
+ std::vector<std::string> param_values, struct
AdbcError* error)
+ : conn_(conn), param_values_(std::move(param_values)), error_(error) {
+ query_ = std::string(query);
Review Comment:
nit: prefer the initializer list (so `query_(query)` above)
--
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]
