From a41183f6c7cc1a8dd0916894925604aa8e3fe201 Mon Sep 17 00:00:00 2001 From: Marco Bubke Date: Mon, 31 Jul 2017 19:44:39 +0200 Subject: [PATCH] Sqlite: Add variadic bind and write functions You can now write SqliteWriteStatement statement("UPDATE test SET name=?, number=? WHERE rowid=?", database); statement.write("see", 7.23, 1); and SqliteWriteStatement statement("UPDATE test SET name=@name, number=@number WHERE rowid=@id", database); statement.writeNamed("@name", "see", "@number", 7.23, "@id", 1); This is more type safe than using variants and performant too. Change-Id: Ie1ed2a6d326b956be5c4ec056214f3f5b1531f45 Reviewed-by: Tim Jenssen --- src/libs/sqlite/sqlitedatabase.cpp | 3 +- src/libs/sqlite/sqlitedatabase.h | 4 +- src/libs/sqlite/sqlitedatabasebackend.cpp | 14 ++--- src/libs/sqlite/sqlitedatabasebackend.h | 11 ++-- src/libs/sqlite/sqlitereadstatement.h | 3 + src/libs/sqlite/sqlitereadwritestatement.cpp | 6 -- src/libs/sqlite/sqlitereadwritestatement.h | 11 ++-- src/libs/sqlite/sqlitestatement.cpp | 24 ++++---- src/libs/sqlite/sqlitestatement.h | 60 ++++++++++++++++++- src/libs/sqlite/sqlitewritestatement.h | 7 ++- .../unittest/sqlitedatabasebackend-test.cpp | 4 +- tests/unit/unittest/sqlitestatement-test.cpp | 58 +++++++++++++++++- 12 files changed, 166 insertions(+), 39 deletions(-) diff --git a/src/libs/sqlite/sqlitedatabase.cpp b/src/libs/sqlite/sqlitedatabase.cpp index e8f35d1171..8c3c1a8cdf 100644 --- a/src/libs/sqlite/sqlitedatabase.cpp +++ b/src/libs/sqlite/sqlitedatabase.cpp @@ -30,7 +30,8 @@ namespace Sqlite { SqliteDatabase::SqliteDatabase() - : m_journalMode(JournalMode::Wal) + : m_databaseBackend(*this), + m_journalMode(JournalMode::Wal) { } diff --git a/src/libs/sqlite/sqlitedatabase.h b/src/libs/sqlite/sqlitedatabase.h index 854ecf975f..1129cf3a06 100644 --- a/src/libs/sqlite/sqlitedatabase.h +++ b/src/libs/sqlite/sqlitedatabase.h @@ -41,6 +41,7 @@ class SQLITE_EXPORT SqliteDatabase { friend class SqliteAbstractTransaction; friend class SqliteStatement; + friend class SqliteBackend; public: SqliteDatabase(); @@ -66,9 +67,10 @@ public: void execute(Utils::SmallStringView sqlStatement); + SqliteDatabaseBackend &backend(); + private: void initializeTables(); - SqliteDatabaseBackend &backend(); private: diff --git a/src/libs/sqlite/sqlitedatabasebackend.cpp b/src/libs/sqlite/sqlitedatabasebackend.cpp index eb7a14a05a..cb346a8654 100644 --- a/src/libs/sqlite/sqlitedatabasebackend.cpp +++ b/src/libs/sqlite/sqlitedatabasebackend.cpp @@ -48,8 +48,9 @@ namespace Sqlite { -SqliteDatabaseBackend::SqliteDatabaseBackend() - : m_databaseHandle(nullptr), +SqliteDatabaseBackend::SqliteDatabaseBackend(SqliteDatabase &database) + : m_database(database), + m_databaseHandle(nullptr), m_cachedTextEncoding(Utf8) { } @@ -124,8 +125,7 @@ sqlite3 *SqliteDatabaseBackend::sqliteDatabaseHandle() void SqliteDatabaseBackend::setPragmaValue(Utils::SmallStringView pragmaKey, Utils::SmallStringView newPragmaValue) { - SqliteReadWriteStatement statement(Utils::SmallString{"PRAGMA ", pragmaKey, "='", newPragmaValue, "'"}, *this); - statement.step(); + execute(Utils::SmallString{"PRAGMA ", pragmaKey, "='", newPragmaValue, "'"}); Utils::SmallString pragmeValueInDatabase = toValue("PRAGMA " + pragmaKey); checkPragmaValue(pragmeValueInDatabase, newPragmaValue); @@ -160,7 +160,7 @@ TextEncoding SqliteDatabaseBackend::textEncoding() Utils::SmallStringVector SqliteDatabaseBackend::columnNames(Utils::SmallStringView tableName) { - SqliteReadWriteStatement statement("SELECT * FROM " + tableName, *this); + SqliteReadWriteStatement statement("SELECT * FROM " + tableName, m_database); return statement.columnNames(); } @@ -176,7 +176,7 @@ int SqliteDatabaseBackend::totalChangesCount() void SqliteDatabaseBackend::execute(Utils::SmallStringView sqlStatement) { - SqliteReadWriteStatement statement(sqlStatement, *this); + SqliteReadWriteStatement statement(sqlStatement, m_database); statement.step(); } @@ -391,7 +391,7 @@ void SqliteDatabaseBackend::throwException(const char *whatHasHappens) const template Type SqliteDatabaseBackend::toValue(Utils::SmallStringView sqlStatement) { - SqliteReadWriteStatement statement(sqlStatement, *this); + SqliteReadWriteStatement statement(sqlStatement, m_database); statement.next(); diff --git a/src/libs/sqlite/sqlitedatabasebackend.h b/src/libs/sqlite/sqlitedatabasebackend.h index d7027e0ba4..cb9e26e464 100644 --- a/src/libs/sqlite/sqlitedatabasebackend.h +++ b/src/libs/sqlite/sqlitedatabasebackend.h @@ -33,13 +33,17 @@ struct sqlite3; namespace Sqlite { +class SqliteDatabase; + class SQLITE_EXPORT SqliteDatabaseBackend { public: - - SqliteDatabaseBackend(); + SqliteDatabaseBackend(SqliteDatabase &database); ~SqliteDatabaseBackend(); + SqliteDatabaseBackend(const SqliteDatabase &database) = delete; + SqliteDatabase &operator=(const SqliteDatabase &database) = delete; + void setMmapSize(qint64 defaultSize, qint64 maximumSize); void activateMultiThreading(); void activateLogging(); @@ -59,8 +63,6 @@ public: void setTextEncoding(TextEncoding textEncoding); TextEncoding textEncoding(); - - Utils::SmallStringVector columnNames(Utils::SmallStringView tableName); int changesCount(); @@ -105,6 +107,7 @@ protected: Q_NORETURN void throwException(const char *whatHasHappens) const; private: + SqliteDatabase &m_database; sqlite3 *m_databaseHandle; TextEncoding m_cachedTextEncoding; diff --git a/src/libs/sqlite/sqlitereadstatement.h b/src/libs/sqlite/sqlitereadstatement.h index 02c998fb5d..55bd5c15fb 100644 --- a/src/libs/sqlite/sqlitereadstatement.h +++ b/src/libs/sqlite/sqlitereadstatement.h @@ -42,10 +42,13 @@ public: using SqliteStatement::columnCount; using SqliteStatement::columnNames; using SqliteStatement::bind; + using SqliteStatement::bindValues; + using SqliteStatement::bindNameValues; using SqliteStatement::bindingIndexForName; using SqliteStatement::setBindingColumnNames; using SqliteStatement::bindingColumnNames; using SqliteStatement::toValue; + using SqliteStatement::database; protected: void checkIsReadOnlyStatement(); diff --git a/src/libs/sqlite/sqlitereadwritestatement.cpp b/src/libs/sqlite/sqlitereadwritestatement.cpp index b642675e0a..c7d8e468d9 100644 --- a/src/libs/sqlite/sqlitereadwritestatement.cpp +++ b/src/libs/sqlite/sqlitereadwritestatement.cpp @@ -33,10 +33,4 @@ SqliteReadWriteStatement::SqliteReadWriteStatement(Utils::SmallStringView sqlSta { } -SqliteReadWriteStatement::SqliteReadWriteStatement(Utils::SmallStringView sqlStatement, - SqliteDatabaseBackend &backend) - : SqliteStatement(sqlStatement, backend) -{ -} - } // namespace Sqlite diff --git a/src/libs/sqlite/sqlitereadwritestatement.h b/src/libs/sqlite/sqlitereadwritestatement.h index 8488fcc67c..8f93ff85be 100644 --- a/src/libs/sqlite/sqlitereadwritestatement.h +++ b/src/libs/sqlite/sqlitereadwritestatement.h @@ -37,9 +37,11 @@ public: SqliteReadWriteStatement(Utils::SmallStringView sqlStatement, SqliteDatabase &database); using SqliteStatement::next; - using SqliteStatement::step; + using SqliteStatement::execute; using SqliteStatement::reset; using SqliteStatement::bind; + using SqliteStatement::bindValues; + using SqliteStatement::bindNameValues; using SqliteStatement::bindingIndexForName; using SqliteStatement::setBindingColumnNames; using SqliteStatement::bindingColumnNames; @@ -49,10 +51,9 @@ public: using SqliteStatement::columnCount; using SqliteStatement::columnNames; using SqliteStatement::toValue; - -private: - explicit SqliteReadWriteStatement(Utils::SmallStringView sqlStatement, - SqliteDatabaseBackend &backend); + using SqliteStatement::database; + using SqliteStatement::write; + using SqliteStatement::writeNamed; }; } // namespace Sqlite diff --git a/src/libs/sqlite/sqlitestatement.cpp b/src/libs/sqlite/sqlitestatement.cpp index 37081ca106..fee0130acc 100644 --- a/src/libs/sqlite/sqlitestatement.cpp +++ b/src/libs/sqlite/sqlitestatement.cpp @@ -43,14 +43,8 @@ namespace Sqlite { SqliteStatement::SqliteStatement(Utils::SmallStringView sqlStatement, SqliteDatabase &database) - : SqliteStatement(sqlStatement, database.backend()) -{ - -} - -SqliteStatement::SqliteStatement(Utils::SmallStringView sqlStatement, SqliteDatabaseBackend &databaseBackend) : m_compiledStatement(nullptr, deleteCompiledStatement), - m_databaseBackend(databaseBackend), + m_database(database), m_bindingParameterCount(0), m_columnCount(0), m_isReadyToFetchValues(false) @@ -146,6 +140,11 @@ void SqliteStatement::step() const next(); } +void SqliteStatement::execute() const +{ + next(); +} + int SqliteStatement::columnCount() const { return m_columnCount; @@ -203,7 +202,7 @@ template SQLITE_EXPORT void SqliteStatement::bind(Utils::SmallStringView name, q template SQLITE_EXPORT void SqliteStatement::bind(Utils::SmallStringView name, double value); template SQLITE_EXPORT void SqliteStatement::bind(Utils::SmallStringView name, Utils::SmallStringView text); -int SqliteStatement::bindingIndexForName(Utils::SmallStringView name) +int SqliteStatement::bindingIndexForName(Utils::SmallStringView name) const { return sqlite3_bind_parameter_index(m_compiledStatement.get(), name.data()); } @@ -241,12 +240,12 @@ void SqliteStatement::prepare(Utils::SmallStringView sqlStatement) sqlite3 *SqliteStatement::sqliteDatabaseHandle() const { - return m_databaseBackend.sqliteDatabaseHandle(); + return m_database.backend().sqliteDatabaseHandle(); } TextEncoding SqliteStatement::databaseTextEncoding() { - return m_databaseBackend.textEncoding(); + return m_database.backend().textEncoding(); } bool SqliteStatement::checkForStepError(int resultCode) const @@ -359,6 +358,11 @@ QString SqliteStatement::columnName(int column) const return QString::fromUtf8(sqlite3_column_name(m_compiledStatement.get(), column)); } +SqliteDatabase &SqliteStatement::database() const +{ + return m_database; +} + static Utils::SmallString textForColumn(sqlite3_stmt *sqlStatment, int column) { const char *text = reinterpret_cast(sqlite3_column_text(sqlStatment, column)); diff --git a/src/libs/sqlite/sqlitestatement.h b/src/libs/sqlite/sqlitestatement.h index 1c748a3544..5f74c267ef 100644 --- a/src/libs/sqlite/sqlitestatement.h +++ b/src/libs/sqlite/sqlitestatement.h @@ -51,6 +51,7 @@ protected: bool next() const; void step() const; + void execute() const; void reset() const; template @@ -64,10 +65,36 @@ protected: void bind(int index, double value); void bind(int index, Utils::SmallStringView value); + template + void bindValues(Values ... values) + { + bindValuesByIndex(1, values...); + } + + template + void write(Values ... values) + { + bindValuesByIndex(1, values...); + execute(); + } + + template + void bindNameValues(Values ... values) + { + bindValuesByName(values...); + } + + template + void writeNamed(Values ... values) + { + bindValuesByName(values...); + execute(); + } + template void bind(Utils::SmallStringView name, Type value); - int bindingIndexForName(Utils::SmallStringView name); + int bindingIndexForName(Utils::SmallStringView name) const; void setBindingColumnNames(const Utils::SmallStringVector &bindingColumnNames); const Utils::SmallStringVector &bindingColumnNames() const; @@ -107,14 +134,43 @@ protected: QString columnName(int column) const; + SqliteDatabase &database() const; + protected: explicit SqliteStatement(Utils::SmallStringView sqlStatement, SqliteDatabaseBackend &databaseBackend); +private: + template + void bindValuesByIndex(int index, Type value) + { + bind(index, value); + } + + template + void bindValuesByIndex(int index, Type value, Value ... values) + { + bind(index, value); + bindValuesByIndex(index + 1, values...); + } + + template + void bindValuesByName(Utils::SmallStringView name, Type value) + { + bind(bindingIndexForName(name), value); + } + + template + void bindValuesByName(Utils::SmallStringView name, Type value, Values ... values) + { + bind(bindingIndexForName(name), value); + bindValuesByName(values...); + } + private: std::unique_ptr m_compiledStatement; Utils::SmallStringVector m_bindingColumnNames; - SqliteDatabaseBackend &m_databaseBackend; + SqliteDatabase &m_database; int m_bindingParameterCount; int m_columnCount; mutable bool m_isReadyToFetchValues; diff --git a/src/libs/sqlite/sqlitewritestatement.h b/src/libs/sqlite/sqlitewritestatement.h index 907222423d..a9a1b3de37 100644 --- a/src/libs/sqlite/sqlitewritestatement.h +++ b/src/libs/sqlite/sqlitewritestatement.h @@ -34,12 +34,17 @@ class SQLITE_EXPORT SqliteWriteStatement : private SqliteStatement public: explicit SqliteWriteStatement(Utils::SmallStringView sqlStatement, SqliteDatabase &database); - using SqliteStatement::step; + using SqliteStatement::execute; using SqliteStatement::reset; using SqliteStatement::bind; + using SqliteStatement::bindValues; + using SqliteStatement::bindNameValues; using SqliteStatement::bindingIndexForName; using SqliteStatement::setBindingColumnNames; using SqliteStatement::bindingColumnNames; + using SqliteStatement::database; + using SqliteStatement::write; + using SqliteStatement::writeNamed; protected: void checkIsWritableStatement(); diff --git a/tests/unit/unittest/sqlitedatabasebackend-test.cpp b/tests/unit/unittest/sqlitedatabasebackend-test.cpp index 88c7a53063..be93866671 100644 --- a/tests/unit/unittest/sqlitedatabasebackend-test.cpp +++ b/tests/unit/unittest/sqlitedatabasebackend-test.cpp @@ -25,6 +25,7 @@ #include "googletest.h" +#include #include #include #include @@ -43,7 +44,8 @@ protected: void TearDown() override; Utils::PathString databaseFilePath = QDir::tempPath() + "/SqliteDatabaseBackendTest.db"; - Sqlite::SqliteDatabaseBackend databaseBackend; + Sqlite::SqliteDatabase database; + Sqlite::SqliteDatabaseBackend &databaseBackend = database.backend(); }; using SqliteDatabaseBackendSlowTest = SqliteDatabaseBackend; diff --git a/tests/unit/unittest/sqlitestatement-test.cpp b/tests/unit/unittest/sqlitestatement-test.cpp index 13a51253de..3bf97e6d73 100644 --- a/tests/unit/unittest/sqlitestatement-test.cpp +++ b/tests/unit/unittest/sqlitestatement-test.cpp @@ -35,8 +35,8 @@ #include namespace { - using testing::ElementsAre; +using testing::PrintToString; using Sqlite::SqliteException; using Sqlite::SqliteDatabase; @@ -44,12 +44,30 @@ using Sqlite::SqliteReadStatement; using Sqlite::SqliteReadWriteStatement; using Sqlite::SqliteWriteStatement; +MATCHER_P3(HasValues, value1, value2, rowid, + std::string(negation ? "isn't" : "is") + + PrintToString(value1) + + ", " + PrintToString(value2) + + " and " + PrintToString(rowid) + ) +{ + SqliteDatabase &database = arg.database(); + + SqliteReadStatement statement("SELECT name, number FROM test WHERE rowid=?", database); + statement.bind(1, rowid); + + statement.next(); + + return statement.text(0) == value1 && statement.text(1) == value2; +} + class SqliteStatement : public ::testing::Test { protected: void SetUp() override; void TearDown() override; +protected: SqliteDatabase database; }; @@ -250,6 +268,44 @@ TEST_F(SqliteStatement, RequestBindingNamesFromStatement) ASSERT_THAT(statement.bindingColumnNames(), ElementsAre("name", "number", "id")); } +TEST_F(SqliteStatement, BindValues) +{ + SqliteWriteStatement statement("UPDATE test SET name=?, number=? WHERE rowid=?", database); + + statement.bindValues("see", 7.23, 1); + statement.execute(); + + ASSERT_THAT(statement, HasValues("see", "7.23", 1)); +} + +TEST_F(SqliteStatement, WriteValues) +{ + SqliteWriteStatement statement("UPDATE test SET name=?, number=? WHERE rowid=?", database); + + statement.write("see", 7.23, 1); + + ASSERT_THAT(statement, HasValues("see", "7.23", 1)); +} + +TEST_F(SqliteStatement, BindNamedValues) +{ + SqliteWriteStatement statement("UPDATE test SET name=@name, number=@number WHERE rowid=@id", database); + + statement.bindNameValues("@name", "see", "@number", 7.23, "@id", 1); + statement.execute(); + + ASSERT_THAT(statement, HasValues("see", "7.23", 1)); +} + +TEST_F(SqliteStatement, WriteNamedValues) +{ + SqliteWriteStatement statement("UPDATE test SET name=@name, number=@number WHERE rowid=@id", database); + + statement.writeNamed("@name", "see", "@number", 7.23, "@id", 1); + + ASSERT_THAT(statement, HasValues("see", "7.23", 1)); +} + TEST_F(SqliteStatement, ClosedDatabase) { database.close(); -- GitLab