[#2284] Fixed asserts in PostgresSQL support code

src/hooks/dhcp/pgsql_cb/pgsql_cb_dhcp4.cc
    Fixed an incorrect tagged statement

src/lib/pgsql/pgsql_connection.*
    PgSqlConnection::executePreparedStatement() - new convenience
    function to executing prepared statements

    PgSqlConnection::selectQuery()
    PgSqlConnection::insertQuery()
    PgSqlConnection::updateDeleteQuery()
    - now calls executePreparedStatement()

src/lib/pgsql/pgsql_exchange.cc
    PsqlBindArray::toText() - emits text for empty array

src/lib/pgsql/tests/pgsql_connection_unittest.cc
    TEST_F(PgSqlConnectionTest, executePreparedStatement) - new test

5 files changed, 128 insertions, 32 deletions

diff --git a/src/lib/pgsql/pgsql_connection.cc b/src/lib/pgsql/pgsql_connection.cc
index c958e78d10..c1efe81a8d 100644
--- a/src/lib/pgsql/pgsql_connection.cc
+++ b/src/lib/pgsql/pgsql_connection.cc
@@ -466,26 +466,49 @@ PgSqlConnection::executeSQL(const std::string& sql) {
     checkStatementError(r, statement);
 }
 
+PgSqlResultPtr
+PgSqlConnection::executePreparedStatement(PgSqlTaggedStatement& statement,
+                             const PsqlBindArray& in_bindings) {
+    checkUnusable();
+
+    if (statement.nbparams != in_bindings.size()) {
+        isc_throw (InvalidOperation, "executePreparedStatement:"
+                   << " expected: " << statement.nbparams
+                   << " parameters, given: " << in_bindings.size()
+                   << ", statement: " << statement.name
+                   << ", SQL: " << statement.text);
+    }
+
+    const char* const* values = 0;
+    const int * lengths = 0;
+    const int * formats = 0;
+    if (statement.nbparams > 0) {
+        values = static_cast<const char* const*>(&in_bindings.values_[0]);
+        lengths = static_cast<const int *>(&in_bindings.lengths_[0]);
+        formats = static_cast<const int *>(&in_bindings.formats_[0]);
+    }
+
+    PgSqlResultPtr result_set;
+    result_set.reset(new PgSqlResult(PQexecPrepared(conn_, statement.name, statement.nbparams,
+                                                    values, lengths, formats, 0)));
+
+    checkStatementError(*result_set, statement);
+    return(result_set);
+}
+
 void
 PgSqlConnection::selectQuery(PgSqlTaggedStatement& statement,
                              const PsqlBindArray& in_bindings,
                              ConsumeResultRowFun process_result_row) {
-    checkUnusable();
-
-    PgSqlResult result_set(PQexecPrepared(conn_, statement.name,
-                                          statement.nbparams,
-                                          &in_bindings.values_[0],
-                                          &in_bindings.lengths_[0],
-                                          &in_bindings.formats_[0], 0));
-
-    checkStatementError(result_set, statement);
+    // Execute the prepared statement.
+    PgSqlResultPtr result_set = executePreparedStatement(statement, in_bindings);
 
     // Iterate over the returned rows and invoke the row consumption
     // function on each one.
-    int rows = result_set.getRows();
+    int rows = result_set->getRows();
     for (int row = 0; row < rows; ++row) {
         try {
-            process_result_row(result_set, row);
+            process_result_row(*result_set, row);
         } catch (const std::exception& ex) {
             // Rethrow the exception with a bit more data.
             isc_throw(BadValue, ex.what() << ". Statement is <" <<
@@ -497,32 +520,17 @@ PgSqlConnection::selectQuery(PgSqlTaggedStatement& statement,
 void
 PgSqlConnection::insertQuery(PgSqlTaggedStatement& statement,
                              const PsqlBindArray& in_bindings) {
-    checkUnusable();
-
-    PgSqlResult result_set(PQexecPrepared(conn_, statement.name,
-                                          statement.nbparams,
-                                          &in_bindings.values_[0],
-                                          &in_bindings.lengths_[0],
-                                          &in_bindings.formats_[0], 0));
-
-    checkStatementError(result_set, statement);
+    // Execute the prepared statement.
+    PgSqlResultPtr result_set = executePreparedStatement(statement, in_bindings);
 }
 
 uint64_t
 PgSqlConnection::updateDeleteQuery(PgSqlTaggedStatement& statement,
                                    const PsqlBindArray& in_bindings) {
-    checkUnusable();
-
-    PgSqlResult result_set(PQexecPrepared(conn_, statement.name,
-                                          statement.nbparams,
-                                          &in_bindings.values_[0],
-                                          &in_bindings.lengths_[0],
-                                          &in_bindings.formats_[0], 0));
-
-    checkStatementError(result_set, statement);
+    // Execute the prepared statement.
+    PgSqlResultPtr result_set = executePreparedStatement(statement, in_bindings);
 
-    // Return the number of affected rows.
-    return (boost::lexical_cast<int>(PQcmdTuples(result_set)));
+    return (boost::lexical_cast<int>(PQcmdTuples(*result_set)));
 }
 
 } // end of isc::db namespace
diff --git a/src/lib/pgsql/pgsql_connection.h b/src/lib/pgsql/pgsql_connection.h
index 2f11474ea0..5422613fd3 100644
--- a/src/lib/pgsql/pgsql_connection.h
+++ b/src/lib/pgsql/pgsql_connection.h
@@ -406,6 +406,27 @@ public:
         }
     }
 
+    /// @brief Excutes a prepared SQL statement.
+    ///
+    /// It executes the given prepared SQL statement, after checking
+    /// for usability and input parameter sanity.  After the statement
+    /// is excuted @c checkStatementError() is invoked to ensure we detect
+    /// connectivity issues properly. Upon successful execution, the
+    /// the result set is returned.  It may be used for any form of
+    /// prepared SQL statement (e.g query, insert, udpate, delete...),
+    /// with or without input parameters.
+    ///
+    /// @param statement PgSqlTaggedStatement describing the prepared
+    /// statement to execute.
+    /// @param in_bindings array of input parameter bindings. If the SQL
+    /// statement requires no input arguments, this parameter should either
+    /// be omitted or an empty PsqlBindArray should be supplied.
+    /// @throw InvalidOperation if the number of parameters expected
+    /// by the statement does not match the size of the input bind array.
+    PgSqlResultPtr executePreparedStatement(PgSqlTaggedStatement& statement,
+                                            const PsqlBindArray& in_bindings
+                                            = PsqlBindArray());
+
     /// @brief Executes SELECT query using prepared statement.
     ///
     /// The statement parameter refers to an existing prepared statement
diff --git a/src/lib/pgsql/pgsql_exchange.cc b/src/lib/pgsql/pgsql_exchange.cc
index a16d8e057c..7246166548 100644
--- a/src/lib/pgsql/pgsql_exchange.cc
+++ b/src/lib/pgsql/pgsql_exchange.cc
@@ -260,6 +260,10 @@ std::string
 PsqlBindArray::toText() const {
     std::ostringstream stream;
 
+    if (values_.size() == 0) {
+        return ("bindarray is empty");
+    }
+
     for (int i = 0; i < values_.size(); ++i) {
         stream << i << " : ";
 
diff --git a/src/lib/pgsql/pgsql_exchange.h b/src/lib/pgsql/pgsql_exchange.h
index d81474da86..2dec0a83f4 100644
--- a/src/lib/pgsql/pgsql_exchange.h
+++ b/src/lib/pgsql/pgsql_exchange.h
@@ -151,6 +151,8 @@ typedef boost::shared_ptr<PgSqlResult> PgSqlResultPtr;
 typedef boost::shared_ptr<const std::string> ConstStringPtr;
 
 struct PsqlBindArray {
+    PsqlBindArray() : values_(0), lengths_(0), formats_(0) {};
+
     /// @brief Vector of pointers to the data values.
     std::vector<const char *> values_;
     /// @brief Vector of data lengths for each value.
diff --git a/src/lib/pgsql/tests/pgsql_connection_unittest.cc b/src/lib/pgsql/tests/pgsql_connection_unittest.cc
index 64be3234db..8b7ab7a701 100644
--- a/src/lib/pgsql/tests/pgsql_connection_unittest.cc
+++ b/src/lib/pgsql/tests/pgsql_connection_unittest.cc
@@ -88,6 +88,8 @@ public:
         DELETE_BY_INT_RANGE,
         INSERT_VALUE,
         UPDATE_BY_INT_VALUE,
+        GET_ALL_ROWS,
+        DELETE_ALL_ROWS,
         NUM_STATEMENTS
     };
 
@@ -113,7 +115,13 @@ public:
 
         { 2, { OID_INT4, OID_TEXT }, "UPDATE_BY_INT_VALUE",
           "UPDATE basics SET text_col = $2"
-          " WHERE int_col = $1" }
+          " WHERE int_col = $1" },
+
+        { 0, { OID_NONE }, "GET_ALL_ROWS",
+          "SELECT int_col, text_col FROM basics" },
+
+        { 0, { OID_NONE }, "DELETE_ALL_ROWS",
+          "DELETE FROM basics" }
     }};
 
     /// @brief Structure for holding data values describing a single
@@ -271,6 +279,59 @@ public:
     }
 };
 
+/// @brief Verifies basics of input parameter sanity checking and statement
+/// execution enforced by executePreparedStatement.  Higher order tests
+/// verify actual data CRUD results.
+TEST_F(PgSqlConnectionTest, executePreparedStatement) {
+
+    // Executing with no paramters when they are required should throw.
+    // First we'll omit the bindings (defaults to empty).
+    PgSqlResultPtr r;
+    ASSERT_THROW_MSG(r = conn_->executePreparedStatement(tagged_statements[INSERT_VALUE]),
+                     InvalidOperation,
+                     "executePreparedStatement: expected: 2 parameters, given: 0,"
+                     " statement: INSERT_INT_TEXT, SQL: INSERT INTO basics "
+                     "(int_col,text_col) VALUES ($1, $2)");
+
+    // Now we'll pass in an empty array.
+    PsqlBindArray in_bindings;
+    ASSERT_THROW_MSG(r = conn_->executePreparedStatement(tagged_statements[INSERT_VALUE],
+                                                         in_bindings),
+                     InvalidOperation,
+                     "executePreparedStatement: expected: 2 parameters, given: 0,"
+                     " statement: INSERT_INT_TEXT, SQL: INSERT INTO basics "
+                     "(int_col,text_col) VALUES ($1, $2)");
+
+    // Executing without parameters when none are expected should be fine.
+    // First we'll simply omit the array.
+    ASSERT_NO_THROW(r = conn_->executePreparedStatement(tagged_statements[GET_ALL_ROWS]));
+
+    // Now with an empty array.
+    ASSERT_NO_THROW(r = conn_->executePreparedStatement(tagged_statements[GET_ALL_ROWS], in_bindings));
+
+    // Executing with parameters when none are required should throw.
+    in_bindings.add(1);
+    in_bindings.add(2);
+    ASSERT_THROW_MSG(r = conn_->executePreparedStatement(tagged_statements[GET_ALL_ROWS],
+                                                         in_bindings),
+                     InvalidOperation,
+                     "executePreparedStatement: expected: 0 parameters, given: 2,"
+                     " statement: GET_ALL_ROWS, SQL: SELECT int_col, text_col FROM basics");
+
+    // Executing with the correct number of parameters should work.
+    ASSERT_NO_THROW(r = conn_->executePreparedStatement(tagged_statements[GET_BY_INT_RANGE],
+                                                         in_bindings));
+
+    // Executing with too many parameters should fail.
+    in_bindings.add(3);
+    ASSERT_THROW_MSG(r = conn_->executePreparedStatement(tagged_statements[GET_BY_INT_RANGE],
+                                                         in_bindings),
+                     InvalidOperation,
+                     "executePreparedStatement: expected: 2 parameters, given: 3,"
+                     " statement: GET_BY_INT_RANGE, SQL: SELECT int_col, text_col"
+                     " FROM basics WHERE int_col >= $1 and int_col <= $2");
+}
+
 /// @brief Verify that we can insert rows with
 /// PgSqlConnection::insertQuery() and fetch
 /// them using PgSqlConnection::selectQuery().