BIND 10 trac1330, updated. 2f51afcbc57c6d58e7d90f37962f3b93bc768e1b [1330] Test to check first record returned is OK passes

BIND 10 source code commits bind10-changes at lists.isc.org
Fri Nov 4 21:23:59 UTC 2011


The branch, trac1330 has been updated
       via  2f51afcbc57c6d58e7d90f37962f3b93bc768e1b (commit)
       via  918c35143eb61d6e0ac96e98f2a95b12d55fdc0c (commit)
      from  480da1fe075da66aa8a144d37c23bac2fcfa1e2c (commit)

Those revisions listed above that are new to this repository have
not appeared on any other notification email; so we list those
revisions in full, below.

- Log -----------------------------------------------------------------
commit 2f51afcbc57c6d58e7d90f37962f3b93bc768e1b
Author: Stephen Morris <stephen at isc.org>
Date:   Fri Nov 4 21:22:42 2011 +0000

    [1330] Test to check first record returned is OK passes
    
    Added code to handle getting set of records between two indexes.

commit 918c35143eb61d6e0ac96e98f2a95b12d55fdc0c
Author: Stephen Morris <stephen at isc.org>
Date:   Fri Nov 4 20:38:49 2011 +0000

    [1330] Distinguish between no records for zone and none for serial number

-----------------------------------------------------------------------

Summary of changes:
 src/lib/datasrc/database.h                         |    3 +-
 src/lib/datasrc/sqlite3_accessor.cc                |  212 ++++++++++++++++----
 src/lib/datasrc/sqlite3_accessor.h                 |   41 ++++-
 src/lib/datasrc/tests/database_unittest.cc         |    2 +-
 src/lib/datasrc/tests/sqlite3_accessor_unittest.cc |   11 +-
 5 files changed, 220 insertions(+), 49 deletions(-)

-----------------------------------------------------------------------
diff --git a/src/lib/datasrc/database.h b/src/lib/datasrc/database.h
index f2bb4e7..1ae583c 100644
--- a/src/lib/datasrc/database.h
+++ b/src/lib/datasrc/database.h
@@ -319,7 +319,8 @@ public:
      *
      * \return Newly created iterator context. Must not be NULL.
      */
-    virtual IteratorContextPtr getDiffs(int id, int start, int end) const = 0;
+    virtual IteratorContextPtr
+    getDiffs(int id, uint32_t start, uint32_t end) const = 0;
 
     /// Start a transaction for updating a zone.
     ///
diff --git a/src/lib/datasrc/sqlite3_accessor.cc b/src/lib/datasrc/sqlite3_accessor.cc
index ccd9677..e5c35b6 100644
--- a/src/lib/datasrc/sqlite3_accessor.cc
+++ b/src/lib/datasrc/sqlite3_accessor.cc
@@ -54,10 +54,11 @@ enum StatementID {
     FIND_PREVIOUS = 10,
     ADD_RECORD_DIFF = 11,
     GET_RECORD_DIFF = 12,       // This is temporary for testing "add diff"
-    LOW_DIFF_ID = 13,
-    HIGH_DIFF_ID = 14,
-    DIFFS = 15,
-    NUM_STATEMENTS = 16
+    COUNT_DIFF_RECS = 13,
+    LOW_DIFF_ID = 14,
+    HIGH_DIFF_ID = 15,
+    DIFF_RECS = 16,
+    NUM_STATEMENTS = 17
 };
 
 const char* const text_statements[NUM_STATEMENTS] = {
@@ -95,13 +96,18 @@ const char* const text_statements[NUM_STATEMENTS] = {
 
     // Two statements to select the lowest ID and highest ID in a set of
     // differences.
+    "SELECT COUNT(id) FROM diffs "  // COUNT_DIFF_RECS
+        "WHERE zone_id=?1",
     "SELECT id FROM diffs "     // LOW_DIFF_ID
         "WHERE zone_id=?1 AND version=?2 and OPERATION=0 "
         "ORDER BY id ASC LIMIT 1",
     "SELECT id FROM diffs "     // HIGH_DIFF_ID
         "WHERE zone_id=?1 AND version=?2 and OPERATION=1 "
         "ORDER BY id DESC LIMIT 1",
-    "SELECT name, rrtype, ttl, rdata FROM diffs "   // DIFFS
+
+    // In the next statement, note the redundant ID.  This is to ensure
+    // that the columns match the column IDs passed to the iterator
+    "SELECT rrtype, ttl, id, rdata, name FROM diffs "   // DIFF_RECS
         "WHERE zone_id=?1 AND id>=?2 and id<=?3"
 };
 
@@ -595,31 +601,86 @@ SQLite3Accessor::getAllRecords(int id) const {
 
 /// \brief Difference Iterator
 ///
-///
+/// This iterator is used to search through the differences table for the
+/// resouce records making up an IXFR between two versions of a zone.
 
 class SQLite3Accessor::DiffContext : public DatabaseAccessor::IteratorContext {
 public:
 
-    // Construct an iterator for difference records.
+    /// \brief Constructor
+    ///
+    /// Constructs the iterator for the difference sequence.  It is
+    /// passed two parameters, the first and last versions in the difference
+    /// sequence.  Note that because of serial number rollover, it may well
+    /// be that the start serial number is greater than the end one.
+    ///
+    /// \param zone_id ID of the zone (in the zone table)
+    /// \param start Serial number of first version in difference sequence
+    /// \param end Serial number of last version in difference sequence
+    ///
+    /// \exception any A number of exceptions can be expected
     DiffContext(const boost::shared_ptr<const SQLite3Accessor>& accessor,
-                int id, int start, int end) :
-        accessor_(accessor)
+                int zone_id, uint32_t start, uint32_t end) :
+        accessor_(accessor),
+        last_status_(SQLITE_ROW)
     {
         try {
-            int low_id = findIndex(LOW_DIFF_ID, id, start);
-            int high_id = findIndex(HIGH_DIFF_ID, id, end);
-            std::cout << "Low index is " << low_id << ", high index is " << high_id << "\n";
+            int low_id = findIndex(LOW_DIFF_ID, zone_id, start);
+            int high_id = findIndex(HIGH_DIFF_ID, zone_id, end);
+
+            // Prepare the statement that will return data values
+            clearBindings(DIFF_RECS);
+            bindInt(DIFF_RECS, 1, zone_id);
+            bindInt(DIFF_RECS, 2, low_id);
+            bindInt(DIFF_RECS, 3, high_id);
+
+            // last_status_ has been initialized to pretend that the last
+            // getNext() returned a record.
+
         } catch (...) {
             accessor_->dbparameters_->finalizeStatements();
             throw;
         }
     }
 
-    virtual ~DiffContext() {}
+    /// \brief Destructor
+    virtual ~DiffContext()
+    {}
 
-    virtual bool getNext(std::string (&data)[COLUMN_COUNT]) {
-        static_cast<void>(data[0]);
-        return (false);
+    /// \brief Get Next Diff Record
+    ///
+    /// Returns the next difference record in the difference sequence.
+    ///
+    /// \param data Array of std::strings COLUMN_COUNT long.  The results
+    ///        are returned in this.
+    ///
+    /// \return bool true if data is returned, false if not.
+    ///
+    /// \exceptions any Varied
+    bool getNext(std::string (&data)[COLUMN_COUNT]) {
+
+        // Get a pointer to the statement for brevity (does not transfer
+        // resources)
+        sqlite3_stmt* stmt = accessor_->dbparameters_->getStatement(DIFF_RECS);
+
+        // If there is a row to get, get it.
+        if (last_status_ != SQLITE_DONE) {
+            const int rc(sqlite3_step(stmt));
+            if (rc == SQLITE_ROW) {
+                // Copy the data across to the output array
+                copyColumn(DIFF_RECS, data, TYPE_COLUMN);
+                copyColumn(DIFF_RECS, data, TTL_COLUMN);
+                copyColumn(DIFF_RECS, data, NAME_COLUMN);
+                copyColumn(DIFF_RECS, data, RDATA_COLUMN);
+            } else if (rc != SQLITE_DONE) {
+                accessor_->dbparameters_->finalizeStatements();
+                isc_throw(DataSourceError,
+                          "Unexpected failure in sqlite3_step: " <<
+                          sqlite3_errmsg(accessor_->dbparameters_->db_));
+            }
+            last_status_ = rc;
+        }
+        return (last_status_ == SQLITE_ROW);
     }
 
 private:
@@ -633,7 +694,7 @@ private:
     void clearBindings(int stindex) {
         if (sqlite3_clear_bindings(
             accessor_->dbparameters_->getStatement(stindex)) != SQLITE_OK) {
-            isc_throw(SQLite3Error, "Could not clear SQL statement bindings in '" <<
+            isc_throw(SQLite3Error, "Could not clear statement bindings in '" <<
                       text_statements[stindex] << "': " << 
                       sqlite3_errmsg(accessor_->dbparameters_->db_));
         }
@@ -647,8 +708,8 @@ private:
     /// \param varindex Index of variable to which to bind
     /// \param value Value of variable to bind
     /// \exception SQLite3Error on an error
-    void bindInt(int stindex, int varindex, int value) {
-        if (sqlite3_bind_int(accessor_->dbparameters_->getStatement(stindex),
+    void bindInt(int stindex, int varindex, sqlite3_int64 value) {
+        if (sqlite3_bind_int64(accessor_->dbparameters_->getStatement(stindex),
                              varindex, value) != SQLITE_OK) {
             isc_throw(SQLite3Error, "Could not bind value to parameter " <<
                       varindex << " in statement '" <<
@@ -657,25 +718,19 @@ private:
         }
     }
 
-    /// \brief Find index
+    ///\brief Get Single Value
     ///
-    /// Executes the prepared statement locating the high or low index in
-    /// the diffs table and returns that index.
+    /// Executes a prepared statement (which has parameters bound to it)
+    /// for which the result of a single value is expected.
     ///
-    /// \param stmt_id Index of the prepared statement to execute
-    /// \param zone_id ID of the zone for which the index is being sought
-    /// \param serial Zone serial number for which an index is being sought.
+    /// \param stindex Index of prepared statement in statement table.
     ///
-    /// \return int ID of the row in the difss table corresponding to the
-    ///         statement.
+    /// \return Value of SELECT.
     ///
-    /// \exception NoSuchSerial Serial number not found
-    int findIndex(StatementID stindex, int zone_id, uint32_t serial) {
-
-        // Set up the statement
-        clearBindings(stindex);
-        bindInt(stindex, 1, zone_id);
-        bindInt(stindex, 2, serial);
+    /// \exception TooMuchData Multiple rows returned when one expected
+    /// \exception TooLittleData Zero rows returned when one expected
+    /// \exception DataSourceError SQLite3-related error
+    int getSingleValue(StatementID stindex) {
 
         // Get a pointer to the statement for brevity (does not transfer
         // resources)
@@ -687,7 +742,7 @@ private:
         if (rc == SQLITE_ROW) {
 
             // Got some data, extract the value
-            result = sqlite3_column_int(stmt, 1);
+            result = sqlite3_column_int(stmt, 0);
             int rc = sqlite3_step(stmt);
             if (rc == SQLITE_DONE) {
 
@@ -695,15 +750,15 @@ private:
                 return (result);
 
             } else if (rc == SQLITE_ROW) {
-                isc_throw(DataSourceError, "request to return one value from "
-                          "diffs table for serial " << serial << " (zone ID " <<
-                          zone_id << ") returned multiple values");
+                isc_throw(TooMuchData, "request to return one value from "
+                          "diffs table returned multiple values");
             }
         } else if (rc == SQLITE_DONE) {
 
-            // No data in the table for this zone and version.  Note that.
-            isc_throw(NoSuchSerial, "no data on serial number " << serial <<
-                      " for zone ID " << zone_id);
+            // No data in the table.  A bare exception with no explanation is
+            // thrown, as it will be replaced by a more informative one by
+            // the caller.
+            isc_throw(TooLittleData, "");
         }
 
         // We get here on an error.
@@ -714,14 +769,87 @@ private:
         return (result);
     }
 
+    /// \brief Find index
+    ///
+    /// Executes the prepared statement locating the high or low index in
+    /// the diffs table and returns that index.
+    ///
+    /// \param stmt_id Index of the prepared statement to execute
+    /// \param zone_id ID of the zone for which the index is being sought
+    /// \param serial Zone serial number for which an index is being sought.
+    ///
+    /// \return int ID of the row in the difss table corresponding to the
+    ///         statement.
+    ///
+    /// \exception TooLittleData Internal error, no result returned when one
+    ///            was expected.
+    /// \exception NoSuchSerial Serial number not found.
+    /// \exception NoDiffsData No data for this zone found in diffs table
+    int findIndex(StatementID stindex, int zone_id, uint32_t serial) {
+
+        // Set up the statement
+        clearBindings(stindex);
+        bindInt(stindex, 1, zone_id);
+        bindInt(stindex, 2, serial);
+
+        // Execute the statement
+        int result = -1;
+        try {
+            result = getSingleValue(stindex);
+
+        } catch (TooLittleData) {
+
+            // Why is there too little data?  Could be there is no data in
+            // the table for the zone, or there is but there is no data for
+            // that particular serial number.  Do another query to find out.
+            clearBindings(COUNT_DIFF_RECS);
+            bindInt(COUNT_DIFF_RECS, 1, zone_id);
+
+            // If this throws an exception, let it propagate - there is
+            // definitely an error.
+            result = getSingleValue(COUNT_DIFF_RECS);
+            if (result == 0) {
+                isc_throw(NoDiffRecs, "no data in differences table for "
+                          "zone ID " << zone_id);
+            } else {
+                isc_throw(NoSuchSerial, "no data in differences table for "
+                          "zone ID " << zone_id << ", serial number " <<
+                          serial);
+            }
+        }
+
+        return (result);
+    }
+
+    /// \brief Copy Column to Output
+    ///
+    /// Copies the textual data in the result set to the specified column
+    /// in the output.
+    ///
+    /// \param stindex Index of prepared statement used to access data
+    /// \param data Array of columns passed to getNext
+    /// \param column Column of output to copy
+    void copyColumn(StatementID stindex, std::string (&data)[COLUMN_COUNT],
+                    int column) {
+
+        // Get a pointer to the statement for brevity (does not transfer
+        // resources)
+        sqlite3_stmt* stmt = accessor_->dbparameters_->getStatement(stindex);
+        data[column] = convertToPlainChar(sqlite3_column_text(stmt,
+                                                              column),
+                                          accessor_->dbparameters_->db_);
+    }
     
+    // Attributes
+
     boost::shared_ptr<const SQLite3Accessor> accessor_; // Accessor object
+    int last_status_;   // Last status received from sqlite3_step
 };
 
 // ... and return the iterator
 
 DatabaseAccessor::IteratorContextPtr
-SQLite3Accessor::getDiffs(int id, int start, int end) const {
+SQLite3Accessor::getDiffs(int id, uint32_t start, uint32_t end) const {
     return (IteratorContextPtr(new DiffContext(shared_from_this(), id, start,
                                end)));
 }
diff --git a/src/lib/datasrc/sqlite3_accessor.h b/src/lib/datasrc/sqlite3_accessor.h
index 7e51588..eef3b7f 100644
--- a/src/lib/datasrc/sqlite3_accessor.h
+++ b/src/lib/datasrc/sqlite3_accessor.h
@@ -47,10 +47,46 @@ public:
 };
 
 /**
+ * \brief Too Much Data
+ *
+ * Thrown if a query expecting a certain number of rows back returned too
+ * many rows.
+ */
+class TooMuchData : public Exception {
+public:
+    TooMuchData(const char* file, size_t line, const char* what) :
+        isc::Exception(file, line, what) {}
+};
+
+/**
+ * \brief Too Little Data
+ *
+ * Thrown if a query expecting a certain number of rows back returned too
+ * few rows (including none).
+ */
+class TooLittleData : public Exception {
+public:
+    TooLittleData(const char* file, size_t line, const char* what) :
+        isc::Exception(file, line, what) {}
+};
+
+/**
+ * \brief No difference records
+ *
+ * Thrown if there are no difference records in the table for the requested
+ * zone.
+ */
+class NoDiffRecs : public Exception {
+public:
+    NoDiffRecs(const char* file, size_t line, const char* what) :
+        isc::Exception(file, line, what) {}
+};
+
+/**
  * \brief No such serial number when obtaining difference iterator
  *
  * Thrown if either the start or end version requested for the difference
- * iterator doe nsot exist.
+ * iterator does not exist.
  */
 class NoSuchSerial : public Exception {
 public:
@@ -157,7 +193,8 @@ public:
      *
      * \return Iterator containing difference records.
      */
-    virtual IteratorContextPtr getDiffs(int id, int start, int end) const;
+    virtual IteratorContextPtr
+    getDiffs(int id, uint32_t start, uint32_t end) const;
                                         
 
 
diff --git a/src/lib/datasrc/tests/database_unittest.cc b/src/lib/datasrc/tests/database_unittest.cc
index 206c762..83d4e81 100644
--- a/src/lib/datasrc/tests/database_unittest.cc
+++ b/src/lib/datasrc/tests/database_unittest.cc
@@ -252,7 +252,7 @@ public:
                   "This database datasource can't be iterated");
     }
 
-    virtual IteratorContextPtr getDiffs(int, int, int) const {
+    virtual IteratorContextPtr getDiffs(int, uint32_t, uint32_t) const {
         isc_throw(isc::NotImplemented,
                   "This database datasource can't be iterated");
     }
diff --git a/src/lib/datasrc/tests/sqlite3_accessor_unittest.cc b/src/lib/datasrc/tests/sqlite3_accessor_unittest.cc
index 732f56f..0d8454d 100644
--- a/src/lib/datasrc/tests/sqlite3_accessor_unittest.cc
+++ b/src/lib/datasrc/tests/sqlite3_accessor_unittest.cc
@@ -212,7 +212,7 @@ TEST_F(SQLite3AccessorTest, iterator) {
 // Test that at attempt to create a difference iterator for a serial that
 // does not exist throws an exception.
 
-TEST_F(SQLite3AccessorTest, diffIteratorNoVersion) {
+TEST_F(SQLite3AccessorTest, diffIteratorNoRecords) {
 
     // Our test zone is conveniently small, but not empty
     initAccessor(SQLITE_DBFILE_DIFFS, "IN");
@@ -229,6 +229,11 @@ TEST_F(SQLite3AccessorTest, diffIteratorNoVersion) {
     EXPECT_THROW(accessor->getDiffs(zone_info.second, 1231, 2234),
                  NoSuchSerial);
 
+    // Check that valid versions - but for the wrong zone which does not hold
+    // any records - throws the correct exception.
+    EXPECT_THROW(accessor->getDiffs(zone_info.second + 42, 1231, 1234),
+                 NoDiffRecs);
+
 }
 
 // Try to iterate through a valid set of differences
@@ -249,8 +254,8 @@ TEST_F(SQLite3AccessorTest, validSequence) {
     // Check the records
     EXPECT_TRUE(context->getNext(data));
     EXPECT_EQ("SOA", data[DatabaseAccessor::TYPE_COLUMN]);
-    EXPECT_EQ("3600", data[DatabaseAccessor::TTL_COLUMN]);
-    EXPECT_EQ("ns1.example.org. admin.example.org. 1230, 3600 1800 2419200 7200",
+    EXPECT_EQ("1800", data[DatabaseAccessor::TTL_COLUMN]);
+    EXPECT_EQ("ns1.example.org. admin.example.org. 1230 3600 1800 2419200 7200",
         data[DatabaseAccessor::RDATA_COLUMN]);
     EXPECT_EQ("example.org.", data[DatabaseAccessor::NAME_COLUMN]);
 }




More information about the bind10-changes mailing list