[4276] Replace use of PGresult with PgSqlResult throughout

src/lib/dhcpsrv/pgsql_connection.cc

@@ -22,15 +22,15 @@
 // code is 5 decimal, ASCII, digits, the first two define the category of
 // error, the last three are the specific error.  PostgreSQL makes the state
 // code as a char[5].  Macros for each code are defined in PostgreSQL's
-// server/utils/errcodes.h, although they require a second macro, 
+// server/utils/errcodes.h, although they require a second macro,
 // MAKE_SQLSTATE for completion.  For example, duplicate key error as:
 //
 // #define ERRCODE_UNIQUE_VIOLATION MAKE_SQLSTATE('2','3','5','0','5')
-// 
-// PostgreSQL deliberately omits the MAKE_SQLSTATE macro so callers can/must 
+//
+// PostgreSQL deliberately omits the MAKE_SQLSTATE macro so callers can/must
 // supply their own.  We'll define it as an initlizer_list:
 #define MAKE_SQLSTATE(ch1,ch2,ch3,ch4,ch5) {ch1,ch2,ch3,ch4,ch5}
-// So we can use it like this: const char some_error[] = ERRCODE_xxxx; 
+// So we can use it like this: const char some_error[] = ERRCODE_xxxx;
 #define PGSQL_STATECODE_LEN 5
 #include <utils/errcodes.h>
 
@@ -101,7 +101,7 @@ PgSqlConnection::openDatabase() {
         isc_throw(NoDatabaseName, "must specify a name for the database");
     }
 
-    // Connect to Postgres, saving the low level connection pointer 
+    // Connect to Postgres, saving the low level connection pointer
     // in the holder object
     PGconn* new_conn = PQconnectdb(dbconnparameters.c_str());
     if (!new_conn) {
@@ -120,8 +120,8 @@ PgSqlConnection::openDatabase() {
     conn_.setConnection(new_conn);
 }
 
-bool 
-PgSqlConnection::compareError(PGresult*& r, const char* error_state) {
+bool
+PgSqlConnection::compareError(const PgSqlResult& r, const char* error_state) {
     const char* sqlstate = PQresultErrorField(r, PG_DIAG_SQLSTATE);
     // PostgreSQL garuantees it will always be 5 characters long
     return ((sqlstate != NULL) &&
@@ -129,7 +129,7 @@ PgSqlConnection::compareError(PGresult*& r, const char* error_state) {
 }
 
 void
-PgSqlConnection::checkStatementError(PGresult*& r, 
+PgSqlConnection::checkStatementError(const PgSqlResult& r,
                                      PgSqlTaggedStatement& statement) const {
     int s = PQresultStatus(r);
     if (s != PGRES_COMMAND_OK && s != PGRES_TUPLES_OK) {

src/lib/dhcpsrv/pgsql_connection.h

@@ -18,8 +18,9 @@ namespace isc {
 namespace dhcp {
 
 // Maximum number of parameters that can be used a statement
-// @todo This allows us to use an initializer list (since we don't
-// require C++11).  It's unlikely we'd go past in a single statement.
+// @todo This allows us to use an initializer list (since we can't
+// require C++11).  It's unlikely we'd go past this many a single
+// statement.
 const size_t PGSQL_MAX_PARAMETERS_IN_QUERY = 32;
 
 /// @brief  Defines a Postgresql SQL statement
@@ -95,7 +96,7 @@ public:
 
     /// @brief Conversion Operator
     ///
-    /// Allows the PgSqlResult object to be passed as the context argument to
+    /// Allows the PgSqlResult object to be passed as the result set argument to
     /// PQxxxx functions.
     operator PGresult*() const {
         return (result_);
@@ -108,13 +109,12 @@ public:
         return (result_);
     }
 
-
 private:
     PGresult*     result_;     ///< Result set to be freed
 };
 
 
-/// @brief PgSql Handle Holder
+/// @brief Postgresql connection handle Holder
 ///
 /// Small RAII object for safer initialization, will close the database
 /// connection upon destruction.  This means that if an exception is thrown
@@ -241,7 +241,7 @@ public:
     ///
     /// @return True if the result set's SQL state equals the error_state,
     /// false otherwise.
-    bool compareError(PGresult*& r, const char* error_state);
+    bool compareError(const PgSqlResult& r, const char* error_state);
 
     /// @brief Checks result of the r object
     ///
@@ -263,7 +263,8 @@ public:
     /// @param statement - tagged statement that was executed
     ///
     /// @throw isc::dhcp::DbOperationError Detailed PostgreSQL failure
-    void checkStatementError(PGresult*& r, PgSqlTaggedStatement& statement) const;
+    void checkStatementError(const PgSqlResult& r,
+                             PgSqlTaggedStatement& statement) const;
 
     /// @brief PgSql connection handle
     ///

src/lib/dhcpsrv/pgsql_exchange.cc

@@ -55,8 +55,8 @@ std::string PsqlBindArray::toText() {
             } else {
                 stream << "0x";
                 for (int i = 0; i < lengths_[i]; ++i) {
-                    stream << std::setfill('0') << std::setw(2) 
-                           << std::setbase(16) 
+                    stream << std::setfill('0') << std::setw(2)
+                           << std::setbase(16)
                            << static_cast<unsigned int>(data[i]);
                 }
                 stream << std::endl;
@@ -67,7 +67,7 @@ std::string PsqlBindArray::toText() {
     return (stream.str());
 }
 
-std::string 
+std::string
 PgSqlExchange::convertToDatabaseTime(const time_t input_time) {
     struct tm tinfo;
     char buffer[20];
@@ -77,9 +77,9 @@ PgSqlExchange::convertToDatabaseTime(const time_t input_time) {
 }
 
 std::string
-PgSqlExchange::convertToDatabaseTime(const time_t cltt, 
+PgSqlExchange::convertToDatabaseTime(const time_t cltt,
                                      const uint32_t valid_lifetime) {
-    // Calculate expiry time. Store it in the 64-bit value so as we can 
+    // Calculate expiry time. Store it in the 64-bit value so as we can
     // detect overflows.
     int64_t expire_time_64 = static_cast<int64_t>(cltt)
                              + static_cast<int64_t>(valid_lifetime);
@@ -96,7 +96,7 @@ PgSqlExchange::convertToDatabaseTime(const time_t cltt,
     return (convertToDatabaseTime(static_cast<time_t>(expire_time_64)));
 }
 
-time_t 
+time_t
 PgSqlExchange::convertFromDatabaseTime(const std::string& db_time_val) {
     // Convert string time value to time_t
     time_t new_time;
@@ -110,7 +110,7 @@ PgSqlExchange::convertFromDatabaseTime(const std::string& db_time_val) {
 }
 
 const char*
-PgSqlExchange::getRawColumnValue(PGresult*& r, const int row, 
+PgSqlExchange::getRawColumnValue(const PgSqlResult& r, const int row,
                                  const size_t col) const {
     const char* value = PQgetvalue(r, row, col);
     if (!value) {
@@ -120,9 +120,9 @@ PgSqlExchange::getRawColumnValue(PGresult*& r, const int row,
     return (value);
 }
 
-void 
-PgSqlExchange::getColumnValue(PGresult*& r, const int row, const size_t col,
-                              bool &value) const {
+void
+PgSqlExchange::getColumnValue(const PgSqlResult& r, const int row,
+                              const size_t col, bool &value) const {
     const char* data = getRawColumnValue(r, row, col);
     if (!strlen(data) || *data == 'f') {
         value = false;
@@ -135,9 +135,9 @@ PgSqlExchange::getColumnValue(PGresult*& r, const int row, const size_t col,
     }
 }
 
-void 
-PgSqlExchange::getColumnValue(PGresult*& r, const int row, const size_t col,
-                              uint32_t &value) const {
+void
+PgSqlExchange::getColumnValue(const PgSqlResult& r, const int row,
+                              const size_t col, uint32_t &value) const {
     const char* data = getRawColumnValue(r, row, col);
     try {
         value = boost::lexical_cast<uint32_t>(data);
@@ -148,9 +148,9 @@ PgSqlExchange::getColumnValue(PGresult*& r, const int row, const size_t col,
     }
 }
 
-void 
-PgSqlExchange::getColumnValue(PGresult*& r, const int row, const size_t col,
-                              int32_t &value) const {
+void
+PgSqlExchange::getColumnValue(const PgSqlResult& r, const int row,
+                              const size_t col, int32_t &value) const {
     const char* data = getRawColumnValue(r, row, col);
     try {
         value = boost::lexical_cast<int32_t>(data);
@@ -161,9 +161,9 @@ PgSqlExchange::getColumnValue(PGresult*& r, const int row, const size_t col,
     }
 }
 
-void 
-PgSqlExchange::getColumnValue(PGresult*& r, const int row, const size_t col,
-                              uint8_t &value) const {
+void
+PgSqlExchange::getColumnValue(const PgSqlResult& r, const int row,
+                              const size_t col, uint8_t &value) const {
     const char* data = getRawColumnValue(r, row, col);
     try {
         // lexically casting as uint8_t doesn't convert from char
@@ -176,9 +176,10 @@ PgSqlExchange::getColumnValue(PGresult*& r, const int row, const size_t col,
     }
 }
 
-void 
-PgSqlExchange::convertFromBytea(PGresult*& r, const int row, const size_t col,
-                                uint8_t* buffer, const size_t buffer_size,
+void
+PgSqlExchange::convertFromBytea(const PgSqlResult& r, const int row,
+                                const size_t col, uint8_t* buffer,
+                                const size_t buffer_size,
                                 size_t &bytes_converted) const {
     // Returns converted bytes in a dynamically allocated buffer, and
     // sets bytes_converted.

src/lib/dhcpsrv/pgsql_exchange.h

@@ -157,7 +157,7 @@ public:
     ///
     /// @return a const char* pointer to the column's raw data
     /// @throw  DbOperationError if the value cannot be fetched.
-    const char* getRawColumnValue(PGresult*& r, const int row,
+    const char* getRawColumnValue(const PgSqlResult& r, const int row,
                                   const size_t col) const;
 
     /// @brief Fetches boolean text ('t' or 'f') as a bool.
@@ -169,7 +169,7 @@ public:
     ///
     /// @throw  DbOperationError if the value cannot be fetched or is
     /// invalid.
-    void getColumnValue(PGresult*& r, const int row, const size_t col,
+    void getColumnValue(const PgSqlResult& r, const int row, const size_t col,
                         bool &value) const;
 
     /// @brief Fetches an integer text column as a uint32_t.
@@ -181,7 +181,7 @@ public:
     ///
     /// @throw  DbOperationError if the value cannot be fetched or is
     /// invalid.
-    void getColumnValue(PGresult*& r, const int row, const size_t col,
+    void getColumnValue(const PgSqlResult& r, const int row, const size_t col,
                         uint32_t &value) const;
 
     /// @brief Fetches an integer text column as a int32_t.
@@ -193,7 +193,7 @@ public:
     ///
     /// @throw  DbOperationError if the value cannot be fetched or is
     /// invalid.
-    void getColumnValue(PGresult*& r, const int row, const size_t col,
+    void getColumnValue(const PgSqlResult& r, const int row, const size_t col,
                         int32_t &value) const;
 
     /// @brief Fetches an integer text column as a uint8_t.
@@ -205,7 +205,7 @@ public:
     ///
     /// @throw  DbOperationError if the value cannot be fetched or is
     /// invalid.
-    void getColumnValue(PGresult*& r, const int row, const size_t col,
+    void getColumnValue(const PgSqlResult& r, const int row, const size_t col,
                         uint8_t &value) const;
 
     /// @brief Converts a column in a row in a result set to a binary bytes
@@ -224,7 +224,7 @@ public:
     ///
     /// @throw  DbOperationError if the value cannot be fetched or is
     /// invalid.
-    void convertFromBytea(PGresult*& r, const int row, const size_t col,
+    void convertFromBytea(const PgSqlResult& r, const int row, const size_t col,
                           uint8_t* buffer, const size_t buffer_size,
                           size_t &bytes_converted) const;
 

src/lib/dhcpsrv/pgsql_lease_mgr.cc

@@ -371,7 +371,7 @@ public:
     ///
     /// @return Lease4Ptr to the newly created Lease4 object
     /// @throw DbOperationError if the lease cannot be created.
-    Lease4Ptr convertFromDatabase(PGresult*& r, int row) {
+    Lease4Ptr convertFromDatabase(const PgSqlResult& r, int row) {
         try {
             getColumnValue(r, row, ADDRESS_COL, addr4_);
 
@@ -559,7 +559,7 @@ public:
     ///
     /// @return Lease6Ptr to the newly created Lease4 object
     /// @throw DbOperationError if the lease cannot be created.
-    Lease6Ptr convertFromDatabase(PGresult*& r, int row) {
+    Lease6Ptr convertFromDatabase(const PgSqlResult& r, int row) {
         try {
             isc::asiolink::IOAddress addr(getIPv6Value(r, row, ADDRESS_COL));
 
@@ -618,8 +618,8 @@ public:
     ///
     /// @throw  DbOperationError if the value cannot be fetched or is
     /// invalid.
-    void getLeaseTypeColumnValue(PGresult*& r, const int row, const size_t col,
-                        Lease6::Type& value) const {
+    void getLeaseTypeColumnValue(const PgSqlResult& r, const int row,
+                                 const size_t col, Lease6::Type& value) const {
         uint32_t raw_value = 0;
         getColumnValue(r, row , col, raw_value);
         switch (raw_value) {
@@ -644,7 +644,7 @@ public:
     /// @return isc::asiolink::IOAddress containing the IPv6 address.
     /// @throw  DbOperationError if the value cannot be fetched or is
     /// invalid.
-    isc::asiolink::IOAddress getIPv6Value(PGresult*& r, const int row,
+    isc::asiolink::IOAddress getIPv6Value(const PgSqlResult& r, const int row,
                                           const size_t col) const {
         const char* data = getRawColumnValue(r, row, col);
         try {
@@ -722,11 +722,11 @@ PgSqlLeaseMgr::getDBVersion() {
 bool
 PgSqlLeaseMgr::addLeaseCommon(StatementIndex stindex,
                               PsqlBindArray& bind_array) {
-    PGresult* r = PQexecPrepared(conn_, tagged_statements[stindex].name,
-                                  tagged_statements[stindex].nbparams,
-                                  &bind_array.values_[0],
-                                  &bind_array.lengths_[0],
-                                  &bind_array.formats_[0], 0);
+    PgSqlResult r(PQexecPrepared(conn_, tagged_statements[stindex].name,
+                                 tagged_statements[stindex].nbparams,
+                                 &bind_array.values_[0],
+                                 &bind_array.lengths_[0],
+                                 &bind_array.formats_[0], 0));
 
     int s = PQresultStatus(r);
 
@@ -735,15 +735,12 @@ PgSqlLeaseMgr::addLeaseCommon(StatementIndex stindex,
         // the case, we return false to indicate that the row was not added.
         // Otherwise we throw an exception.
         if (conn_.compareError(r, PgSqlConnection::DUPLICATE_KEY)) {
-            PQclear(r);
             return (false);
         }
 
         conn_.checkStatementError(r, tagged_statements[stindex]);
     }
 
-    PQclear(r);
-
     return (true);
 }
 
@@ -776,17 +773,16 @@ void PgSqlLeaseMgr::getLeaseCollection(StatementIndex stindex,
     LOG_DEBUG(dhcpsrv_logger, DHCPSRV_DBG_TRACE_DETAIL,
               DHCPSRV_PGSQL_GET_ADDR4).arg(tagged_statements[stindex].name);
 
-    PGresult* r = PQexecPrepared(conn_, tagged_statements[stindex].name,
-                       tagged_statements[stindex].nbparams,
-                       &bind_array.values_[0],
-                       &bind_array.lengths_[0],
-                       &bind_array.formats_[0], 0);
+    PgSqlResult r(PQexecPrepared(conn_, tagged_statements[stindex].name,
+                                 tagged_statements[stindex].nbparams,
+                                 &bind_array.values_[0],
+                                 &bind_array.lengths_[0],
+                                 &bind_array.formats_[0], 0));
 
     conn_.checkStatementError(r, tagged_statements[stindex]);
 
     int rows = PQntuples(r);
     if (single && rows > 1) {
-        PQclear(r);
         isc_throw(MultipleRecords, "multiple records were found in the "
                       "database where only one was expected for query "
                       << tagged_statements[stindex].name);
@@ -795,8 +791,6 @@ void PgSqlLeaseMgr::getLeaseCollection(StatementIndex stindex,
     for(int i = 0; i < rows; ++ i) {
         result.push_back(exchange->convertFromDatabase(r, i));
     }
-
-    PQclear(r);
 }
 
 
@@ -1094,16 +1088,15 @@ PgSqlLeaseMgr::updateLeaseCommon(StatementIndex stindex,
     LOG_DEBUG(dhcpsrv_logger, DHCPSRV_DBG_TRACE_DETAIL,
               DHCPSRV_PGSQL_ADD_ADDR4).arg(tagged_statements[stindex].name);
 
-    PGresult* r = PQexecPrepared(conn_, tagged_statements[stindex].name,
-                                  tagged_statements[stindex].nbparams,
-                                  &bind_array.values_[0],
-                                  &bind_array.lengths_[0],
-                                  &bind_array.formats_[0], 0);
+    PgSqlResult r(PQexecPrepared(conn_, tagged_statements[stindex].name,
+                                 tagged_statements[stindex].nbparams,
+                                 &bind_array.values_[0],
+                                 &bind_array.lengths_[0],
+                                 &bind_array.formats_[0], 0));
 
     conn_.checkStatementError(r, tagged_statements[stindex]);
 
     int affected_rows = boost::lexical_cast<int>(PQcmdTuples(r));
-    PQclear(r);
 
     // Check success case first as it is the most likely outcome.
     if (affected_rows == 1) {
@@ -1165,15 +1158,14 @@ PgSqlLeaseMgr::updateLease6(const Lease6Ptr& lease) {
 uint64_t
 PgSqlLeaseMgr::deleteLeaseCommon(StatementIndex stindex,
                                  PsqlBindArray& bind_array) {
-    PGresult* r = PQexecPrepared(conn_, tagged_statements[stindex].name,
-                                  tagged_statements[stindex].nbparams,
-                                  &bind_array.values_[0],
-                                  &bind_array.lengths_[0],
-                                  &bind_array.formats_[0], 0);
+    PgSqlResult r(PQexecPrepared(conn_, tagged_statements[stindex].name,
+                                 tagged_statements[stindex].nbparams,
+                                 &bind_array.values_[0],
+                                 &bind_array.lengths_[0],
+                                 &bind_array.formats_[0], 0));
 
     conn_.checkStatementError(r, tagged_statements[stindex]);
     int affected_rows = boost::lexical_cast<int>(PQcmdTuples(r));
-    PQclear(r);
 
     return (affected_rows);
 }
@@ -1253,7 +1245,7 @@ PgSqlLeaseMgr::getVersion() const {
     LOG_DEBUG(dhcpsrv_logger, DHCPSRV_DBG_TRACE_DETAIL,
               DHCPSRV_PGSQL_GET_VERSION);
 
-    PGresult* r = PQexecPrepared(conn_, "get_version", 0, NULL, NULL, NULL, 0);
+    PgSqlResult r(PQexecPrepared(conn_, "get_version", 0, NULL, NULL, NULL, 0));
     conn_.checkStatementError(r, tagged_statements[GET_VERSION]);
 
     istringstream tmp;
@@ -1267,8 +1259,6 @@ PgSqlLeaseMgr::getVersion() const {
     tmp.str(PQgetvalue(r, 0, 1));
     tmp >> minor;
 
-    PQclear(r);
-
     return make_pair<uint32_t, uint32_t>(version, minor);
 }
 

src/lib/dhcpsrv/pgsql_lease_mgr.h

@@ -1,4 +1,4 @@
-// Copyright (C) 2013-2015 Internet Systems Consortium, Inc. ("ISC")
+// Copyright (C) 2013-2016 Internet Systems Consortium, Inc. ("ISC")
 //
 // This Source Code Form is subject to the terms of the Mozilla Public
 // License, v. 2.0. If a copy of the MPL was not distributed with this
@@ -473,28 +473,6 @@ private:
         getLeaseCollection(stindex, bind_array, exchange6_, result);
     }
 
-    /// @brief Checks result of the r object
-    ///
-    /// This function is used to determine whether or not the SQL statement
-    /// execution succeeded, and in the event of failures, decide whether or
-    /// not the failures are recoverable.
-    ///
-    /// If the error is recoverable, the method will throw a DbOperationError.
-    /// In the error is deemed unrecoverable, such as a loss of connectivity
-    /// with the server, this method will log the error and call exit(-1);
-    ///
-    /// @todo Calling exit() is viewed as a short term solution for Kea 1.0.
-    /// Two tickets are likely to alter this behavior, first is #3639, which
-    /// calls for the ability to attempt to reconnect to the database. The
-    /// second ticket, #4087 which calls for the implementation of a generic,
-    /// FatalException class which will propagate outward.
-    ///
-    /// @param r result of the last PostgreSQL operation
-    /// @param index will be used to print out compiled statement name
-    ///
-    /// @throw isc::dhcp::DbOperationError Detailed PostgreSQL failure
-    void checkStatementError(PGresult*& r, StatementIndex index) const;
-
     /// @brief Get Lease4 Common Code
     ///
     /// This method performs the common actions for the various getLease4()
@@ -571,7 +549,8 @@ private:
     ///
     /// @throw isc::dhcp::DbOperationError An operation on the open database has
     ///        failed.
-    uint64_t deleteLeaseCommon(StatementIndex stindex, PsqlBindArray& bind_array);
+    uint64_t deleteLeaseCommon(StatementIndex stindex,
+                               PsqlBindArray& bind_array);
 
     /// @brief Delete expired-reclaimed leases.
     ///