[324] various fixes for SQLite3 schema

- STRING => TEXT
- make sure the right side of LIKE is a placeholder for a string, not an
  expression (otherwise it doesn't use indices)
- introduce the "records_bytype_and_rname" index.  it's necessary to optimize
  the FIND_PREVIOUS query.

compatcheck/sqlite3-difftbl-check.py.in

@@ -47,9 +47,9 @@ except sqlite3.OperationalError as ex:
                         'zone_id INTEGER NOT NULL, ' +
                         'version INTEGER NOT NULL, ' +
                         'operation INTEGER NOT NULL, ' +
-                        'name STRING NOT NULL COLLATE NOCASE, ' +
-                        'rrtype STRING NOT NULL COLLATE NOCASE, ' +
-                        'ttl INTEGER NOT NULL, rdata STRING NOT NULL)')
+                        'name TEXT NOT NULL COLLATE NOCASE, ' +
+                        'rrtype TEXT NOT NULL COLLATE NOCASE, ' +
+                        'ttl INTEGER NOT NULL, rdata TEXT NOT NULL)')
         else:
             sys.stdout.write('Found an older version of SQLite3 DB file: ' +
                              db_file + '\n' + "Perform '" + os.getcwd() +

src/lib/datasrc/sqlite3_accessor.cc

@@ -245,31 +245,38 @@ const char* const SCHEMA_LIST[] = {
     "CREATE TABLE schema_version (version INTEGER NOT NULL)",
     "INSERT INTO schema_version VALUES (1)",
     "CREATE TABLE zones (id INTEGER PRIMARY KEY, "
-    "name STRING NOT NULL COLLATE NOCASE, "
-    "rdclass STRING NOT NULL COLLATE NOCASE DEFAULT 'IN', "
+    "name TEXT NOT NULL COLLATE NOCASE, "
+    "rdclass TEXT NOT NULL COLLATE NOCASE DEFAULT 'IN', "
     "dnssec BOOLEAN NOT NULL DEFAULT 0)",
     "CREATE INDEX zones_byname ON zones (name)",
     "CREATE TABLE records (id INTEGER PRIMARY KEY, "
-        "zone_id INTEGER NOT NULL, name STRING NOT NULL COLLATE NOCASE, "
-        "rname STRING NOT NULL COLLATE NOCASE, ttl INTEGER NOT NULL, "
-        "rdtype STRING NOT NULL COLLATE NOCASE, sigtype STRING COLLATE NOCASE, "
-        "rdata STRING NOT NULL)",
+        "zone_id INTEGER NOT NULL, name TEXT NOT NULL COLLATE NOCASE, "
+        "rname TEXT NOT NULL COLLATE NOCASE, ttl INTEGER NOT NULL, "
+        "rdtype TEXT NOT NULL COLLATE NOCASE, sigtype TEXT COLLATE NOCASE, "
+        "rdata TEXT NOT NULL)",
     "CREATE INDEX records_byname ON records (name)",
     "CREATE INDEX records_byrname ON records (rname)",
+    // The next index is a tricky one.  It's necessary for
+    // FIND_PREVIOUS to use the index efficiently; since there's an
+    // "inequality", the rname column must be placed later.  records_byrname
+    // may not be sufficient especially when the zone is not signed (and
+    // defining a separate index for rdtype only doesn't work either; SQLite3
+    // would then create a temporary B-tree for "ORDER BY").
+    "CREATE INDEX records_bytype_and_rname ON records (rdtype, rname)",
     "CREATE TABLE nsec3 (id INTEGER PRIMARY KEY, zone_id INTEGER NOT NULL, "
-        "hash STRING NOT NULL COLLATE NOCASE, "
-        "owner STRING NOT NULL COLLATE NOCASE, "
-        "ttl INTEGER NOT NULL, rdtype STRING NOT NULL COLLATE NOCASE, "
-        "rdata STRING NOT NULL)",
+        "hash TEXT NOT NULL COLLATE NOCASE, "
+        "owner TEXT NOT NULL COLLATE NOCASE, "
+        "ttl INTEGER NOT NULL, rdtype TEXT NOT NULL COLLATE NOCASE, "
+        "rdata TEXT NOT NULL)",
     "CREATE INDEX nsec3_byhash ON nsec3 (hash)",
     "CREATE TABLE diffs (id INTEGER PRIMARY KEY, "
         "zone_id INTEGER NOT NULL, "
         "version INTEGER NOT NULL, "
         "operation INTEGER NOT NULL, "
-        "name STRING NOT NULL COLLATE NOCASE, "
-        "rrtype STRING NOT NULL COLLATE NOCASE, "
+        "name TEXT NOT NULL COLLATE NOCASE, "
+        "rrtype TEXT NOT NULL COLLATE NOCASE, "
         "ttl INTEGER NOT NULL, "
-        "rdata STRING NOT NULL)",
+        "rdata TEXT NOT NULL)",
     NULL
 };
 

src/lib/datasrc/sqlite3_datasrc.cc

@@ -26,7 +26,7 @@
 #include <dns/rrset.h>
 #include <dns/rrsetlist.h>
 
-#define SQLITE_SCHEMA_VERSION 1
+#define SQLITE_SCHEMA_VERSION 2
 
 using namespace std;
 using namespace isc::dns;
@@ -57,33 +57,34 @@ struct Sqlite3Parameters {
 namespace {
 const char* const SCHEMA_LIST[] = {
     "CREATE TABLE schema_version (version INTEGER NOT NULL)",
-    "INSERT INTO schema_version VALUES (1)",
+    "INSERT INTO schema_version VALUES (2)",
     "CREATE TABLE zones (id INTEGER PRIMARY KEY, "
-    "name STRING NOT NULL COLLATE NOCASE, "
-    "rdclass STRING NOT NULL COLLATE NOCASE DEFAULT 'IN', "
+    "name TEXT NOT NULL COLLATE NOCASE, "
+    "rdclass TEXT NOT NULL COLLATE NOCASE DEFAULT 'IN', "
     "dnssec BOOLEAN NOT NULL DEFAULT 0)",
     "CREATE INDEX zones_byname ON zones (name)",
     "CREATE TABLE records (id INTEGER PRIMARY KEY, "
-    "zone_id INTEGER NOT NULL, name STRING NOT NULL COLLATE NOCASE, "
-    "rname STRING NOT NULL COLLATE NOCASE, ttl INTEGER NOT NULL, "
-    "rdtype STRING NOT NULL COLLATE NOCASE, sigtype STRING COLLATE NOCASE, "
-    "rdata STRING NOT NULL)",
+    "zone_id INTEGER NOT NULL, name TEXT NOT NULL COLLATE NOCASE, "
+    "rname TEXT NOT NULL COLLATE NOCASE, ttl INTEGER NOT NULL, "
+    "rdtype TEXT NOT NULL COLLATE NOCASE, sigtype TEXT COLLATE NOCASE, "
+    "rdata TEXT NOT NULL)",
     "CREATE INDEX records_byname ON records (name)",
     "CREATE INDEX records_byrname ON records (rname)",
+    "CREATE INDEX records_bytype_and_rname ON records (rdtype, rname)",
     "CREATE TABLE nsec3 (id INTEGER PRIMARY KEY, zone_id INTEGER NOT NULL, "
-    "hash STRING NOT NULL COLLATE NOCASE, "
-    "owner STRING NOT NULL COLLATE NOCASE, "
-    "ttl INTEGER NOT NULL, rdtype STRING NOT NULL COLLATE NOCASE, "
-    "rdata STRING NOT NULL)",
+    "hash TEXT NOT NULL COLLATE NOCASE, "
+    "owner TEXT NOT NULL COLLATE NOCASE, "
+    "ttl INTEGER NOT NULL, rdtype TEXT NOT NULL COLLATE NOCASE, "
+    "rdata TEXT NOT NULL)",
     "CREATE INDEX nsec3_byhash ON nsec3 (hash)",
     "CREATE TABLE diffs (id INTEGER PRIMARY KEY, "
         "zone_id INTEGER NOT NULL, "
         "version INTEGER NOT NULL, "
         "operation INTEGER NOT NULL, "
-        "name STRING NOT NULL COLLATE NOCASE, "
-        "rrtype STRING NOT NULL COLLATE NOCASE, "
+        "name TEXT NOT NULL COLLATE NOCASE, "
+        "rrtype TEXT NOT NULL COLLATE NOCASE, "
         "ttl INTEGER NOT NULL, "
-        "rdata STRING NOT NULL)",
+        "rdata TEXT NOT NULL)",
     NULL
 };
 
@@ -109,12 +110,16 @@ const char* const q_referral_str = "SELECT rdtype, ttl, sigtype, rdata FROM "
 const char* const q_any_str = "SELECT rdtype, ttl, sigtype, rdata "
     "FROM records WHERE zone_id=?1 AND name=?2";
 
+// Note: the wildcard symbol '%' is expected to be added to the text
+// for the placeholder for LIKE given via sqlite3_bind_text().  We don't
+// use the expression such as (?2 || '%') because it would disable the use
+// of indices and could result in terrible performance.
 const char* const q_count_str = "SELECT COUNT(*) FROM records "
-    "WHERE zone_id=?1 AND rname LIKE (?2 || '%');";
+    "WHERE zone_id=?1 AND rname LIKE ?2;";
 
 const char* const q_previous_str = "SELECT name FROM records "
-    "WHERE zone_id=?1 AND rdtype = 'NSEC' AND "
-    "rname < $2 ORDER BY rname DESC LIMIT 1";
+    "WHERE rname < ?2 AND zone_id=?1 AND rdtype = 'NSEC' "
+    "ORDER BY rname DESC LIMIT 1";
 
 const char* const q_nsec3_str = "SELECT rdtype, ttl, rdata FROM nsec3 "
     "WHERE zone_id = ?1 AND hash = $2";
@@ -314,8 +319,9 @@ Sqlite3DataSrc::findRecords(const Name& name, const RRType& rdtype,
                   " to SQL statement (qcount)");
     }
 
-    const string revname_text = name.reverse().toText();
-    rc = sqlite3_bind_text(dbparameters->q_count_, 2, revname_text.c_str(),
+    const string revname_text = name.reverse().toText() + "%";
+    rc = sqlite3_bind_text(dbparameters->q_count_, 2,
+                           revname_text.c_str(),
                            -1, SQLITE_STATIC);
     if (rc != SQLITE_OK) {
         isc_throw(Sqlite3Error, "Could not bind name " << name.reverse() <<

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

@@ -23,6 +23,9 @@ RR_NAME_INDEX = 2
 RR_TTL_INDEX = 4
 RR_RDATA_INDEX = 7
 
+# Current version of schema (maybe we need a minor version, too)
+SCHEMA_VERSION = 2
+
 class Sqlite3DSError(Exception):
     """ Define exceptions."""
     pass
@@ -48,39 +51,43 @@ def create(cur):
         row = cur.fetchone()
     except sqlite3.OperationalError:
         cur.execute("CREATE TABLE schema_version (version INTEGER NOT NULL)")
-        cur.execute("INSERT INTO schema_version VALUES (1)")
+        cur.execute("INSERT INTO schema_version VALUES (" +
+                    str(SCHEMA_VERSION) + ")")
         cur.execute("""CREATE TABLE zones (id INTEGER PRIMARY KEY,
-                    name STRING NOT NULL COLLATE NOCASE,
-                    rdclass STRING NOT NULL COLLATE NOCASE DEFAULT 'IN',
+                    name TEXT NOT NULL COLLATE NOCASE,
+                    rdclass TEXT NOT NULL COLLATE NOCASE DEFAULT 'IN',
                     dnssec BOOLEAN NOT NULL DEFAULT 0)""")
         cur.execute("CREATE INDEX zones_byname ON zones (name)")
         cur.execute("""CREATE TABLE records (id INTEGER PRIMARY KEY,
                     zone_id INTEGER NOT NULL,
-                    name STRING NOT NULL COLLATE NOCASE,
-                    rname STRING NOT NULL COLLATE NOCASE,
+                    name TEXT NOT NULL COLLATE NOCASE,
+                    rname TEXT NOT NULL COLLATE NOCASE,
                     ttl INTEGER NOT NULL,
-                    rdtype STRING NOT NULL COLLATE NOCASE,
-                    sigtype STRING COLLATE NOCASE,
-                    rdata STRING NOT NULL)""")
+                    rdtype TEXT NOT NULL COLLATE NOCASE,
+                    sigtype TEXT COLLATE NOCASE,
+                    rdata TEXT NOT NULL)""")
         cur.execute("CREATE INDEX records_byname ON records (name)")
         cur.execute("CREATE INDEX records_byrname ON records (rname)")
+        cur.execute("""CREATE INDEX records_bytype_and_rname ON records
+                       (rdtype, rname)""")
         cur.execute("""CREATE TABLE nsec3 (id INTEGER PRIMARY KEY,
                     zone_id INTEGER NOT NULL,
-                    hash STRING NOT NULL COLLATE NOCASE,
-                    owner STRING NOT NULL COLLATE NOCASE,
+                    hash TEXT NOT NULL COLLATE NOCASE,
+                    owner TEXT NOT NULL COLLATE NOCASE,
                     ttl INTEGER NOT NULL,
-                    rdtype STRING NOT NULL COLLATE NOCASE,
-                    rdata STRING NOT NULL)""")
+                    rdtype TEXT NOT NULL COLLATE NOCASE,
+                    rdata TEXT NOT NULL)""")
         cur.execute("CREATE INDEX nsec3_byhash ON nsec3 (hash)")
         cur.execute("""CREATE TABLE diffs (id INTEGER PRIMARY KEY,
                     zone_id INTEGER NOT NULL,
                     version INTEGER NOT NULL,
                     operation INTEGER NOT NULL,
-                    name STRING NOT NULL COLLATE NOCASE,
-                    rrtype STRING NOT NULL COLLATE NOCASE,
+                    name TEXT NOT NULL COLLATE NOCASE,
+                    rrtype TEXT NOT NULL COLLATE NOCASE,
                     ttl INTEGER NOT NULL,
-                    rdata STRING NOT NULL)""")
-        row = [1]
+                    rdata TEXT NOT NULL)""")
+        cur.execute("SELECT version FROM schema_version")
+        row = cur.fetchone()
     cur.execute("COMMIT TRANSACTION")
     return row
 
@@ -115,8 +122,9 @@ def open(dbfile, connect_timeout=5.0):
         row = create(cur)
         conn.isolation_level = iso_lvl
 
-    if row == None or row[0] != 1:
-        raise Sqlite3DSError("Bad database schema version")
+    if row == None or row[0] != SCHEMA_VERSION:
+        bad_version = "(unknown)" if row is None else str(row[0])
+        raise Sqlite3DSError("Bad database schema version: " + bad_version)
 
     return conn, cur