diff --git a/CMakeLists.txt b/CMakeLists.txt index 72766cdc..c9d53e41 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -43,6 +43,10 @@ set(COMMON_SRCS src/database/dbanalyzer.cpp src/database/dbanalyzer.h src/database/dbtree.cpp + src/database/pagedresultmodel.cpp + src/database/pagedresultmodel.h + src/database/sqldatabaseadapter.cpp + src/database/sqldatabaseadapter.h src/database/dbtree.h src/gui/highlighter.cpp src/gui/highlighter.h diff --git a/CONTEXT.md b/CONTEXT.md index 97782b21..5179c875 100644 --- a/CONTEXT.md +++ b/CONTEXT.md @@ -36,6 +36,10 @@ architecture, the modules, and the tests all refer to the same things. - **Tree** — renders a DatabaseInfo into the left-hand QTreeWidget. - **QueryExecutor** — runs a list of Queries against a Database, returns results. Pure logic, no widgets. +- **PagedResult** — a lazily fetched view over a Query's result set. Rows are + read in pages as they are scrolled into view, so a result set of any size can + be browsed without holding it in memory. Ordering is done by the Database, + not over the rows already fetched. - **QueryResultPresenter** — renders QueryExecutor output into the result area of the main window. Owns the scroll area and table views. - **SchemaExporter** — produces a `CREATE TABLE` script from a DatabaseInfo. diff --git a/src/database/dbexport.h b/src/database/dbexport.h index f5f4f946..50533a09 100644 --- a/src/database/dbexport.h +++ b/src/database/dbexport.h @@ -10,8 +10,8 @@ class DbExport { } protected: - [[nodiscard]] DatabaseInfo getDatabaseInfo() const { return info; } - [[nodiscard]] QStringList getTextTypes() const { return textTypes; } + [[nodiscard]] const DatabaseInfo &getDatabaseInfo() const { return info; } + [[nodiscard]] const QStringList &getTextTypes() const { return textTypes; } static bool isInternalTable(const Table &table); private: diff --git a/src/database/dbexportdata.cpp b/src/database/dbexportdata.cpp index a421fef7..1f5badc9 100644 --- a/src/database/dbexportdata.cpp +++ b/src/database/dbexportdata.cpp @@ -3,26 +3,37 @@ QStringList DbDataExport::getColumnDefs(const Table &table) { QStringList columnDefinitions; - for (const auto &column: table.columns) { + for (const auto &column : table.columns) { columnDefinitions.append(column.name); } return columnDefinitions; } -QStringList DbDataExport::getColumnValueDefs(const Table &table, - const QList &values) const { - QStringList valueDefinitions; - for (int i = 0; i < table.columns.size(); ++i) { - const auto &column = table.columns.at(i); - auto value = values.at(i).toString(); +// Whether a column holds text depends only on the schema, so this is resolved +// once per table rather than once per exported cell. +QList DbDataExport::getTextColumnFlags(const Table &table) const { + QList isTextColumn; + isTextColumn.reserve(table.columns.size()); + for (const auto &column : table.columns) { bool isText = false; - for (const auto &type: getTextTypes()) { + for (const auto &type : getTextTypes()) { if (column.dataType.contains(type, Qt::CaseInsensitive)) { isText = true; break; } } - if (isText) { + isTextColumn.append(isText); + } + return isTextColumn; +} + +QStringList DbDataExport::getColumnValueDefs(const QList &isTextColumn, + const QList &values) { + QStringList valueDefinitions; + valueDefinitions.reserve(isTextColumn.size()); + for (int i = 0; i < isTextColumn.size(); ++i) { + auto value = values.at(i).toString(); + if (isTextColumn.at(i)) { value = value.replace("\"", "\"\""); valueDefinitions.append(QString("\"%1\"").arg(value)); } else { @@ -44,19 +55,20 @@ void DbDataExport::exportDataToSqlFile(IDatabase *database, progress->reset(); QTextStream out(file.get()); - for (const auto &table: this->getDatabaseInfo().tables) { + for (const auto &table : this->getDatabaseInfo().tables) { if (isInternalTable(table)) { continue; } out << "-- " << table.name << "\n"; const auto columns = getColumnDefs(table).join(", "); + const auto isTextColumn = getTextColumnFlags(table); const QueryResult streamResult = database->streamRows( QString("SELECT * FROM \"%1\"").arg(table.name), [&](const QList &values) { if (cancellationToken->isCancellationRequested()) return false; - const auto valueList = getColumnValueDefs(table, values).join(", "); + const auto valueList = getColumnValueDefs(isTextColumn, values).join(", "); out << "INSERT INTO \"" << table.name << "\"(" << columns << ") "; out << "VALUES (" << valueList << ");\n"; progress->increment(); @@ -78,7 +90,7 @@ void DbDataExport::exportDataToCsvFile(IDatabase *database, const CancellationToken *cancellationToken, ExportDataProgress *progress) const { progress->reset(); - for (const auto &table: this->getDatabaseInfo().tables) { + for (const auto &table : this->getDatabaseInfo().tables) { if (isInternalTable(table) || cancellationToken->isCancellationRequested()) { continue; } @@ -90,6 +102,7 @@ void DbDataExport::exportDataToCsvFile(IDatabase *database, } const auto columns = getColumnDefs(table).join(delimiter); + const auto isTextColumn = getTextColumnFlags(table); QTextStream out(file.get()); out << columns << "\n"; @@ -98,7 +111,7 @@ void DbDataExport::exportDataToCsvFile(IDatabase *database, [&](const QList &values) { if (cancellationToken->isCancellationRequested()) return false; - const auto valueList = getColumnValueDefs(table, values).join(delimiter); + const auto valueList = getColumnValueDefs(isTextColumn, values).join(delimiter); out << valueList << "\n"; progress->increment(); return true; diff --git a/src/database/dbexportdata.h b/src/database/dbexportdata.h index 48ef8504..82ae4341 100644 --- a/src/database/dbexportdata.h +++ b/src/database/dbexportdata.h @@ -29,8 +29,10 @@ class DbDataExport : public DbExport { private: static QStringList getColumnDefs(const Table &table); - [[nodiscard]] QStringList getColumnValueDefs(const Table &table, - const QList &values) const; + [[nodiscard]] QList getTextColumnFlags(const Table &table) const; + + static QStringList getColumnValueDefs(const QList &isTextColumn, + const QList &values); }; #endif // DBDATAEXPORT_H diff --git a/src/database/dbtree.cpp b/src/database/dbtree.cpp index 899a2235..d4b63ab7 100644 --- a/src/database/dbtree.cpp +++ b/src/database/dbtree.cpp @@ -49,13 +49,13 @@ void DbTree::populateTree(const DatabaseInfo &info) { const auto tablesRootNodePtr = tablesRootNode.get(); this->tree->addTopLevelItem(tablesRootNode.release()); - foreach(Table table, info.tables) { + for (const auto &table : info.tables) { auto tableNode = std::make_unique(QTreeWidgetItem::UserType + 1); tableNode->setText(0, table.name); const auto tableNodePtr = tableNode.get(); tablesRootNodePtr->addChild(tableNode.release()); - foreach(Column col, table.columns) { + for (const auto &col : table.columns) { auto colName = std::make_unique(); colName->setText(0, col.name); const auto colNamePtr = colName.get(); diff --git a/src/database/idatabase.h b/src/database/idatabase.h index 19b45cb8..39ae1832 100644 --- a/src/database/idatabase.h +++ b/src/database/idatabase.h @@ -9,6 +9,8 @@ #include "queryresult.h" +class QAbstractItemModel; + class IDatabase { public: virtual ~IDatabase() = default; @@ -25,6 +27,16 @@ class IDatabase { virtual QueryResult runStatement(const QString &sql) = 0; + // A lazily fetched, pageable model over the statement's result set. Rows + // are read in pages as they are scrolled into view, so a result set of any + // size can be browsed. The caller owns the returned model. + // Returns nullptr when the statement produces no result set (an INSERT, + // say) or when it failed -- `error` then holds the reason, and is cleared + // otherwise. A SELECT matching no rows still returns a model, so that its + // columns can be displayed. + virtual QAbstractItemModel *createResultModel(const QString &sql, + QString *error = nullptr) = 0; + virtual QueryResult streamRows(const QString &sql, const std::function &)> &onRow) = 0; }; diff --git a/src/database/inmemorydatabase.cpp b/src/database/inmemorydatabase.cpp index 08c903eb..c547f693 100644 --- a/src/database/inmemorydatabase.cpp +++ b/src/database/inmemorydatabase.cpp @@ -1,8 +1,7 @@ #include "inmemorydatabase.h" + #include #include -#include -#include InMemoryDatabase::InMemoryDatabase() { database = QSqlDatabase::addDatabase("QSQLITE", "in_memory_connection"); @@ -21,11 +20,6 @@ bool InMemoryDatabase::open() { return database.open(); } -void InMemoryDatabase::close() { - if (database.isOpen()) - database.close(); -} - void InMemoryDatabase::shrink() { if (!database.isOpen()) return; @@ -33,73 +27,3 @@ void InMemoryDatabase::shrink() { QSqlQuery query(database); query.exec("VACUUM"); } - -QueryResult InMemoryDatabase::runStatement(const QString &sql) { - QueryResult result; - if (!database.isOpen()) { - result.ok = false; - result.error = "Database is not open"; - return result; - } - - QSqlQuery query(database); - if (!query.exec(sql)) { - result.ok = false; - result.error = query.lastError().text(); - return result; - } - - result.isSelect = query.isSelect(); - if (result.isSelect) { - const QSqlRecord record = query.record(); - for (int i = 0; i < record.count(); ++i) { - result.columns.append(record.fieldName(i)); - } - while (query.next()) { - QueryRow row; - row.values.reserve(record.count()); - for (int i = 0; i < record.count(); ++i) { - row.values.append(query.value(i)); - } - result.rows.append(row); - } - } else { - result.rowsAffected = query.numRowsAffected(); - } - return result; -} - -QueryResult InMemoryDatabase::streamRows(const QString &sql, - const std::function &)> &onRow) { - QueryResult result; - if (!database.isOpen()) { - result.ok = false; - result.error = "Database is not open"; - return result; - } - - QSqlQuery query(database); - query.setForwardOnly(true); - if (!query.exec(sql)) { - result.ok = false; - result.error = query.lastError().text(); - return result; - } - - result.isSelect = query.isSelect(); - const QSqlRecord record = query.record(); - for (int i = 0; i < record.count(); ++i) { - result.columns.append(record.fieldName(i)); - } - - while (query.next()) { - QList values; - values.reserve(record.count()); - for (int i = 0; i < record.count(); ++i) { - values.append(query.value(i)); - } - if (!onRow(values)) - break; - } - return result; -} diff --git a/src/database/inmemorydatabase.h b/src/database/inmemorydatabase.h index 011d2300..696ac4ba 100644 --- a/src/database/inmemorydatabase.h +++ b/src/database/inmemorydatabase.h @@ -1,12 +1,12 @@ #ifndef INMEMORYDATABASE_H #define INMEMORYDATABASE_H -#include #include -#include "idatabase.h" +#include "sqldatabaseadapter.h" -class InMemoryDatabase : public IDatabase { +// Test adapter: a SQLite database held in memory. +class InMemoryDatabase final : public SqlDatabaseAdapter { public: InMemoryDatabase(); @@ -14,22 +14,7 @@ class InMemoryDatabase : public IDatabase { bool open() override; - void close() override; - void shrink() override; - - [[nodiscard]] QString getFilename() const override { return source; } - - QueryResult runStatement(const QString &sql) override; - - QueryResult streamRows(const QString &sql, - const std::function &)> &onRow) override; - - [[nodiscard]] QSqlDatabase getConnection() const { return database; } - -private: - QSqlDatabase database; - QString source; }; #endif // INMEMORYDATABASE_H diff --git a/src/database/pagedresultmodel.cpp b/src/database/pagedresultmodel.cpp new file mode 100644 index 00000000..fb294e00 --- /dev/null +++ b/src/database/pagedresultmodel.cpp @@ -0,0 +1,54 @@ +#include "pagedresultmodel.h" + +#include +#include +#include + +#include + +PagedResultModel::PagedResultModel(const QSqlDatabase &database, QString sql, QObject *parent) + : QSqlQueryModel(parent), + database(database), + statement(std::move(sql)) { + run(statement); +} + +bool PagedResultModel::run(const QString &sql) { + QSqlQuery query(this->database); + if (!query.exec(sql)) { + this->error = query.lastError().text(); + return false; + } + + // QSqlQueryModel reads the first page here and the rest on demand. + setQuery(std::move(query)); + if (lastError().isValid()) { + this->error = lastError().text(); + return false; + } + + this->error.clear(); + return true; +} + +void PagedResultModel::sort(const int column, const Qt::SortOrder order) { + if (column < 0 || column >= columnCount()) + return; + + QString columnName = record().fieldName(column); + if (columnName.isEmpty()) + return; + + // A quoted identifier escapes a double quote by doubling it. + columnName.replace('"', "\"\""); + + const QString sorted = QString("SELECT * FROM (%1) ORDER BY \"%2\" %3") + .arg(statement, + columnName, + order == Qt::AscendingOrder ? "ASC" : "DESC"); + + // Not every statement can be wrapped in a subquery. run() leaves the model + // untouched when it fails, so there is nothing to restore -- and re-running + // the original would execute a statement with side effects a second time. + run(sorted); +} diff --git a/src/database/pagedresultmodel.h b/src/database/pagedresultmodel.h new file mode 100644 index 00000000..810b685d --- /dev/null +++ b/src/database/pagedresultmodel.h @@ -0,0 +1,31 @@ +#ifndef PAGEDRESULTMODEL_H +#define PAGEDRESULTMODEL_H + +#include +#include +#include + +// A lazily fetched view over a Query's result set. Rows are read in pages as +// they are scrolled into view, so a result set of any size can be browsed +// without holding it in memory. Sorting re-runs the statement with an ORDER BY +// so that the database does the ordering rather than the rows already fetched. +class PagedResultModel final : public QSqlQueryModel { + Q_OBJECT + +public: + PagedResultModel(const QSqlDatabase &database, QString sql, QObject *parent = nullptr); + + // Empty unless the statement failed to execute. + [[nodiscard]] QString errorText() const { return error; } + + void sort(int column, Qt::SortOrder order) override; + +private: + bool run(const QString &sql); + + QSqlDatabase database; + QString statement; + QString error; +}; + +#endif // PAGEDRESULTMODEL_H diff --git a/src/database/queryexecutor.cpp b/src/database/queryexecutor.cpp index 1d3adeb8..572a9e30 100644 --- a/src/database/queryexecutor.cpp +++ b/src/database/queryexecutor.cpp @@ -1,5 +1,13 @@ #include "queryexecutor.h" +namespace { + // A quoted identifier escapes a double quote by doubling it. + QString quoteIdentifier(const QString &name) { + QString quoted = name; + return quoted.replace('"', "\"\""); + } +} + QueryExecutor::QueryExecutor(IDatabase *database) : database(database) { } @@ -9,10 +17,15 @@ QList QueryExecutor::runScript(const QString &script, QStringList * return runStatements(raw, errors); } -QList QueryExecutor::runStatements(const QStringList &statements, QStringList *errors) { +QList QueryExecutor::runStatements(const QStringList &statements, + QStringList *errors) { QList results; - for (const auto &raw: statements) { - const QString sql = raw.trimmed().replace('\n', "", Qt::CaseInsensitive); + results.reserve(statements.size()); + for (const auto &raw : statements) { + // Newlines are left alone: SQLite treats them as whitespace, and + // stripping them would merge tokens across lines and swallow the rest + // of a line after a `--` comment. + const QString sql = raw.trimmed(); if (sql.isEmpty()) continue; @@ -21,15 +34,39 @@ QList QueryExecutor::runStatements(const QStringList &statements, Q if (errors != nullptr) errors->append(result.error.isEmpty() ? QString("Query failed") : result.error); } - results.append(result); + results.append(std::move(result)); } return results; } QueryResult QueryExecutor::previewTable(const QString &tableName, int limit) const { - QString sql = QString("SELECT * FROM \"%1\"").arg(tableName); + QString sql = QString("SELECT * FROM \"%1\"").arg(quoteIdentifier(tableName)); if (limit > 0) { sql += QString(" LIMIT %1").arg(limit); } return database->runStatement(sql); } + +QList QueryExecutor::runStatementsPaged(const QStringList &statements, + QStringList *errors) const { + QList models; + for (const auto &raw : statements) { + const QString sql = raw.trimmed(); + if (sql.isEmpty()) + continue; + + QString error; + // Statements that return no rows still run; they just have nothing to show. + if (auto *model = database->createResultModel(sql, &error)) { + models.append(model); + } else if (!error.isEmpty() && errors != nullptr) { + errors->append(error); + } + } + return models; +} + +QAbstractItemModel *QueryExecutor::previewTablePaged(const QString &tableName, + QString *error) const { + return database->createResultModel(QString("SELECT * FROM \"%1\"").arg(quoteIdentifier(tableName)), error); +} diff --git a/src/database/queryexecutor.h b/src/database/queryexecutor.h index 1e0509a9..eb84ba72 100644 --- a/src/database/queryexecutor.h +++ b/src/database/queryexecutor.h @@ -8,16 +8,28 @@ #include "idatabase.h" #include "queryresult.h" +class QAbstractItemModel; + class QueryExecutor { public: explicit QueryExecutor(IDatabase *database); QList runScript(const QString &script, QStringList *errors = nullptr); - QList runStatements(const QStringList &statements, QStringList *errors = nullptr); + QList runStatements(const QStringList &statements, + QStringList *errors = nullptr); [[nodiscard]] QueryResult previewTable(const QString &tableName, int limit = -1) const; + // Display variants. Each returns a lazily fetched model whose rows are read + // in pages as they are scrolled into view, so a result set of any size can + // be browsed. The caller owns the returned models. + QList runStatementsPaged(const QStringList &statements, + QStringList *errors = nullptr) const; + + [[nodiscard]] QAbstractItemModel *previewTablePaged(const QString &tableName, + QString *error = nullptr) const; + private: IDatabase *database; }; diff --git a/src/database/sqldatabaseadapter.cpp b/src/database/sqldatabaseadapter.cpp new file mode 100644 index 00000000..7f7f6189 --- /dev/null +++ b/src/database/sqldatabaseadapter.cpp @@ -0,0 +1,115 @@ +#include "sqldatabaseadapter.h" + +#include +#include +#include + +#include + +#include "pagedresultmodel.h" + +void SqlDatabaseAdapter::close() { + if (database.isOpen()) + database.close(); +} + +QueryResult SqlDatabaseAdapter::runStatement(const QString &sql) { + QueryResult result; + if (!database.isOpen()) { + result.ok = false; + result.error = "Database is not open"; + return result; + } + + QSqlQuery query(database); + if (!query.exec(sql)) { + result.ok = false; + result.error = query.lastError().text(); + return result; + } + + result.isSelect = query.isSelect(); + if (result.isSelect) { + const QSqlRecord record = query.record(); + const int columnCount = record.count(); + result.columns.reserve(columnCount); + for (int i = 0; i < columnCount; ++i) { + result.columns.append(record.fieldName(i)); + } + while (query.next()) { + QueryRow row; + row.values.reserve(columnCount); + for (int i = 0; i < columnCount; ++i) { + row.values.append(query.value(i)); + } + result.rows.append(std::move(row)); + } + } else { + result.rowsAffected = query.numRowsAffected(); + } + return result; +} + +QAbstractItemModel *SqlDatabaseAdapter::createResultModel(const QString &sql, QString *error) { + // Cleared up front so a caller reusing one QString never sees a stale error + // on the paths that return nullptr without failing. + if (error != nullptr) + error->clear(); + + if (!database.isOpen()) { + if (error != nullptr) + *error = "Database is not open"; + return nullptr; + } + + auto model = std::make_unique(database, sql); + if (!model->errorText().isEmpty()) { + if (error != nullptr) + *error = model->errorText(); + return nullptr; + } + + // The statement ran but returns no rows, so there is nothing to display. + if (model->columnCount() == 0) + return nullptr; + + return model.release(); +} + +QueryResult SqlDatabaseAdapter::streamRows(const QString &sql, + const std::function &)> &onRow) { + QueryResult result; + if (!database.isOpen()) { + result.ok = false; + result.error = "Database is not open"; + return result; + } + + QSqlQuery query(database); + query.setForwardOnly(true); + if (!query.exec(sql)) { + result.ok = false; + result.error = query.lastError().text(); + return result; + } + + result.isSelect = query.isSelect(); + const QSqlRecord record = query.record(); + const int columnCount = record.count(); + result.columns.reserve(columnCount); + for (int i = 0; i < columnCount; ++i) { + result.columns.append(record.fieldName(i)); + } + + QList values; + values.reserve(columnCount); + while (query.next()) { + values.clear(); + for (int i = 0; i < columnCount; ++i) { + values.append(query.value(i)); + } + if (!onRow(values)) + break; + } + return result; +} diff --git a/src/database/sqldatabaseadapter.h b/src/database/sqldatabaseadapter.h new file mode 100644 index 00000000..db95b24a --- /dev/null +++ b/src/database/sqldatabaseadapter.h @@ -0,0 +1,33 @@ +#ifndef SQLDATABASEADAPTER_H +#define SQLDATABASEADAPTER_H + +#include +#include + +#include "idatabase.h" + +// Shared base for the IDatabase adapters. They differ only in how their +// connection is named, created and opened; everything that reads from an open +// connection is the same for all of them and lives here. +class SqlDatabaseAdapter : public IDatabase { +public: + void close() override; + + [[nodiscard]] QString getFilename() const override { return source; } + + QueryResult runStatement(const QString &sql) override; + + QAbstractItemModel *createResultModel(const QString &sql, + QString *error = nullptr) override; + + QueryResult streamRows(const QString &sql, + const std::function &)> &onRow) override; + + [[nodiscard]] QSqlDatabase getConnection() const { return database; } + +protected: + QSqlDatabase database; + QString source; +}; + +#endif // SQLDATABASEADAPTER_H diff --git a/src/database/sqlitedatabase.cpp b/src/database/sqlitedatabase.cpp index 679c020a..f9ed156f 100644 --- a/src/database/sqlitedatabase.cpp +++ b/src/database/sqlitedatabase.cpp @@ -1,8 +1,7 @@ #include "sqlitedatabase.h" + #include -#include #include -#include SqliteDatabase::SqliteDatabase() { database = QSqlDatabase::addDatabase("QSQLITE"); @@ -20,11 +19,6 @@ bool SqliteDatabase::open() { return database.open(); } -void SqliteDatabase::close() { - if (database.isOpen()) - database.close(); -} - void SqliteDatabase::shrink() { if (!database.isOpen()) database.open(); @@ -32,73 +26,3 @@ void SqliteDatabase::shrink() { QSqlQuery query(database); query.exec("VACUUM"); } - -QueryResult SqliteDatabase::runStatement(const QString &sql) { - QueryResult result; - if (!database.isOpen()) { - result.ok = false; - result.error = "Database is not open"; - return result; - } - - QSqlQuery query(database); - if (!query.exec(sql)) { - result.ok = false; - result.error = query.lastError().text(); - return result; - } - - result.isSelect = query.isSelect(); - if (result.isSelect) { - const QSqlRecord record = query.record(); - for (int i = 0; i < record.count(); ++i) { - result.columns.append(record.fieldName(i)); - } - while (query.next()) { - QueryRow row; - row.values.reserve(record.count()); - for (int i = 0; i < record.count(); ++i) { - row.values.append(query.value(i)); - } - result.rows.append(row); - } - } else { - result.rowsAffected = query.numRowsAffected(); - } - return result; -} - -QueryResult SqliteDatabase::streamRows(const QString &sql, - const std::function &)> &onRow) { - QueryResult result; - if (!database.isOpen()) { - result.ok = false; - result.error = "Database is not open"; - return result; - } - - QSqlQuery query(database); - query.setForwardOnly(true); - if (!query.exec(sql)) { - result.ok = false; - result.error = query.lastError().text(); - return result; - } - - result.isSelect = query.isSelect(); - const QSqlRecord record = query.record(); - for (int i = 0; i < record.count(); ++i) { - result.columns.append(record.fieldName(i)); - } - - while (query.next()) { - QList values; - values.reserve(record.count()); - for (int i = 0; i < record.count(); ++i) { - values.append(query.value(i)); - } - if (!onRow(values)) - break; - } - return result; -} diff --git a/src/database/sqlitedatabase.h b/src/database/sqlitedatabase.h index ae366eee..1280db22 100644 --- a/src/database/sqlitedatabase.h +++ b/src/database/sqlitedatabase.h @@ -1,12 +1,12 @@ #ifndef SQLITEDATABASE_H #define SQLITEDATABASE_H -#include #include -#include "idatabase.h" +#include "sqldatabaseadapter.h" -class SqliteDatabase : public IDatabase { +// Production adapter: a SQLite database file on disk. +class SqliteDatabase final : public SqlDatabaseAdapter { public: SqliteDatabase(); @@ -14,22 +14,7 @@ class SqliteDatabase : public IDatabase { bool open() override; - void close() override; - void shrink() override; - - [[nodiscard]] QString getFilename() const override { return source; } - - QueryResult runStatement(const QString &sql) override; - - QueryResult streamRows(const QString &sql, - const std::function &)> &onRow) override; - - [[nodiscard]] QSqlDatabase getConnection() const { return database; } - -private: - QSqlDatabase database; - QString source; }; #endif // SQLITEDATABASE_H diff --git a/src/gui/exportorchestrator.cpp b/src/gui/exportorchestrator.cpp index 32346c25..165e9584 100644 --- a/src/gui/exportorchestrator.cpp +++ b/src/gui/exportorchestrator.cpp @@ -1,5 +1,6 @@ #include "exportorchestrator.h" #include "../database/dbexportdata.h" +#include #include #include @@ -11,6 +12,13 @@ ExportOrchestrator::ExportOrchestrator(QObject *parent) ExportOrchestrator::~ExportOrchestrator() { cancel(); + + // Both tasks read members of this object, so neither may outlive it. cancel() + // above makes them stop at their next check rather than at the end of the + // export. Queued progress callbacks are guarded separately by a QPointer. + completed_ = true; + exportTask_.waitForFinished(); + pollingTask_.waitForFinished(); } void ExportOrchestrator::exportToSql(DatabaseInfo info, QString filePath, IDatabase *db) { @@ -36,42 +44,53 @@ void ExportOrchestrator::cancel() { } void ExportOrchestrator::onExportComplete() { - uint64_t rows = progress_->getAffectedRows(); - progress_.release(); - tcs_.release(); + const uint64_t rows = progress_->getAffectedRows(); completed_ = true; + progress_.reset(); + tcs_.reset(); emit exportCompleted(rows); } void ExportOrchestrator::startProgressPolling(CancellationToken token) { - auto _ = QtConcurrent::run([this, token]() { + // The polling thread outlives the export, so it keeps the progress and + // cancellation state alive through shared ownership instead of raw pointers. + auto progress = progress_; + auto tcs = tcs_; + const QPointer self(this); + pollingTask_ = QtConcurrent::run([this, self, token, progress, tcs]() { do { std::this_thread::sleep_for(std::chrono::milliseconds(100)); if (completed_) break; - MainThread::run([this]() { - emit exportProgress(progress_->getAffectedRows()); + const uint64_t rows = progress->getAffectedRows(); + // Queued onto the GUI thread, so it can run after this object is + // gone; QPointer clears itself when that happens. + MainThread::run([self, rows]() { + if (!self.isNull()) + emit self->exportProgress(rows); }); } while (!token.isCancellationRequested() && - !progress_->isCompleted()); + !progress->isCompleted()); }); } void ExportOrchestrator::runExport(IDatabase *db, DatabaseInfo info, std::function fn) { - progress_ = std::make_unique(); - tcs_ = std::make_unique(); + progress_ = std::make_shared(); + tcs_ = std::make_shared(); completed_ = false; CancellationToken token = tcs_->get(); ExportDataProgress *progressPtr = progress_.get(); - auto future = QtConcurrent::run([db, info, fn, token, progressPtr]() { - fn(db, info, const_cast(&token), progressPtr); + exportTask_ = QtConcurrent::run([db, info = std::move(info), fn = std::move(fn), token, progressPtr]() { + fn(db, info, &token, progressPtr); }); - future.then([this]() { - MainThread::run([this]() { - this->onExportComplete(); + const QPointer self(this); + exportTask_.then([self]() { + MainThread::run([self]() { + if (!self.isNull()) + self->onExportComplete(); }); }); diff --git a/src/gui/exportorchestrator.h b/src/gui/exportorchestrator.h index ad4441be..f4017c8f 100644 --- a/src/gui/exportorchestrator.h +++ b/src/gui/exportorchestrator.h @@ -2,6 +2,7 @@ #define EXPORTORCHESTRATOR_H #include +#include #include #include #include @@ -39,8 +40,12 @@ private slots: void runExport(IDatabase *db, DatabaseInfo info, std::function fn); - std::unique_ptr progress_; - std::unique_ptr tcs_; + // Held so the destructor can wait for both tasks to stop touching this + // object rather than letting them outlive it. + QFuture exportTask_; + QFuture pollingTask_; + std::shared_ptr progress_; + std::shared_ptr tcs_; std::atomic completed_{false}; }; diff --git a/src/gui/mainwindow.cpp b/src/gui/mainwindow.cpp index 3e1a131e..b874afec 100644 --- a/src/gui/mainwindow.cpp +++ b/src/gui/mainwindow.cpp @@ -32,13 +32,14 @@ MainWindow::MainWindow(QWidget *parent) : this->sessionManager = std::make_unique(this); this->exportOrchestrator = std::make_unique(this); + connect(exportOrchestrator.get(), &ExportOrchestrator::exportProgress, + this, &MainWindow::onExportProgress); + connect(exportOrchestrator.get(), &ExportOrchestrator::exportCompleted, + this, &MainWindow::onExportCompleted); this->recentFilesMenu = std::make_unique("Recent Files"); ui->menuFile->insertMenu(ui->actionSave, recentFilesMenu.get()); - this->statusBar = std::make_unique(this); - this->setStatusBar(statusBar.get()); - sessionManager->init(); sessionManager->loadRecentFiles(recentFilesMenu.get(), this); restoreWindowState(); @@ -211,7 +212,9 @@ void MainWindow::saveWindowState(const QSize &size) const { } void MainWindow::resizeEvent(QResizeEvent *e) { - saveWindowState(e->size()); + // Deliberately does not persist here: a drag-resize delivers events at + // screen refresh rate, and the window state is written on exit anyway. + QMainWindow::resizeEvent(e); } void MainWindow::openRecentFile() { @@ -237,12 +240,8 @@ void MainWindow::saveSession() const { } void MainWindow::createNewFile() { - if (exportOrchestrator->isExporting()) { - const auto msg = "Unable to process request. Data export in progress"; - this->statusBar->showMessage(msg, 5000); - ui->queryResultTab->setCurrentIndex(1); + if (blockedByExport()) return; - } const QString filepath = Prompts::getFilePath(this, QFileDialog::AcceptSave); this->openDatabase(filepath); @@ -250,12 +249,8 @@ void MainWindow::createNewFile() { } void MainWindow::openDatabase(const QString &filename) { - if (exportOrchestrator->isExporting()) { - const auto msg = "Unable to process request. Data export in progress"; - this->statusBar->showMessage(msg, 5000); - ui->queryResultTab->setCurrentIndex(1); + if (blockedByExport()) return; - } if (!this->database->getFilename().isEmpty()) { this->queryPresenter->clearResults(); @@ -277,12 +272,8 @@ void MainWindow::openDatabase(const QString &filename) { } void MainWindow::openExistingFile() { - if (exportOrchestrator->isExporting()) { - const auto msg = "Unable to process request. Data export in progress"; - this->statusBar->showMessage(msg, 5000); - ui->queryResultTab->setCurrentIndex(1); + if (blockedByExport()) return; - } const auto filepath = Prompts::getFilePath(this, QFileDialog::AcceptOpen); this->openDatabase(filepath); sessionManager->addRecentFile(filepath); @@ -295,12 +286,8 @@ void MainWindow::appExit() const { } void MainWindow::shrink() const { - if (exportOrchestrator->isExporting()) { - const auto msg = "Unable to process request. Data export in progress"; - this->statusBar->showMessage(msg, 5000); - ui->queryResultTab->setCurrentIndex(1); + if (blockedByExport()) return; - } if (const QString filename = this->database->getFilename(); filename.isNull() || filename.isEmpty()) return; @@ -314,12 +301,8 @@ void MainWindow::refreshDatabase() const { } void MainWindow::analyzeDatabase() const { - if (exportOrchestrator->isExporting()) { - const auto msg = "Unable to process request. Data export in progress"; - this->statusBar->showMessage(msg, 5000); - ui->queryResultTab->setCurrentIndex(1); + if (blockedByExport()) return; - } DatabaseInfo info; if (!analyzer->analyze(info)) { @@ -327,19 +310,13 @@ void MainWindow::analyzeDatabase() const { } this->tree->populateTree(info); - - this->database->close(); } void MainWindow::executeQuery() const { - if (exportOrchestrator->isExporting()) { - const auto msg = "Unable to process request. Data export in progress"; - this->statusBar->showMessage(msg, 5000); - ui->queryResultTab->setCurrentIndex(1); + if (blockedByExport()) return; - } - QStringList list(ui->textEdit->toPlainText().split(";", Qt::SkipEmptyParts)); + const QStringList list(ui->textEdit->toPlainText().split(";", Qt::SkipEmptyParts)); QStringList errors; QElapsedTimer time; @@ -352,9 +329,19 @@ void MainWindow::executeQuery() const { const auto milliseconds = static_cast(time.elapsed()); const auto msg = "Query execution took " + QString::number(milliseconds / 1000) + " seconds"; - this->showMessage(msg); - foreach(const QString sql, list) { + if (errors.isEmpty()) { + this->showMessage(msg); + } else { + // Report what failed instead of overwriting it with the timing. + ui->queryResultMessagesTextEdit->setPlainText(errors.join("\r\n")); + ui->tabWidget->setCurrentIndex(0); + ui->queryResultTab->setCurrentIndex(1); + this->statusBar()->showMessage( + QString("Query failed with %1 error(s)").arg(errors.size()), 5000); + } + + for (const auto &sql : list) { if (sql.contains("create", Qt::CaseInsensitive) || sql.contains("drop", Qt::CaseInsensitive) || sql.contains("insert", Qt::CaseInsensitive) || @@ -396,11 +383,6 @@ void MainWindow::exportDataToSqlScript() { ui->queryResultTab->setCurrentIndex(1); exportOrchestrator->exportToSql(std::move(info), filepath, database.get()); - - connect(exportOrchestrator.get(), &ExportOrchestrator::exportProgress, - this, &MainWindow::onExportProgress); - connect(exportOrchestrator.get(), &ExportOrchestrator::exportCompleted, - this, &MainWindow::onExportCompleted); } void MainWindow::exportDataToCsvFiles() { @@ -418,11 +400,6 @@ void MainWindow::exportDataToCsvFiles() { ui->queryResultTab->setCurrentIndex(1); exportOrchestrator->exportToCsv(std::move(info), outputFolder, delimiter, database.get()); - - connect(exportOrchestrator.get(), &ExportOrchestrator::exportProgress, - this, &MainWindow::onExportProgress); - connect(exportOrchestrator.get(), &ExportOrchestrator::exportCompleted, - this, &MainWindow::onExportCompleted); } void MainWindow::cancel() const { @@ -463,9 +440,16 @@ void MainWindow::treeNodeChanged(QTreeWidgetItem *item, type() == QTreeWidgetItem::UserType + 1 ) { const QString tableName = item->text(column); - QueryResult result = this->executor->previewTable(tableName); + QString error; + auto *model = this->executor->previewTablePaged(tableName, &error); + + if (model == nullptr) { + this->showMessage(error.isEmpty() ? "Unable to read " + tableName : error); + ui->queryResultTab->setCurrentIndex(1); + return; + } - this->queryPresenter->presentToView(ui->tableView, result); + this->queryPresenter->presentToView(ui->tableView, model); ui->tabWidget->setCurrentIndex(1); } } @@ -481,9 +465,18 @@ void MainWindow::about() { QMessageBox::about(this, "About", text); } +bool MainWindow::blockedByExport() const { + if (!exportOrchestrator->isExporting()) + return false; + + this->statusBar()->showMessage("Unable to process request. Data export in progress", 5000); + ui->queryResultTab->setCurrentIndex(1); + return true; +} + void MainWindow::showMessage(const QString &message) const { ui->queryResultMessagesTextEdit->setPlainText(message); - this->statusBar->showMessage(message, 5000); + this->statusBar()->showMessage(message, 5000); } void MainWindow::onExportProgress(uint64_t rowsExported) { diff --git a/src/gui/mainwindow.h b/src/gui/mainwindow.h index 516a2c79..cc9cc0a6 100644 --- a/src/gui/mainwindow.h +++ b/src/gui/mainwindow.h @@ -80,7 +80,6 @@ public slots: private: std::unique_ptr ui; std::unique_ptr recentFilesMenu; - std::unique_ptr statusBar; std::unique_ptr database; std::unique_ptr analyzer; std::unique_ptr executor; @@ -100,6 +99,10 @@ public slots: void restoreWindowState(); void showMessage(const QString &message) const; + + // Reports that an export is in progress and returns true when it is, so + // callers can bail out with a single guard. + [[nodiscard]] bool blockedByExport() const; }; #endif // MAINWINDOW_H diff --git a/src/gui/queryexecutionpresenter.cpp b/src/gui/queryexecutionpresenter.cpp index 892649d5..2a16cd6f 100644 --- a/src/gui/queryexecutionpresenter.cpp +++ b/src/gui/queryexecutionpresenter.cpp @@ -6,8 +6,7 @@ QueryExecutionPresenter::QueryExecutionPresenter(QWidget *parent, QueryExecutor } bool QueryExecutionPresenter::execute(const QStringList &statements, QStringList *errors) { - const QList results = executor->runStatements(statements, errors); - presenter->present(results); + presenter->present(executor->runStatementsPaged(statements, errors)); return errors == nullptr || errors->isEmpty(); } @@ -15,6 +14,6 @@ void QueryExecutionPresenter::clearResults() { presenter->clear(); } -void QueryExecutionPresenter::presentToView(QTableView *view, const QueryResult &result) { - presenter->presentToView(view, result); +void QueryExecutionPresenter::presentToView(QTableView *view, QAbstractItemModel *model) { + presenter->presentToView(view, model); } diff --git a/src/gui/queryexecutionpresenter.h b/src/gui/queryexecutionpresenter.h index bf105b70..fe62f403 100644 --- a/src/gui/queryexecutionpresenter.h +++ b/src/gui/queryexecutionpresenter.h @@ -18,7 +18,7 @@ class QueryExecutionPresenter { void clearResults(); - void presentToView(QTableView *view, const QueryResult &result); + void presentToView(QTableView *view, QAbstractItemModel *model); private: QWidget *widget; diff --git a/src/gui/queryresultpresenter.cpp b/src/gui/queryresultpresenter.cpp index 62dff2b2..04a654ae 100644 --- a/src/gui/queryresultpresenter.cpp +++ b/src/gui/queryresultpresenter.cpp @@ -1,13 +1,17 @@ #include "queryresultpresenter.h" -#include +#include + +#include +#include +#include QueryResultPresenter::QueryResultPresenter(QWidget *parent) : widget(parent) { - this->container = std::make_unique(this->widget); + this->container = new QWidget(this->widget); this->container->hide(); - this->scrollArea = std::make_unique(this->widget); - this->scrollArea->setWidget(container.get()); + this->scrollArea = new QScrollArea(this->widget); + this->scrollArea->setWidget(container); this->scrollArea->hide(); } @@ -22,7 +26,14 @@ void QueryResultPresenter::clear() { this->tableViews.clear(); } -void QueryResultPresenter::present(const QList &results) { +void QueryResultPresenter::present(const QList &models) { + // Discard the views (and the models they own) from the previous execution, + // but only once there is something to replace them with: a statement that + // renders nothing leaves the previous results on screen. + if (models.isEmpty()) + return; + this->clear(); + const QRect widgetRect = this->widget->geometry(); const int width = widgetRect.width(); const int height = widgetRect.height(); @@ -30,31 +41,16 @@ void QueryResultPresenter::present(const QList &results) { int yOffset = 0; int count = 0; - for (const auto &result: results) { - if (!result.isSelect) - continue; - + for (auto *model: models) { if (count > 0) yOffset += height; - auto model = std::make_unique( - static_cast(result.rows.size()), - static_cast(result.columns.size()), - container.get()); - - for (int col = 0; col < result.columns.size(); ++col) { - model->setHorizontalHeaderItem(col, new QStandardItem(result.columns.at(col))); - } - for (int row = 0; row < result.rows.size(); ++row) { - const auto &values = result.rows.at(row).values; - for (int col = 0; col < result.columns.size(); ++col) { - auto *item = new QStandardItem(values.at(col).toString()); - model->setItem(row, col, item); - } - } - - const auto tablePtr = new QTableView(container.get()); - tablePtr->setModel(model.release()); + const auto tablePtr = new QTableView(container); + + // Parent the model to its view so it is destroyed along with the view. + model->setParent(tablePtr); + tablePtr->setModel(model); + tablePtr->setSortingEnabled(true); tablePtr->setGeometry(QRect(0, yOffset, width, height)); tablePtr->show(); this->tableViews.append(tablePtr); @@ -66,23 +62,19 @@ void QueryResultPresenter::present(const QList &results) { this->container->setGeometry(newParentRect); } -void QueryResultPresenter::presentToView(QTableView *view, const QueryResult &result) { - auto model = std::make_unique( - static_cast(result.rows.size()), - static_cast(result.columns.size()), - view); +void QueryResultPresenter::presentToView(QTableView *view, QAbstractItemModel *model) { + if (model == nullptr) + return; - for (int col = 0; col < result.columns.size(); ++col) { - model->setHorizontalHeaderItem(col, new QStandardItem(result.columns.at(col))); - } - for (int row = 0; row < result.rows.size(); ++row) { - const auto &values = result.rows.at(row).values; - for (int col = 0; col < result.columns.size(); ++col) { - auto *item = new QStandardItem(values.at(col).toString()); - model->setItem(row, col, item); - } - } + // setModel() does not delete the outgoing model, and only clears its + // selection model in some cases, so drop whatever survives to stop them + // accumulating on the shared view. QPointer guards against a double delete. + const QPointer previousModel = view->model(); + const QPointer previousSelection = view->selectionModel(); - view->setModel(model.release()); + model->setParent(view); + view->setModel(model); + delete previousSelection.data(); + delete previousModel.data(); view->setSortingEnabled(true); } diff --git a/src/gui/queryresultpresenter.h b/src/gui/queryresultpresenter.h index 5a027d4b..ecc5f3c4 100644 --- a/src/gui/queryresultpresenter.h +++ b/src/gui/queryresultpresenter.h @@ -7,7 +7,7 @@ #include #include -#include "../database/queryresult.h" +class QAbstractItemModel; class QueryResultPresenter { public: @@ -15,14 +15,17 @@ class QueryResultPresenter { void clear(); - void present(const QList &results); + // Takes ownership of the models and binds each to its own table view. + void present(const QList &models); - void presentToView(QTableView *view, const QueryResult &result); + // Takes ownership of the model and binds it to the given view. + void presentToView(QTableView *view, QAbstractItemModel *model); private: QWidget *widget; - std::unique_ptr scrollArea; - std::unique_ptr container; + // Owned by the parent widget, as with every other widget in the tree. + QScrollArea *scrollArea; + QWidget *container; QList tableViews{}; }; diff --git a/src/settings/recentfiles.cpp b/src/settings/recentfiles.cpp index b0268f52..d93b5164 100644 --- a/src/settings/recentfiles.cpp +++ b/src/settings/recentfiles.cpp @@ -34,7 +34,7 @@ void RecentFiles::add(const QString &filepath) { } QTextStream out(file.get()); - for (const auto &path: files) { + for (const auto &path : files) { out << path << "\n"; } @@ -42,7 +42,7 @@ void RecentFiles::add(const QString &filepath) { } void RecentFiles::clear() { - QFile(getRecentsFilePath()).deleteLater(); + QFile::remove(getRecentsFilePath()); } QStringList RecentFiles::getList() { diff --git a/tests/CMakeLists.txt b/tests/CMakeLists.txt index 391a0c14..b2003312 100644 --- a/tests/CMakeLists.txt +++ b/tests/CMakeLists.txt @@ -4,7 +4,7 @@ project(SQLiteQueryTests LANGUAGES CXX) set(CMAKE_CXX_STANDARD 17) set(CMAKE_CXX_STANDARD_REQUIRED ON) -find_package(Qt6 REQUIRED COMPONENTS Core Sql) +find_package(Qt6 REQUIRED COMPONENTS Core Sql Widgets) # Fetch Google Test include(FetchContent) @@ -22,6 +22,8 @@ set(TEST_SRCS test_dbexportdata.cpp test_dbexportschema.cpp test_exportorchestrator.cpp + test_queryresultpresenter.cpp + test_pagedresultmodel.cpp test_main.cpp ) @@ -31,6 +33,8 @@ add_executable(${PROJECT_NAME} ${TEST_SRCS}) set(PROD_SRCS ../src/database/sqlitedatabase.cpp ../src/database/inmemorydatabase.cpp + ../src/database/pagedresultmodel.cpp + ../src/database/sqldatabaseadapter.cpp ../src/database/queryexecutor.cpp ../src/database/dbanalyzer.cpp ../src/database/dbexportdata.cpp @@ -39,6 +43,7 @@ set(PROD_SRCS ../src/database/progress.h ../src/threading/cancellation.cpp ../src/gui/exportorchestrator.cpp + ../src/gui/queryresultpresenter.cpp ) target_sources(${PROJECT_NAME} PRIVATE ${PROD_SRCS}) @@ -47,6 +52,7 @@ target_link_libraries(${PROJECT_NAME} PRIVATE Qt6::Core Qt6::Gui Qt6::Sql + Qt6::Widgets GTest::gtest_main GTest::gtest ) diff --git a/tests/test_dbanalyzer.cpp b/tests/test_dbanalyzer.cpp index 92c0363e..2c4d0b2b 100644 --- a/tests/test_dbanalyzer.cpp +++ b/tests/test_dbanalyzer.cpp @@ -155,3 +155,17 @@ TEST_F(DbAnalyzerTest, AnalyzeWithPrimaryKey) { ASSERT_NE(idCol, nullptr); EXPECT_TRUE(idCol->primaryKey); } + +// The analyzer must leave the database usable: the window analyses a database +// immediately after opening it, and then runs the user's queries against it. +TEST_F(DbAnalyzerTest, AnalyzeLeavesDatabaseUsable) { + DatabaseInfo info; + ASSERT_TRUE(analyzer->analyze(info)); + + runSql("INSERT INTO users (name, age) VALUES ('Alice', 30)"); + + QueryExecutor executor(db.get()); + const QueryResult preview = executor.previewTable("users"); + EXPECT_TRUE(preview.ok) << preview.error.toStdString(); + EXPECT_EQ(preview.rows.size(), 1); +} diff --git a/tests/test_main.cpp b/tests/test_main.cpp index 91370ba7..398aa1e0 100644 --- a/tests/test_main.cpp +++ b/tests/test_main.cpp @@ -1,8 +1,12 @@ -#include +#include #include int main(int argc, char *argv[]) { - QCoreApplication app(argc, argv); + // Widget tests need a QApplication but no real display. + if (!qEnvironmentVariableIsSet("QT_QPA_PLATFORM")) + qputenv("QT_QPA_PLATFORM", "offscreen"); + + QApplication app(argc, argv); ::testing::InitGoogleTest(&argc, argv); int result = RUN_ALL_TESTS(); return result; diff --git a/tests/test_pagedresultmodel.cpp b/tests/test_pagedresultmodel.cpp new file mode 100644 index 00000000..f8bb2a77 --- /dev/null +++ b/tests/test_pagedresultmodel.cpp @@ -0,0 +1,176 @@ +#include +#include +#include + +#include "database/sqlitedatabase.h" +#include "database/queryexecutor.h" + +class PagedResultTest : public ::testing::Test { +protected: + void SetUp() override { + tempDir = std::make_unique(); + dbPath = tempDir->path() + "/paged.db"; + db = std::make_unique(); + db->setSource(dbPath); + db->open(); + + QStringList sql; + sql << "CREATE TABLE big (id INTEGER PRIMARY KEY, name TEXT)"; + sql << "BEGIN TRANSACTION"; + for (int i = 0; i < RowCount; ++i) + sql << QString("INSERT INTO big (name) VALUES ('name%1')").arg(i); + sql << "COMMIT"; + QueryExecutor(db.get()).runStatements(sql); + } + + static constexpr int RowCount = 50000; + + std::unique_ptr tempDir; + QString dbPath; + std::unique_ptr db; +}; + +// The whole point: opening a large result set must not read all of it. +TEST_F(PagedResultTest, DoesNotFetchEveryRowUpFront) { + const std::unique_ptr model( + db->createResultModel("SELECT * FROM big")); + + ASSERT_NE(model, nullptr); + EXPECT_EQ(model->columnCount(), 2); + EXPECT_GT(model->rowCount(), 0); + EXPECT_LT(model->rowCount(), RowCount) << "the entire result set was loaded up front"; + EXPECT_TRUE(model->canFetchMore(QModelIndex())); +} + +// ...and scrolling must be able to reach every row, however many there are. +TEST_F(PagedResultTest, FetchesEveryRowOnDemand) { + const std::unique_ptr model( + db->createResultModel("SELECT * FROM big")); + ASSERT_NE(model, nullptr); + + while (model->canFetchMore(QModelIndex())) + model->fetchMore(QModelIndex()); + + EXPECT_EQ(model->rowCount(), RowCount); + EXPECT_EQ(model->data(model->index(0, 1)).toString(), "name0"); + EXPECT_EQ(model->data(model->index(RowCount - 1, 1)).toString(), + QString("name%1").arg(RowCount - 1)); +} + +TEST_F(PagedResultTest, SortsInTheDatabase) { + const std::unique_ptr model( + db->createResultModel("SELECT * FROM big")); + ASSERT_NE(model, nullptr); + + model->sort(0, Qt::DescendingOrder); + + // Sorted by the database, so the first row is the last id, even though only + // the first page has been fetched. + EXPECT_LT(model->rowCount(), RowCount); + EXPECT_EQ(model->data(model->index(0, 0)).toInt(), RowCount); +} + +TEST_F(PagedResultTest, ReportsErrorForInvalidStatement) { + QString error; + const std::unique_ptr model( + db->createResultModel("this is not sql at all", &error)); + + EXPECT_EQ(model, nullptr); + EXPECT_FALSE(error.isEmpty()); +} + +TEST_F(PagedResultTest, ReturnsNoModelForStatementWithoutRows) { + QString error; + const std::unique_ptr model( + db->createResultModel("INSERT INTO big (name) VALUES ('extra')", &error)); + + EXPECT_EQ(model, nullptr); + EXPECT_TRUE(error.isEmpty()) << error.toStdString(); + + // The statement still ran. + const QueryResult count = db->runStatement("SELECT COUNT(*) FROM big"); + EXPECT_EQ(count.rows.at(0).values.at(0).toInt(), RowCount + 1); +} + +// Multi-line SQL is how anyone writes a query in the editor; the newlines must +// survive, and a `--` comment must not swallow the rest of the statement. +TEST_F(PagedResultTest, RunsMultiLineStatements) { + QueryExecutor executor(db.get()); + QStringList statements; + statements << "SELECT id, name\nFROM big\nWHERE id <= 10"; + QStringList errors; + + const auto models = executor.runStatementsPaged(statements, &errors); + + EXPECT_TRUE(errors.isEmpty()) << errors.join("; ").toStdString(); + ASSERT_EQ(models.size(), 1); + EXPECT_EQ(models.at(0)->columnCount(), 2); + qDeleteAll(models); +} + +TEST_F(PagedResultTest, KeepsStatementsAfterALineComment) { + QueryExecutor executor(db.get()); + QStringList statements; + statements << "SELECT id -- the identifier\nFROM big"; + QStringList errors; + + const auto models = executor.runStatementsPaged(statements, &errors); + + EXPECT_TRUE(errors.isEmpty()) << errors.join("; ").toStdString(); + ASSERT_EQ(models.size(), 1); + qDeleteAll(models); +} + +// A table name may legitimately contain a double quote, and it reaches the +// query as an identifier, so it has to be escaped rather than interpolated. +TEST_F(PagedResultTest, PreviewsATableWhoseNameContainsAQuote) { + QueryExecutor executor(db.get()); + QStringList create; + create << "CREATE TABLE \"we\"\"ird\" (id INTEGER PRIMARY KEY)"; + create << "INSERT INTO \"we\"\"ird\" (id) VALUES (7)"; + executor.runStatements(create); + + QString error; + const std::unique_ptr model( + executor.previewTablePaged("we\"ird", &error)); + + ASSERT_NE(model, nullptr) << error.toStdString(); + EXPECT_EQ(model->rowCount(), 1); + EXPECT_EQ(model->data(model->index(0, 0)).toInt(), 7); +} + +// Sorting a statement that cannot be wrapped in a subquery must not re-run it. +TEST_F(PagedResultTest, DoesNotReExecuteAStatementItCannotSort) { + const QueryResult before = db->runStatement("SELECT COUNT(*) FROM big"); + const int rowsBefore = before.rows.at(0).values.at(0).toInt(); + + const std::unique_ptr model( + db->createResultModel("INSERT INTO big (name) VALUES ('sorted') RETURNING id")); + ASSERT_NE(model, nullptr) << "RETURNING should produce a result set"; + + model->sort(0, Qt::AscendingOrder); + + const QueryResult after = db->runStatement("SELECT COUNT(*) FROM big"); + EXPECT_EQ(after.rows.at(0).values.at(0).toInt(), rowsBefore + 1) + << "the statement ran a second time while sorting"; +} + +TEST_F(PagedResultTest, ClearsStaleErrorOnSuccess) { + QString error = "left over from an earlier call"; + + const std::unique_ptr model( + db->createResultModel("SELECT * FROM big", &error)); + + EXPECT_NE(model, nullptr); + EXPECT_TRUE(error.isEmpty()) << error.toStdString(); +} + +TEST_F(PagedResultTest, ClearsStaleErrorWhenThereIsNoResultSet) { + QString error = "left over from an earlier call"; + + const std::unique_ptr model( + db->createResultModel("INSERT INTO big (name) VALUES ('x')", &error)); + + EXPECT_EQ(model, nullptr); + EXPECT_TRUE(error.isEmpty()) << error.toStdString(); +} diff --git a/tests/test_queryexecutor.cpp b/tests/test_queryexecutor.cpp index 3364c2ad..d119d9ba 100644 --- a/tests/test_queryexecutor.cpp +++ b/tests/test_queryexecutor.cpp @@ -73,6 +73,19 @@ TEST_F(QueryExecutorTest, RunInvalidQuery) { EXPECT_GT(errors.size(), 0); } +// Arbitrary text is not valid SQL and must be reported, not silently ignored. +TEST_F(QueryExecutorTest, RunNonSqlTextReportsError) { + QStringList statements; + statements << "this is not sql at all"; + QStringList errors; + const auto results = executor->runStatements(statements, &errors); + + ASSERT_EQ(results.size(), 1); + EXPECT_FALSE(results.at(0).ok); + ASSERT_EQ(errors.size(), 1); + EXPECT_FALSE(errors.at(0).isEmpty()); +} + TEST_F(QueryExecutorTest, RunMultipleQueries) { QStringList statements; statements << "SELECT COUNT(*) FROM test_users" @@ -124,6 +137,24 @@ TEST_F(QueryExecutorTest, PreviewTableWithLimit) { EXPECT_EQ(result.rows.size(), 2); } +// Browsing a large table must not pull the whole table into memory. +TEST_F(QueryExecutorTest, PreviewTableCapsLargeTables) { + QStringList rows; + rows << "BEGIN TRANSACTION"; + for (int i = 0; i < 20000; ++i) { + rows << QString("INSERT INTO test_users (name, age) VALUES ('user%1', %1)").arg(i); + } + rows << "COMMIT"; + executor->runStatements(rows); + + const QueryResult unlimited = executor->previewTable("test_users"); + EXPECT_GT(unlimited.rows.size(), 1000); + + const QueryResult limited = executor->previewTable("test_users", 1000); + EXPECT_TRUE(limited.ok); + EXPECT_EQ(limited.rows.size(), 1000); +} + TEST_F(QueryExecutorTest, PreviewNonexistentTable) { QueryResult result = executor->previewTable("nonexistent_table"); diff --git a/tests/test_queryresultpresenter.cpp b/tests/test_queryresultpresenter.cpp new file mode 100644 index 00000000..94b74a87 --- /dev/null +++ b/tests/test_queryresultpresenter.cpp @@ -0,0 +1,99 @@ +#include +#include +#include +#include +#include + +#include "gui/queryresultpresenter.h" + +class QueryResultPresenterTest : public ::testing::Test { +protected: + void SetUp() override { + parent = std::make_unique(); + parent->resize(800, 600); + parent->show(); + presenter = std::make_unique(parent.get()); + } + + // Stands in for a PagedResult; the presenter only ever binds models. + static QAbstractItemModel *modelWithRows(const int rows) { + auto *model = new QStandardItemModel(rows, 2); + for (int row = 0; row < rows; ++row) { + model->setItem(row, 0, new QStandardItem(QString::number(row))); + model->setItem(row, 1, new QStandardItem("name" + QString::number(row))); + } + return model; + } + + std::unique_ptr parent; + std::unique_ptr presenter; +}; + +TEST_F(QueryResultPresenterTest, PresentShowsResults) { + presenter->present({modelWithRows(3)}); + + const auto views = parent->findChildren(); + ASSERT_EQ(views.size(), 1); + ASSERT_NE(views.at(0)->model(), nullptr); + EXPECT_EQ(views.at(0)->model()->rowCount(), 3); + EXPECT_TRUE(views.at(0)->isVisible()); +} + +TEST_F(QueryResultPresenterTest, PresentShowsOneViewPerResult) { + presenter->present({modelWithRows(3), modelWithRows(5)}); + + const auto views = parent->findChildren(); + ASSERT_EQ(views.size(), 2); +} + +TEST_F(QueryResultPresenterTest, PresentDiscardsPreviousResults) { + for (int i = 1; i <= 4; ++i) { + presenter->present({modelWithRows(i)}); + + const auto views = parent->findChildren(); + ASSERT_EQ(views.size(), 1) << "views from execution " << i - 1 << " were left behind"; + ASSERT_NE(views.at(0)->model(), nullptr); + EXPECT_EQ(views.at(0)->model()->rowCount(), i); + } +} + +TEST_F(QueryResultPresenterTest, PresentKeepsResultsWhenNothingToRender) { + presenter->present({modelWithRows(3)}); + presenter->present({}); // e.g. an INSERT, which displays nothing + + const auto views = parent->findChildren(); + ASSERT_EQ(views.size(), 1) << "a statement that renders nothing cleared the results"; + ASSERT_NE(views.at(0)->model(), nullptr); + EXPECT_EQ(views.at(0)->model()->rowCount(), 3); +} + +TEST_F(QueryResultPresenterTest, PresentTakesOwnershipOfModels) { + auto *model = modelWithRows(3); + presenter->present({model}); + + const auto views = parent->findChildren(); + ASSERT_EQ(views.size(), 1); + EXPECT_EQ(model->parent(), views.at(0)) << "model is not owned by its view"; +} + +TEST_F(QueryResultPresenterTest, PresentToViewReplacesModel) { + QTableView view(parent.get()); + + for (int i = 1; i <= 4; ++i) { + presenter->presentToView(&view, modelWithRows(i)); + + ASSERT_NE(view.model(), nullptr) << "preview " << i; + EXPECT_EQ(view.model()->rowCount(), i) << "preview " << i; + EXPECT_EQ(view.model()->parent(), &view) << "preview " << i; + } +} + +TEST_F(QueryResultPresenterTest, PresentToViewIgnoresMissingModel) { + QTableView view(parent.get()); + presenter->presentToView(&view, modelWithRows(3)); + + presenter->presentToView(&view, nullptr); + + ASSERT_NE(view.model(), nullptr) << "a failed preview cleared the view"; + EXPECT_EQ(view.model()->rowCount(), 3); +}