[324] make sure latest SQLite3 version (2.0) will be used on creation.

also, in sqlite3 accessor and the old data source API, check both the
major and minor versions, log any mismatches, and throw if major doesn't
match.  right now there's no test for it, which is bad, but this seems to
be best way forward from the given patch.

src/lib/datasrc/datasrc_messages.mes

@@ -70,6 +70,25 @@ The maximum allowed number of items of the hotspot cache is set to the given
 number. If there are too many, some of them will be dropped. The size of 0
 means no limit.
 
+% DATASRC_SQLITE_COMPATIBLE_VERSION database schema V%1.%2 not up to date (expecting V%3.%4) but is compatible
+The version of the SQLite3 database schema used to hold the zone data
+is not the latest one - the current version of BIND 10 was written
+with a later schema version in mind.  However, the database is
+compatible with the current version of BIND 10, and BIND 10 will run
+without any problems.
+
+Consult the release notes for your version of BIND 10.  Depending on
+the changes made to the database schema, it is possible that improved
+performance could result if the database were upgraded.
+
+% DATASRC_SQLITE_INCOMPATIBLE_VERSION database schema V%1.%2 incompatible with version (V%3.%4) expected
+The version of the SQLite3 database schema used to hold the zone data
+is incompatible with the version expected by BIND 10.  As a result,
+BIND 10 is unable to run using the database file as the data source.
+
+The database should be updated using the means described in the BIND
+10 documentation.
+
 % DATASRC_DATABASE_COVER_NSEC_UNSUPPORTED %1 doesn't support DNSSEC when asked for NSEC data covering %2
 The datasource tried to provide an NSEC proof that the named domain does not
 exist, but the database backend doesn't support DNSSEC. No proof is included

src/lib/datasrc/sqlite3_accessor.cc

@@ -15,8 +15,11 @@
 #include <sqlite3.h>
 
 #include <string>
+#include <utility>
 #include <vector>
 
+#include <exceptions/exceptions.h>
+
 #include <datasrc/sqlite3_accessor.h>
 #include <datasrc/logger.h>
 #include <datasrc/data_source.h>
@@ -27,7 +30,20 @@
 using namespace std;
 using namespace isc::data;
 
-#define SQLITE_SCHEMA_VERSION 1
+namespace {
+// Expected schema.  The major version must match else there is an error.  If
+// the minor version of the database is less than this, a warning is output.
+//
+// It is assumed that a program written to run on m.n of the database will run
+// with a database version m.p, where p is any number.  However, if p < n,
+// we assume that the database structure was upgraded for some reason, and that
+// some advantage may result if the database is upgraded. Conversely, if p > n,
+// The database is at a later version than the program was written for and the
+// program may not be taking advantage of features (possibly performance
+// improvements) added to the database.
+const int SQLITE_SCHEMA_MAJOR_VERSION = 2;
+const int SQLITE_SCHEMA_MINOR_VERSION = 0;
+}
 
 namespace isc {
 namespace datasrc {
@@ -125,8 +141,8 @@ const char* const text_statements[NUM_STATEMENTS] = {
 
 struct SQLite3Parameters {
     SQLite3Parameters() :
-        db_(NULL), version_(-1), in_transaction(false), updating_zone(false),
-        updated_zone_id(-1)
+        db_(NULL), version_(-1), minor_(-1), in_transaction(false),
+        updating_zone(false), updated_zone_id(-1)
     {
         for (int i = 0; i < NUM_STATEMENTS; ++i) {
             statements_[i] = NULL;
@@ -165,6 +181,7 @@ struct SQLite3Parameters {
 
     sqlite3* db_;
     int version_;
+    int minor_;
     bool in_transaction; // whether or not a transaction has been started
     bool updating_zone;          // whether or not updating the zone
     int updated_zone_id;        // valid only when in_transaction is true
@@ -255,8 +272,9 @@ public:
 };
 
 const char* const SCHEMA_LIST[] = {
-    "CREATE TABLE schema_version (version INTEGER NOT NULL)",
-    "INSERT INTO schema_version VALUES (1)",
+    "CREATE TABLE schema_version (version INTEGER NOT NULL, "
+        "minor INTEGER NOT NULL DEFAULT 0)",
+    "INSERT INTO schema_version VALUES (2, 0)",
     "CREATE TABLE zones (id INTEGER PRIMARY KEY, "
     "name TEXT NOT NULL COLLATE NOCASE, "
     "rdclass TEXT NOT NULL COLLATE NOCASE DEFAULT 'IN', "
@@ -315,14 +333,13 @@ void doSleep() {
 
 // returns the schema version if the schema version table exists
 // returns -1 if it does not
-int checkSchemaVersion(sqlite3* db) {
+int checkSchemaVersionElement(sqlite3* db, const char* const query) {
     sqlite3_stmt* prepared = NULL;
     // At this point in time, the database might be exclusively locked, in
     // which case even prepare() will return BUSY, so we may need to try a
     // few times
     for (size_t i = 0; i < 50; ++i) {
-        int rc = sqlite3_prepare_v2(db, "SELECT version FROM schema_version",
-                                    -1, &prepared, NULL);
+        int rc = sqlite3_prepare_v2(db, query, -1, &prepared, NULL);
         if (rc == SQLITE_ERROR) {
             // this is the error that is returned when the table does not
             // exist
@@ -344,8 +361,26 @@ int checkSchemaVersion(sqlite3* db) {
     return (version);
 }
 
+// Returns the schema major and minor version numbers in a pair.
+// Returns (-1, -1) if the table does not exist, (1, 0) for a V1
+// database, and (n, m) for any other.
+pair<int, int> checkSchemaVersion(sqlite3* db) {
+    int major = checkSchemaVersionElement(db,
+        "SELECT version FROM schema_version");
+    if (major == -1) {
+        return (make_pair(-1, -1));
+    } else if (major == 1) {
+        return (make_pair(1, 0));
+    } else {
+        int minor = checkSchemaVersionElement(db,
+            "SELECT minor FROM schema_version");
+        return (make_pair(major, minor));
+    }
+}
+
 // return db version
-int create_database(sqlite3* db) {
+pair<int, int>
+createDatabase(sqlite3* db) {
     // try to get an exclusive lock. Once that is obtained, do the version
     // check *again*, just in case this process was racing another
     //
@@ -363,8 +398,8 @@ int create_database(sqlite3* db) {
         }
         doSleep();
     }
-    int schema_version = checkSchemaVersion(db);
-    if (schema_version == -1) {
+    pair<int, int> schema_version = checkSchemaVersion(db);
+    if (schema_version.first == -1) {
         for (int i = 0; SCHEMA_LIST[i] != NULL; ++i) {
             if (sqlite3_exec(db, SCHEMA_LIST[i], NULL, NULL, NULL) !=
                 SQLITE_OK) {
@@ -373,21 +408,36 @@ int create_database(sqlite3* db) {
             }
         }
         sqlite3_exec(db, "COMMIT TRANSACTION", NULL, NULL, NULL);
-        return (SQLITE_SCHEMA_VERSION);
-    } else {
-        return (schema_version);
+
+        // Return the version.  We query again to ensure that the only point
+        // in which the current schema version is defined is in the create
+        // statements.
+        schema_version = checkSchemaVersion(db);
     }
+
+    return (schema_version);
 }
 
 void
 checkAndSetupSchema(Initializer* initializer) {
     sqlite3* const db = initializer->params_.db_;
 
-    int schema_version = checkSchemaVersion(db);
-    if (schema_version != SQLITE_SCHEMA_VERSION) {
-        schema_version = create_database(db);
-    }
-    initializer->params_.version_ = schema_version;
+    pair<int, int> schema_version = checkSchemaVersion(db);
+    if (schema_version.first == -1) {
+        schema_version = createDatabase(db);
+    } else if (schema_version.first != SQLITE_SCHEMA_MAJOR_VERSION) {
+        LOG_ERROR(logger, DATASRC_SQLITE_INCOMPATIBLE_VERSION)
+            .arg(schema_version.first).arg(schema_version.second)
+            .arg(SQLITE_SCHEMA_MAJOR_VERSION).arg(SQLITE_SCHEMA_MINOR_VERSION);
+        isc_throw(IncompatibleDbVersion, "incompatible database version");
+    } else if (schema_version.second < SQLITE_SCHEMA_MINOR_VERSION) {
+        LOG_WARN(logger, DATASRC_SQLITE_COMPATIBLE_VERSION)
+            .arg(schema_version.first).arg(schema_version.second)
+            .arg(SQLITE_SCHEMA_MAJOR_VERSION).arg(SQLITE_SCHEMA_MINOR_VERSION);
+    }
+
+    initializer->params_.version_ = schema_version.first;
+    initializer->params_.minor_ = schema_version.second;
 }
 
 }

src/lib/datasrc/sqlite3_accessor.h

@@ -47,6 +47,12 @@ public:
         DataSourceError(file, line, what) {}
 };
 
+class IncompatibleDbVersion : public Exception {
+public:
+    IncompatibleDbVersion(const char* file, size_t line, const char* what) :
+        isc::Exception(file, line, what) {}
+};
+
 /**
  * \brief Too Much Data
  *

src/lib/datasrc/sqlite3_datasrc.cc

@@ -14,19 +14,33 @@
 
 #include <string>
 #include <sstream>
+#include <utility>
 
 #include <sqlite3.h>
 
 #include <datasrc/sqlite3_datasrc.h>
 #include <datasrc/logger.h>
-
+#include <exceptions/exceptions.h>
 #include <dns/rrttl.h>
 #include <dns/rdata.h>
 #include <dns/rdataclass.h>
 #include <dns/rrset.h>
 #include <dns/rrsetlist.h>
 
-#define SQLITE_SCHEMA_VERSION 2
+namespace {
+// Expected schema.  The major version must match else there is an error.  If
+// the minor version of the database is less than this, a warning is output.
+//
+// It is assumed that a program written to run on m.n of the database will run
+// with a database version m.p, where p is any number.  However, if p < n,
+// we assume that the database structure was upgraded for some reason, and that
+// some advantage may result if the database is upgraded. Conversely, if p > n,
+// The database is at a later version than the program was written for and the
+// program may not be taking advantage of features (possibly performance
+// improvements) added to the database.
+const int SQLITE_SCHEMA_MAJOR_VERSION = 2;
+const int SQLITE_SCHEMA_MINOR_VERSION = 0;
+}
 
 using namespace std;
 using namespace isc::dns;
@@ -36,13 +50,14 @@ namespace isc {
 namespace datasrc {
 
 struct Sqlite3Parameters {
-    Sqlite3Parameters() :  db_(NULL), version_(-1),
+    Sqlite3Parameters() :  db_(NULL), version_(-1), minor_(-1),
         q_zone_(NULL), q_record_(NULL), q_addrs_(NULL), q_referral_(NULL),
         q_any_(NULL), q_count_(NULL), q_previous_(NULL), q_nsec3_(NULL),
         q_prevnsec3_(NULL)
     {}
     sqlite3* db_;
     int version_;
+    int minor_;
     sqlite3_stmt* q_zone_;
     sqlite3_stmt* q_record_;
     sqlite3_stmt* q_addrs_;
@@ -56,7 +71,8 @@ struct Sqlite3Parameters {
 
 namespace {
 const char* const SCHEMA_LIST[] = {
-    "CREATE TABLE schema_version (version INTEGER NOT NULL)",
+    "CREATE TABLE schema_version (version INTEGER NOT NULL, )"
+        "minor INTEGER NOT NULL DEFAULT 0)",
     "INSERT INTO schema_version VALUES (2)",
     "CREATE TABLE zones (id INTEGER PRIMARY KEY, "
     "name TEXT NOT NULL COLLATE NOCASE, "
@@ -89,6 +105,7 @@ const char* const SCHEMA_LIST[] = {
 };
 
 const char* const q_version_str = "SELECT version FROM schema_version";
+const char* const q_minor_str = "SELECT minor FROM schema_version";
 
 const char* const q_zone_str = "SELECT id FROM zones WHERE name=?1";
 
@@ -681,15 +698,15 @@ void do_sleep() {
     nanosleep(&req, NULL);
 }
 
-// returns the schema version if the schema version table exists
+// returns the schema version element if the schema version table exists
 // returns -1 if it does not
-int check_schema_version(sqlite3* db) {
+int check_schema_version_element(sqlite3* db, const char* const version_query) {
     sqlite3_stmt* prepared = NULL;
     // At this point in time, the database might be exclusively locked, in
     // which case even prepare() will return BUSY, so we may need to try a
     // few times
     for (size_t i = 0; i < 50; ++i) {
-        int rc = sqlite3_prepare_v2(db, q_version_str, -1, &prepared, NULL);
+        int rc = sqlite3_prepare_v2(db, version_query, -1, &prepared, NULL);
         if (rc == SQLITE_ERROR) {
             // this is the error that is returned when the table does not
             // exist
@@ -711,8 +728,25 @@ int check_schema_version(sqlite3* db) {
     return (version);
 }
 
+// Returns the schema major and minor version numbers in a pair.
+// Returns (-1, -1) if the table does not exist, (1, 0) for a V1
+// database, and (n, m) for any other.
+pair<int, int> check_schema_version(sqlite3* db) {
+    int major = check_schema_version_element(db, q_version_str);
+    if (major == -1) {
+        return (make_pair(-1, -1));
+    } else if (major == 1) {
+        return (make_pair(1, 0));
+    } else {
+        int minor = check_schema_version_element(db, q_minor_str);
+        return (make_pair(major, minor));
+    }
+}
+
+
+
 // return db version
-int create_database(sqlite3* db) {
+pair<int, int> create_database(sqlite3* db) {
     // try to get an exclusive lock. Once that is obtained, do the version
     // check *again*, just in case this process was racing another
     //
@@ -730,8 +764,9 @@ int create_database(sqlite3* db) {
         }
         do_sleep();
     }
-    int schema_version = check_schema_version(db);
-    if (schema_version == -1) {
+
+    pair<int, int> schema_version = check_schema_version(db);
+    if (schema_version.first == -1) {
         for (int i = 0; SCHEMA_LIST[i] != NULL; ++i) {
             if (sqlite3_exec(db, SCHEMA_LIST[i], NULL, NULL, NULL) !=
                 SQLITE_OK) {
@@ -740,22 +775,39 @@ int create_database(sqlite3* db) {
             }
         }
         sqlite3_exec(db, "COMMIT TRANSACTION", NULL, NULL, NULL);
-        return (SQLITE_SCHEMA_VERSION);
-    } else {
-        return (schema_version);
+
+        // Return the version. We query again to ensure that the only point
+        // in which the current schema version is defined is in the
+        // CREATE statements.
+        schema_version = check_schema_version(db);
     }
+    return (schema_version);
 }
 
 void
 checkAndSetupSchema(Sqlite3Initializer* initializer) {
     sqlite3* const db = initializer->params_.db_;
 
-    int schema_version = check_schema_version(db);
-    if (schema_version != SQLITE_SCHEMA_VERSION) {
+    // Note: we use the same SCHEMA_xxx_VERSION log IDs here and in
+    // sqlite3_accessor.cc, which is against our policy of ID uniqueness.
+    // The assumption is that this file will soon be deprecated, and we don't
+    // bother to define separate IDs for the short period.
+    pair<int, int> schema_version = check_schema_version(db);
+    if (schema_version.first == -1) {
         schema_version = create_database(db);
-    }
-    initializer->params_.version_ = schema_version;
-
+    } else if (schema_version.first != SQLITE_SCHEMA_MAJOR_VERSION) {
+        LOG_ERROR(logger, DATASRC_SQLITE_INCOMPATIBLE_VERSION)
+            .arg(schema_version.first).arg(schema_version.second)
+            .arg(SQLITE_SCHEMA_MAJOR_VERSION).arg(SQLITE_SCHEMA_MINOR_VERSION);
+        isc_throw(IncompatibleDbVersion, "Incompatible database version");
+    } else if (schema_version.second < SQLITE_SCHEMA_MINOR_VERSION) {
+        LOG_WARN(logger, DATASRC_SQLITE_COMPATIBLE_VERSION)
+            .arg(schema_version.first).arg(schema_version.second)
+            .arg(SQLITE_SCHEMA_MAJOR_VERSION).arg(SQLITE_SCHEMA_MINOR_VERSION);
+    }
+
+    initializer->params_.version_ = schema_version.first;
+    initializer->params_.minor_ = schema_version.second;
     initializer->params_.q_zone_ = prepare(db, q_zone_str);
     initializer->params_.q_record_ = prepare(db, q_record_str);
     initializer->params_.q_addrs_ = prepare(db, q_addrs_str);

src/lib/datasrc/sqlite3_datasrc.h

@@ -41,6 +41,12 @@ public:
         isc::Exception(file, line, what) {}
 };
 
+class IncompatibleDbVersion : public Exception {
+public:
+    IncompatibleDbVersion(const char* file, size_t line, const char* what) :
+        isc::Exception(file, line, what) {}
+};
+
 class Sqlite3DataSrc : public DataSrc {
     ///
     /// \name Constructors, Assignment Operator and Destructor.

src/lib/python/isc/datasrc/sqlite3_ds.py

@@ -23,7 +23,7 @@ RR_NAME_INDEX = 2
 RR_TTL_INDEX = 4
 RR_RDATA_INDEX = 7
 
-# Current version of schema (maybe we need a minor version, too)
+# Current major version of schema
 SCHEMA_VERSION = 2
 
 class Sqlite3DSError(Exception):
@@ -50,9 +50,10 @@ def create(cur):
         cur.execute("SELECT version FROM schema_version")
         row = cur.fetchone()
     except sqlite3.OperationalError:
-        cur.execute("CREATE TABLE schema_version (version INTEGER NOT NULL)")
+        cur.execute("""CREATE TABLE schema_version (version INTEGER NOT NULL),
+                    minor INTEGER NOT NULL DEFAULT 0""")
         cur.execute("INSERT INTO schema_version VALUES (" +
-                    str(SCHEMA_VERSION) + ")")
+                    str(SCHEMA_VERSION) + ")") # use the default minor version
         cur.execute("""CREATE TABLE zones (id INTEGER PRIMARY KEY,
                     name TEXT NOT NULL COLLATE NOCASE,
                     rdclass TEXT NOT NULL COLLATE NOCASE DEFAULT 'IN',