Move query generation from SqliteTableModel into a separate class

CMakeLists.txt

@@ -96,6 +96,7 @@ endif()
 set(SQLB_HDR
 	src/version.h
 	src/sql/sqlitetypes.h
+	src/sql/Query.h
 	src/csvparser.h
 	src/sqlite.h
 	src/grammar/sqlite3TokenTypes.hpp
@@ -167,6 +168,7 @@ set(SQLB_SRC
 	src/sqlitetablemodel.cpp
 	src/RowLoader.cpp
 	src/sql/sqlitetypes.cpp
+	src/sql/Query.cpp
 	src/sqltextedit.cpp
 	src/docktextedit.cpp
 	src/csvparser.cpp

src/SqlUiLexer.cpp

@@ -208,7 +208,7 @@ QStringList SqlUiLexer::autoCompletionWordSeparators() const
     // is configured.
     QStringList wl;
 
-    QString escapeSeparator = sqlb::escapeIdentifier(".");
+    QString escapeSeparator = sqlb::escapeIdentifier(QString("."));
     // Case for non symetric quotes, e.g. "[.]" to "].["
     std::reverse(escapeSeparator.begin(), escapeSeparator.end());
 

src/sql/Query.cpp

@@ -0,0 +1,88 @@
+#include "Query.h"
+
+#include <algorithm>
+
+namespace sqlb
+{
+
+Query::Query()
+{
+}
+
+void Query::clear()
+{
+    m_table.clear();
+    m_rowid_column = "rowid";
+    m_selected_columns.clear();
+    m_where.clear();
+    m_sort.clear();
+}
+
+std::string Query::buildQuery(bool withRowid) const
+{
+    // Selector and display formats
+    std::string selector;
+    if (withRowid)
+        selector = sqlb::escapeIdentifier(m_rowid_column) + ",";
+
+    if(m_selected_columns.empty())
+    {
+        selector += "*";
+    } else {
+        for(const auto& it : m_selected_columns)
+        {
+            if (it.original_column != it.selector)
+                selector += it.selector + " AS " + sqlb::escapeIdentifier(it.original_column) + ",";
+            else
+                selector += sqlb::escapeIdentifier(it.original_column) + ",";
+        }
+        selector.pop_back();
+    }
+
+    // Filter
+    std::string where;
+    if(m_where.size())
+    {
+        where = "WHERE ";
+
+        for(auto i=m_where.cbegin();i!=m_where.cend();++i)
+        {
+            const auto it = findSelectedColumnByName(i->first);
+            std::string column = sqlb::escapeIdentifier(i->first);
+            if(it != m_selected_columns.cend() && it->selector != column)
+                column = it->selector;
+            where += column + " " + i->second + " AND ";
+        }
+
+        // Remove last 'AND '
+        where.erase(where.size() - 4);
+    }
+
+    // Sorting
+    std::string order_by;
+    if(m_sort.size())
+    {
+        order_by = "ORDER BY ";
+        for(const auto& sorted_column : m_sort)
+            order_by += sqlb::escapeIdentifier(sorted_column.column) + " " + sorted_column.direction + ",";
+        order_by.pop_back();
+    }
+
+    return "SELECT " + selector + " FROM " + m_table.toString().toStdString() + " " + where + " " + order_by;
+}
+
+std::vector<SelectedColumn>::iterator Query::findSelectedColumnByName(const std::string& name)
+{
+    return std::find_if(m_selected_columns.begin(), m_selected_columns.end(), [name](const SelectedColumn& c) {
+        return name == c.original_column;
+    });
+}
+
+std::vector<SelectedColumn>::const_iterator Query::findSelectedColumnByName(const std::string& name) const
+{
+    return std::find_if(m_selected_columns.cbegin(), m_selected_columns.cend(), [name](const SelectedColumn& c) {
+        return name == c.original_column;
+    });
+}
+
+}

src/sql/Query.h

@@ -0,0 +1,74 @@
+#ifndef QUERY_H
+#define QUERY_H
+
+#include "sqlitetypes.h"
+
+#include <string>
+#include <unordered_map>
+#include <vector>
+
+namespace sqlb
+{
+
+struct SortedColumn
+{
+    SortedColumn(const std::string& column_, const std::string& direction_) :
+        column(column_),
+        direction(direction_)
+    {}
+
+    std::string column;
+    std::string direction;
+};
+
+struct SelectedColumn
+{
+    SelectedColumn(const std::string& original_column_, const std::string& selector_) :
+        original_column(original_column_),
+        selector(selector_)
+    {}
+
+    std::string original_column;
+    std::string selector;
+};
+
+class Query
+{
+public:
+    Query();
+    Query(const sqlb::ObjectIdentifier& table) :
+        m_table(table)
+    {}
+
+    void clear();
+    std::string buildQuery(bool withRowid) const;
+
+    void setTable(const sqlb::ObjectIdentifier& table) { m_table = table; }
+    sqlb::ObjectIdentifier table() const { return m_table; }
+
+    void setRowIdColumn(const std::string& rowid) { m_rowid_column = rowid; }
+    std::string rowIdColumn() const { return m_rowid_column; }
+
+    const std::vector<SelectedColumn>& selectedColumns() const { return m_selected_columns; }
+    std::vector<SelectedColumn>& selectedColumns() { return m_selected_columns; }
+
+    const std::unordered_map<std::string, std::string>& where() const { return m_where; }
+    std::unordered_map<std::string, std::string>& where() { return m_where; }
+
+    const std::vector<SortedColumn>& orderBy() const { return m_sort; }
+    std::vector<SortedColumn>& orderBy() { return m_sort; }
+
+private:
+    sqlb::ObjectIdentifier m_table;
+    std::string m_rowid_column;
+    std::vector<SelectedColumn> m_selected_columns;
+    std::unordered_map<std::string, std::string> m_where;
+    std::vector<SortedColumn> m_sort;
+
+    std::vector<SelectedColumn>::iterator findSelectedColumnByName(const std::string& name);
+    std::vector<SelectedColumn>::const_iterator findSelectedColumnByName(const std::string& name) const;
+};
+
+}
+
+#endif

src/sql/sqlitetypes.cpp

@@ -34,6 +34,11 @@ QString escapeIdentifier(QString id)
     }
 }
 
+std::string escapeIdentifier(std::string id)
+{
+    return escapeIdentifier(QString::fromStdString(id)).toStdString();
+}
+
 QStringList escapeIdentifier(const QStringList& ids)
 {
     QStringList ret;

src/sql/sqlitetypes.h

@@ -53,6 +53,7 @@ enum escapeQuoting {
 void setIdentifierQuoting(escapeQuoting toQuoting);
 
 QString escapeIdentifier(QString id);
+std::string escapeIdentifier(std::string id);
 
 class ObjectIdentifier
 {

src/sqlitetablemodel.cpp

@@ -98,14 +98,9 @@ void SqliteTableModel::reset()
     beginResetModel();
     clearCache();
 
-    m_sTable.clear();
-    m_sRowidColumn.clear();
-    m_iSortColumn = 0;
-    m_sSortOrder = "ASC";
+    m_query.clear();
     m_headers.clear();
-    m_mWhere.clear();
     m_vDataTypes.clear();
-    m_vDisplayFormat.clear();
     m_pseudoPk.clear();
     m_mCondFormats.clear();
 
@@ -122,12 +117,8 @@ void SqliteTableModel::setTable(const sqlb::ObjectIdentifier& table, int sortCol
     // Unset all previous settings. When setting a table all information on the previously browsed data set is removed first.
     reset();
 
-    // Save the other parameters
-    m_sTable = table;
-    m_vDisplayFormat = display_format;
-
-    for(auto filterIt=filterValues.constBegin(); filterIt!=filterValues.constEnd(); ++filterIt)
-        updateFilter(filterIt.key(), filterIt.value(), false);
+    // Save the table name
+    m_query.setTable(table);
 
     // The first column is the rowid column and therefore is always of type integer
     m_vDataTypes.push_back(SQLITE_INTEGER);
@@ -139,8 +130,9 @@ void SqliteTableModel::setTable(const sqlb::ObjectIdentifier& table, int sortCol
         sqlb::TablePtr t = m_db.getObjectByName<sqlb::Table>(table);
         if(t && t->fields.size()) // parsing was OK
         {
-            m_sRowidColumn = t->rowidColumn();
-            m_headers.push_back(m_sRowidColumn);
+            QString rowid = t->rowidColumn();
+            m_query.setRowIdColumn(rowid.toStdString());
+            m_headers.push_back(rowid);
             m_headers.append(t->fieldNames());
 
             // parse columns types
@@ -166,15 +158,32 @@ void SqliteTableModel::setTable(const sqlb::ObjectIdentifier& table, int sortCol
     if(!allOk)
     {
         QString sColumnQuery = QString::fromUtf8("SELECT * FROM %1;").arg(table.toString());
-        m_sRowidColumn = "rowid";
+        m_query.setRowIdColumn("rowid");
         m_headers.push_back("rowid");
         m_headers.append(getColumns(nullptr, sColumnQuery, m_vDataTypes));
     }
 
-    // Set sort parameters. We're setting the sort column to an invalid value before calling sort() because this way, in sort() the
+    // Store filters and display formats
+    for(auto filterIt=filterValues.constBegin(); filterIt!=filterValues.constEnd(); ++filterIt)
+        updateFilter(filterIt.key(), filterIt.value(), false);
+    if(display_format.size())
+    {
+        for(int i=1;i<m_headers.size();i++)
+        {
+            QString format;
+            if(i <= display_format.size())
+                 format = display_format[i-1];
+            else
+                 format = m_headers[i];
+
+            m_query.selectedColumns().emplace_back(m_headers[i].toStdString(), format.toStdString());
+        }
+    }
+
+    // Set sort parameters. We're setting the sort columns to no sorting before calling sort() because this way, in sort() the
     // current sort order is always changed and thus buildQuery() is always going to be called.
     // This is also why we don't need to call buildQuery() here again.
-    m_iSortColumn = -1;
+    m_query.orderBy().clear();
     sort(sortColumn, sortOrder);
 }
 
@@ -216,7 +225,7 @@ int SqliteTableModel::columnCount(const QModelIndex&) const
 
 int SqliteTableModel::filterCount() const
 {
-    return m_mWhere.size();
+    return m_query.where().size();
 }
 
 QVariant SqliteTableModel::headerData(int section, Qt::Orientation orientation, int role) const
@@ -340,13 +349,13 @@ sqlb::ForeignKeyClause SqliteTableModel::getForeignKeyClause(int column) const
 
     // No foreign keys when not browsing a table. This usually happens when executing custom SQL statements
     // and browsing the result set instead of browsing an entire table.
-    if(m_sTable.isEmpty())
+    if(m_query.table().isEmpty())
         return empty_foreign_key_clause;
 
     // Retrieve database object and check if it is a table. If it isn't stop here and don't return a foreign
     // key. This happens for views which don't have foreign keys (though we might want to think about how we
     // can check for foreign keys in the underlying tables for some purposes like tool tips).
-    sqlb::ObjectPtr obj = m_db.getObjectByName(m_sTable);
+    sqlb::ObjectPtr obj = m_db.getObjectByName(m_query.table());
     if(obj->type() != sqlb::Object::Table)
         return empty_foreign_key_clause;
 
@@ -396,7 +405,7 @@ bool SqliteTableModel::setTypedData(const QModelIndex& index, bool isBlob, const
         // used in a primary key. Otherwise SQLite will always output an 'datatype mismatch' error.
         if(newValue == "" && !newValue.isNull())
         {
-            sqlb::TablePtr table = m_db.getObjectByName<sqlb::Table>(m_sTable);
+            sqlb::TablePtr table = m_db.getObjectByName<sqlb::Table>(m_query.table());
             if(table)
             {
                 auto field = sqlb::findField(table, m_headers.at(index.column()));
@@ -410,10 +419,10 @@ bool SqliteTableModel::setTypedData(const QModelIndex& index, bool isBlob, const
         if(oldValue == newValue && oldValue.isNull() == newValue.isNull())
             return true;
 
-        if(m_db.updateRecord(m_sTable, m_headers.at(index.column()), cached_row.at(0), newValue, isBlob, m_pseudoPk))
+        if(m_db.updateRecord(m_query.table(), m_headers.at(index.column()), cached_row.at(0), newValue, isBlob, m_pseudoPk))
         {
             cached_row.replace(index.column(), newValue);
-            if(m_headers.at(index.column()) == m_sRowidColumn) {
+            if(m_headers.at(index.column()).toStdString() == m_query.rowIdColumn()) {
                 cached_row.replace(0, newValue);
                 const QModelIndex& rowidIndex = index.sibling(index.row(), 0);
                 lock.unlock();
@@ -442,10 +451,10 @@ Qt::ItemFlags SqliteTableModel::flags(const QModelIndex& index) const
 
     // Custom display format set?
     bool custom_display_format = false;
-    if(m_vDisplayFormat.size())
+    if(m_query.selectedColumns().size())
     {
         if(index.column() > 0)
-            custom_display_format = m_vDisplayFormat.at(index.column()-1) != sqlb::escapeIdentifier(headerData(index.column(), Qt::Horizontal).toString());
+            custom_display_format = QString::fromStdString(m_query.selectedColumns().at(index.column()-1).selector) != sqlb::escapeIdentifier(headerData(index.column(), Qt::Horizontal).toString());
     }
 
     if(!isBinary(index) && !custom_display_format)
@@ -456,16 +465,18 @@ Qt::ItemFlags SqliteTableModel::flags(const QModelIndex& index) const
 void SqliteTableModel::sort(int column, Qt::SortOrder order)
 {
     // Don't do anything when the sort order hasn't changed
-    if(m_iSortColumn == column && m_sSortOrder == (order == Qt::AscendingOrder ? "ASC" : "DESC"))
+    if(m_query.orderBy().size() && QString::fromStdString(m_query.orderBy().at(0).column) == m_headers.at(column) && m_query.orderBy().at(0).direction == (order == Qt::AscendingOrder ? "ASC" : "DESC"))
         return;
 
+    // Reset sort order
+    m_query.orderBy().clear();
+
     // Save sort order
 	if (column >= 0 && column < m_headers.size())
-		m_iSortColumn = column;
-    m_sSortOrder = (order == Qt::AscendingOrder ? "ASC" : "DESC");
+        m_query.orderBy().emplace_back(m_headers.at(column).toStdString(), (order == Qt::AscendingOrder ? "ASC" : "DESC"));
 
     // Set the new query (but only if a table has already been set
-    if(!m_sTable.isEmpty())
+    if(!m_query.table().isEmpty())
         buildQuery();
 }
 
@@ -499,7 +510,7 @@ bool SqliteTableModel::insertRows(int row, int count, const QModelIndex& parent)
     std::vector<Row> tempList;
     for(int i=row; i < row + count; ++i)
     {
-        QString rowid = m_db.addRecord(m_sTable);
+        QString rowid = m_db.addRecord(m_query.table());
         if(rowid.isNull())
         {
             return false;
@@ -509,7 +520,7 @@ bool SqliteTableModel::insertRows(int row, int count, const QModelIndex& parent)
 
         // update column with default values
         Row rowdata;
-        if(m_db.getRow(m_sTable, rowid, rowdata))
+        if(m_db.getRow(m_query.table(), rowid, rowdata))
         {
             for(int j=1; j < m_headers.size(); ++j)
             {
@@ -547,7 +558,7 @@ bool SqliteTableModel::removeRows(int row, int count, const QModelIndex& parent)
         }
     }
 
-    bool ok = m_db.deleteRecords(m_sTable, rowids, m_pseudoPk);
+    bool ok = m_db.deleteRecords(m_query.table(), rowids, m_pseudoPk);
 
     if (ok) {
         beginRemoveRows(parent, row, row + count - 1);
@@ -574,7 +585,7 @@ QModelIndex SqliteTableModel::dittoRecord(int old_row)
     int firstEditedColumn = 0;
     int new_row = rowCount() - 1;
 
-    sqlb::TablePtr t = m_db.getObjectByName<sqlb::Table>(m_sTable);
+    sqlb::TablePtr t = m_db.getObjectByName<sqlb::Table>(m_query.table());
 
     QStringList pk = t->primaryKey();
     for (size_t col = 0; col < t->fields.size(); ++col) {
@@ -590,61 +601,9 @@ QModelIndex SqliteTableModel::dittoRecord(int old_row)
     return index(new_row, firstEditedColumn);
 }
 
-QString SqliteTableModel::customQuery(bool withRowid)
-{
-    QString where;
-
-    if(m_mWhere.size())
-    {
-        where = "WHERE ";
-
-        for(QMap<int, QString>::const_iterator i=m_mWhere.constBegin();i!=m_mWhere.constEnd();++i)
-        {
-            QString columnId = sqlb::escapeIdentifier(m_headers.at(i.key()));
-            if(m_vDisplayFormat.size() && m_vDisplayFormat.at(i.key()-1) != columnId)
-                columnId = m_vDisplayFormat.at(i.key()-1);
-            where.append(QString("%1 %2 AND ").arg(columnId).arg(i.value()));
-        }
-
-        // Remove last 'AND '
-        where.chop(4);
-    }
-
-    QString selector;
-    if (withRowid)
-        selector = sqlb::escapeIdentifier(m_headers.at(0)) + ",";
-
-    if(m_vDisplayFormat.empty())
-    {
-        selector += "*";
-    } else {
-        QString columnId;
-        for(int i=0;i<m_vDisplayFormat.size();i++) {
-            columnId = sqlb::escapeIdentifier(m_headers.at(i+1));
-            if (columnId != m_vDisplayFormat.at(i))
-                selector += m_vDisplayFormat.at(i) + " AS " + columnId + ",";
-            else
-                selector += columnId + ",";
-        }
-        selector.chop(1);
-    }
-
-    // Note: Building the SQL string is intentionally split into several parts here instead of arg()'ing it all together as one.
-    // The reason is that we're adding '%' characters automatically around search terms (and even if we didn't the user could add
-    // them manually) which means that e.g. searching for '1' results in another '%1' in the string which then totally confuses
-    // the QString::arg() function, resulting in an invalid SQL.
-    return QString("SELECT %1 FROM %2 ")
-            .arg(selector)
-            .arg(m_sTable.toString())
-            + where
-            + QString("ORDER BY %1 %2")
-            .arg(sqlb::escapeIdentifier(m_headers.at(m_iSortColumn)))
-            .arg(m_sSortOrder);
-}
-
 void SqliteTableModel::buildQuery()
 {
-    setQuery(customQuery(true), true);
+    setQuery(QString::fromStdString(m_query.buildQuery(true)), true);
 }
 
 void SqliteTableModel::removeCommentsFromQuery(QString& query)
@@ -753,9 +712,9 @@ void SqliteTableModel::updateFilter(int column, const QString& value, bool apply
 
     // If the value was set to an empty string remove any filter for this column. Otherwise insert a new filter rule or replace the old one if there is already one
     if(whereClause.isEmpty())
-        m_mWhere.remove(column);
+        m_query.where().erase(m_headers.at(column).toStdString());
     else {
-        m_mWhere.insert(column, whereClause);
+        m_query.where()[m_headers.at(column).toStdString()] = whereClause.toStdString();
     }
 
     // Build the new query
@@ -845,7 +804,7 @@ void SqliteTableModel::setPseudoPk(const QString& pseudoPk)
     {
         m_pseudoPk.clear();
         if(m_headers.size())
-            m_headers[0] = m_sRowidColumn;
+            m_headers[0] = QString::fromStdString(m_query.rowIdColumn());
     } else {
         m_pseudoPk = pseudoPk;
         if(m_headers.size())
@@ -857,9 +816,9 @@ void SqliteTableModel::setPseudoPk(const QString& pseudoPk)
 
 bool SqliteTableModel::isEditable() const
 {
-    return !m_sTable.isEmpty() &&
+    return !m_query.table().isEmpty() &&
             m_db.isOpen() &&
-            ((m_db.getObjectByName(m_sTable) && m_db.getObjectByName(m_sTable)->type() == sqlb::Object::Types::Table) || !m_pseudoPk.isEmpty());
+            ((m_db.getObjectByName(m_query.table()) && m_db.getObjectByName(m_query.table())->type() == sqlb::Object::Types::Table) || !m_pseudoPk.isEmpty());
 }
 
 void SqliteTableModel::triggerCacheLoad (int row) const

src/sqlitetablemodel.h

@@ -9,9 +9,9 @@
 #include <QColor>
 #include <memory>
 
-#include "sql/sqlitetypes.h"
 #include "RowCache.h"
 #include "CondFormat.h"
+#include "sql/Query.h"
 
 struct sqlite3;
 class DBBrowserDB;
@@ -81,14 +81,14 @@ public:
     void setQuery(const QString& sQuery, bool dontClearHeaders = false);
 
     QString query() const { return m_sQuery; }
-    QString customQuery(bool withRowid);
+    QString customQuery(bool withRowid) const { return QString::fromStdString(m_query.buildQuery(withRowid)); }
 
     /// configure for browsing specified table
     void setTable(const sqlb::ObjectIdentifier& table, int sortColumn = 0, Qt::SortOrder sortOrder = Qt::AscendingOrder, const QMap<int, QString> filterValues = QMap<int, QString>(), const QVector<QString> &display_format = QVector<QString>());
 
     void setChunkSize(size_t chunksize);
     void sort(int column, Qt::SortOrder order = Qt::AscendingOrder) override;
-    const sqlb::ObjectIdentifier& currentTableName() const { return m_sTable; }
+    sqlb::ObjectIdentifier currentTableName() const { return m_query.table(); }
 
     Qt::ItemFlags flags(const QModelIndex& index) const override;
 
@@ -172,15 +172,10 @@ private:
     bool nosync_isBinary(const QModelIndex& index) const;
 
     QString m_sQuery;
-    sqlb::ObjectIdentifier m_sTable;
-    QString m_sRowidColumn;
     QString m_pseudoPk;
-    int m_iSortColumn;
-    QString m_sSortOrder;
-    QMap<int, QString> m_mWhere;
-    QVector<QString> m_vDisplayFormat;
     QVector<int> m_vDataTypes;
     QMap<int, QVector<CondFormat>> m_mCondFormats;
+    sqlb::Query m_query;
 
     /**
      * @brief m_chunkSize Size of the next chunk fetch more will try to fetch.

src/src.pro

@@ -70,7 +70,8 @@ HEADERS += \
     CipherSettings.h \
     DotenvFormat.h \
     Palette.h \
-    CondFormat.h
+    CondFormat.h \
+    sql/Query.h
 
 SOURCES += \
     sqlitedb.cpp \
@@ -117,7 +118,8 @@ SOURCES += \
     CipherSettings.cpp \
     DotenvFormat.cpp \
     Palette.cpp \
-    CondFormat.cpp
+    CondFormat.cpp \
+    sql/Query.cpp
 
 RESOURCES += icons/icons.qrc \
              translations/flags/flags.qrc \

src/tests/CMakeLists.txt

@@ -11,6 +11,7 @@ set(TESTSQLOBJECTS_SRC
     ../sqlitetablemodel.cpp
     ../RowLoader.cpp
     ../sql/sqlitetypes.cpp
+    ../sql/Query.cpp
     ../csvparser.cpp
     ../grammar/Sqlite3Lexer.cpp
     ../grammar/Sqlite3Parser.cpp
@@ -27,6 +28,7 @@ set(TESTSQLOBJECTS_HDR
     ../grammar/Sqlite3Lexer.hpp
     ../grammar/Sqlite3Parser.hpp
     ../sql/sqlitetypes.h
+    ../sql/Query.h
     ../Data.h
 )
 
@@ -95,6 +97,7 @@ set(TESTREGEX_SRC
     ../sqlitetablemodel.cpp
     ../RowLoader.cpp
     ../sql/sqlitetypes.cpp
+    ../sql/Query.cpp
     ../grammar/Sqlite3Lexer.cpp
     ../grammar/Sqlite3Parser.cpp
     ../Settings.cpp
@@ -110,6 +113,7 @@ set(TESTREGEX_HDR
     ../grammar/Sqlite3Lexer.hpp
     ../grammar/Sqlite3Parser.hpp
     ../sql/sqlitetypes.h
+    ../sql/Query.h
     ../Data.h
 )