Use composition instead of inheritance for sql.Query

This means we're more loosely coupled to Qt's QSqlQuery, and also can move some
logic for handling batch queries from the table to there.

qutebrowser/completion/models/histcategory.py

@@ -84,7 +84,7 @@ class HistoryCategory(QSqlQueryModel):
         timefmt = ("strftime('{}', last_atime, 'unixepoch', 'localtime')"
                    .format(timestamp_format.replace("'", "`")))
 
-        if not self._query or len(words) != len(self._query.boundValues()):
+        if not self._query or len(words) != len(self._query.bound_values()):
             # if the number of words changed, we need to generate a new query
             # otherwise, we can reuse the prepared query for performance
             self._query = sql.Query(' '.join([
@@ -100,14 +100,14 @@ class HistoryCategory(QSqlQueryModel):
         with debug.log_time('sql', 'Running completion query'):
             self._query.run(**{
                 str(i): w for i, w in enumerate(words)})
-        self.setQuery(self._query)
+        self.setQuery(self._query.query)
 
     def removeRows(self, row, _count, _parent=None):
         """Override QAbstractItemModel::removeRows to re-run sql query."""
         # re-run query to reload updated table
         with debug.log_time('sql', 'Re-running completion query post-delete'):
             self._query.run()
-        self.setQuery(self._query)
+        self.setQuery(self._query.query)
         while self.rowCount() < row:
             self.fetchMore()
         return True

qutebrowser/misc/sql.py

@@ -139,7 +139,7 @@ def version():
         return 'UNAVAILABLE ({})'.format(e)
 
 
-class Query(QSqlQuery):
+class Query:
 
     """A prepared SQL Query."""
 
@@ -151,43 +151,68 @@ class Query(QSqlQuery):
             forward_only: Optimization for queries that will only step forward.
                           Must be false for completion queries.
         """
-        super().__init__(QSqlDatabase.database())
+        self.query = QSqlQuery(QSqlDatabase.database())
+
         log.sql.debug('Preparing SQL query: "{}"'.format(querystr))
-        if not self.prepare(querystr):
+        if not self.query.prepare(querystr):
-            raise SqliteError.from_query('prepare', querystr, self.lastError())
+            raise SqliteError.from_query('prepare', querystr,
-        self.setForwardOnly(forward_only)
+                                         self.query.lastError())
+        self.query.setForwardOnly(forward_only)
 
     def __iter__(self):
-        if not self.isActive():
+        if not self.query.isActive():
             raise SqlError("Cannot iterate inactive query")
-        rec = self.record()
+        rec = self.query.record()
         fields = [rec.fieldName(i) for i in range(rec.count())]
         rowtype = collections.namedtuple('ResultRow', fields)
 
-        while self.next():
+        while self.query.next():
-            rec = self.record()
+            rec = self.query.record()
             yield rowtype(*[rec.value(i) for i in range(rec.count())])
 
     def run(self, **values):
         """Execute the prepared query."""
-        log.sql.debug('Running SQL query: "{}"'.format(self.lastQuery()))
+        log.sql.debug('Running SQL query: "{}"'.format(
+            self.query.lastQuery()))
 
         for key, val in values.items():
-            self.bindValue(':{}'.format(key), val)
+            self.query.bindValue(':{}'.format(key), val)
-        log.sql.debug('query bindings: {}'.format(self.boundValues()))
+        log.sql.debug('query bindings: {}'.format(self.bound_values()))
-        if any(val is None for val in self.boundValues().values()):
+        if any(val is None for val in self.bound_values().values()):
             raise SqlError("Missing bound values!")
 
-        if not self.exec_():
+        if not self.query.exec_():
-            raise SqliteError.from_query('exec', self.lastQuery(),
+            raise SqliteError.from_query('exec', self.query.lastQuery(),
-                                         self.lastError())
+                                         self.query.lastError())
         return self
 
+    def run_batch(self, values):
+        """Execute the query in batch mode."""
+        log.sql.debug('Running SQL query (batch): "{}"'.format(
+            self.query.lastQuery()))
+
+        for key, val in values.items():
+            self.query.bindValue(':{}'.format(key), val)
+
+        db = QSqlDatabase.database()
+        db.transaction()
+        ok = self.query.execBatch()
+        if not ok:
+            raise SqliteError.from_query('exec', self.query.lastQuery(),
+                                         self.query.lastError())
+        db.commit()
+
     def value(self):
         """Return the result of a single-value query (e.g. an EXISTS)."""
-        if not self.next():
+        if not self.query.next():
             raise SqlError("No result for single-result query")
-        return self.record().value(0)
+        return self.query.record().value(0)
+
+    def rows_affected(self):
+        return self.query.numRowsAffected()
+
+    def bound_values(self):
+        return self.query.boundValues()
 
 
 class SqlTable(QObject):
@@ -270,7 +295,7 @@ class SqlTable(QObject):
         q = Query("DELETE FROM {table} where {field} = :val"
                   .format(table=self._name, field=field))
         q.run(val=value)
-        if not q.numRowsAffected():
+        if not q.rows_affected():
             raise KeyError('No row with {} = "{}"'.format(field, value))
         self.changed.emit()
 
@@ -300,14 +325,7 @@ class SqlTable(QObject):
             replace: If true, overwrite rows with a primary key match.
         """
         q = self._insert_query(values, replace)
-        for key, val in values.items():
+        q.run_batch(values)
-            q.bindValue(':{}'.format(key), val)
-
-        db = QSqlDatabase.database()
-        db.transaction()
-        if not q.execBatch():
-            raise SqliteError.from_query('exec', q.lastQuery(), q.lastError())
-        db.commit()
         self.changed.emit()
 
     def delete_all(self):

tests/unit/misc/test_sql.py

@@ -244,7 +244,7 @@ class TestSqlQuery:
     @pytest.mark.parametrize('forward_only', [True, False])
     def test_forward_only(self, forward_only):
         q = sql.Query('SELECT 0 WHERE 0', forward_only=forward_only)
-        assert q.isForwardOnly() == forward_only
+        assert q.query.isForwardOnly() == forward_only
 
     def test_iter_inactive(self):
         q = sql.Query('SELECT 0')