For sorted column remember column name instead of column index

This changes the structures for representing SELECT statements to save
the names of sorted column instead of their indexes. This change has
several benefits:
- It prepares the Query class to store actual real-world SELECT
  statements in the future.
- It prepares us to sort by expressions instead of just columns.
- This way we do not need an extra list of column names to generate the
  ORDER BY part of the SELECT statement when building it.

src/ExtendedTableWidget.cpp

@@ -1143,17 +1143,15 @@ void ExtendedTableWidget::openPrintDialog()
     delete mimeData;
 }
 
-void ExtendedTableWidget::sortByColumns(const std::vector<sqlb::SortedColumn>& columns)
+void ExtendedTableWidget::sortByColumns(const std::vector<sqlb::OrderBy>& columns)
 {
     // Are there even any columns to sort by?
     if(columns.size() == 0)
         return;
 
-    // Are we using a SqliteTableModel as a model? These support multiple sort columns. Other models might not; for those we just use the first sort column
+    // We only support sorting for SqliteTableModels with support for multiple and named sort columns
     SqliteTableModel* sqlite_model = dynamic_cast<SqliteTableModel*>(model());
-    if(sqlite_model == nullptr)
-        model()->sort(static_cast<int>(columns.front().column), columns.front().direction == sqlb::Ascending ? Qt::AscendingOrder : Qt::DescendingOrder);
-    else
+    if(sqlite_model)
         sqlite_model->sort(columns);
 }
 

src/ExtendedTableWidget.h

@@ -64,7 +64,7 @@ public:
 
     int numVisibleRows() const;
 
-    void sortByColumns(const std::vector<sqlb::SortedColumn>& columns);
+    void sortByColumns(const std::vector<sqlb::OrderBy>& columns);
 
     void setFrozenColumns(size_t count);
 

src/MainWindow.cpp

@@ -2500,8 +2500,10 @@ static void loadCondFormatMap(BrowseDataTableSettings::CondFormatMap& condFormat
     }
 }
 
-static void loadBrowseDataTableSettings(BrowseDataTableSettings& settings, QXmlStreamReader& xml)
+static void loadBrowseDataTableSettings(BrowseDataTableSettings& settings, sqlb::ObjectPtr obj, QXmlStreamReader& xml)
 {
+    const auto field_information = obj->fieldInformation();
+
     settings.showRowid = xml.attributes().value("show_row_id").toInt();
     settings.encoding = xml.attributes().value("encoding").toString();
     settings.plotXAxis = xml.attributes().value("plot_x_axis").toString();
@@ -2518,7 +2520,8 @@ static void loadBrowseDataTableSettings(BrowseDataTableSettings& settings, QXmlS
                 {
                     int index = xml.attributes().value("index").toInt();
                     int mode = xml.attributes().value("mode").toInt();
-                    settings.sortColumns.emplace_back(index, mode == Qt::AscendingOrder ? sqlb::Ascending : sqlb::Descending);
+                    if(static_cast<size_t>(index) < field_information.size())
+                        settings.sortColumns.emplace_back(field_information.at(static_cast<size_t>(index)).name, mode == Qt::AscendingOrder ? sqlb::OrderBy::Ascending : sqlb::OrderBy::Descending);
                     xml.skipCurrentElement();
                 }
             }
@@ -2804,7 +2807,7 @@ bool MainWindow::loadProject(QString filename, bool readOnly)
                                         sqlb::ObjectIdentifier (xml.attributes().value("schema").toString().toStdString(),
                                                                 xml.attributes().value("name").toString().toStdString());
                                     BrowseDataTableSettings settings;
-                                    loadBrowseDataTableSettings(settings, xml);
+                                    loadBrowseDataTableSettings(settings, db.getObjectByName(tableIdentifier), xml);
                                     TableBrowser::setSettings(tableIdentifier, settings);
                                 }
                             }
@@ -2890,7 +2893,7 @@ static void saveCondFormatMap(const QString& elementName, const BrowseDataTableS
     xml.writeEndElement();
 }
 
-static void saveBrowseDataTableSettings(const BrowseDataTableSettings& object, QXmlStreamWriter& xml)
+static void saveBrowseDataTableSettings(const BrowseDataTableSettings& object, sqlb::ObjectPtr obj, QXmlStreamWriter& xml)
 {
     xml.writeAttribute("show_row_id", QString::number(object.showRowid));
     xml.writeAttribute("encoding", object.encoding);
@@ -2902,7 +2905,7 @@ static void saveBrowseDataTableSettings(const BrowseDataTableSettings& object, Q
     for(const auto& column : object.sortColumns)
     {
         xml.writeStartElement("column");
-        xml.writeAttribute("index", QString::number(column.column));
+        xml.writeAttribute("index", QString::number(sqlb::getFieldNumber(obj, column.expr)));
         xml.writeAttribute("mode", QString::number(column.direction));
         xml.writeEndElement();
     }
@@ -3094,7 +3097,9 @@ void MainWindow::saveProject(const QString& currentFilename)
             xml.writeStartElement("table");
             xml.writeAttribute("schema", QString::fromStdString(tableIt->first.schema()));
             xml.writeAttribute("name", QString::fromStdString(tableIt->first.name()));
-            saveBrowseDataTableSettings(tableIt->second, xml);
+
+            auto obj = db.getObjectByName(tableIt->first);
+            saveBrowseDataTableSettings(tableIt->second, obj, xml);
             xml.writeEndElement();
         }
         // </browse_table_settings>
@@ -3812,7 +3817,9 @@ void MainWindow::changeTableBrowserTab(TableBrowserDock* dock)
         dock->setFocusStyle(true);
 
         m_currentTabTableModel = dock->tableBrowser()->model();
-        plotDock->updatePlot(m_currentTabTableModel, &dock->tableBrowser()->settings(dock->tableBrowser()->currentlyBrowsedTableName()), true, false);
+        const auto current_table_name = dock->tableBrowser()->currentlyBrowsedTableName();
+        if(!current_table_name.isEmpty())
+            plotDock->updatePlot(m_currentTabTableModel, &dock->tableBrowser()->settings(current_table_name), true, false);
     }
 }
 

src/TableBrowser.cpp

@@ -1111,6 +1111,9 @@ void TableBrowser::headerClicked(int logicalindex)
     // Get the current list of sort columns
     auto& columns = settings.sortColumns;
 
+    // Get the name of the column the user clicked on
+    std::string column = model()->headerData(logicalindex, Qt::Horizontal, Qt::EditRole).toString().toStdString();
+
     // Before sorting, first check if the Control key is pressed. If it is, we want to append this column to the list of sort columns. If it is not,
     // we want to sort only by the new column.
     if(QApplication::keyboardModifiers().testFlag(Qt::ControlModifier))
@@ -1120,27 +1123,26 @@ void TableBrowser::headerClicked(int logicalindex)
         // If the column was control+clicked again, change its sort order.
         // If not already in the sort order, add the column as a new sort column to the list.
         bool present = false;
-        for(sqlb::SortedColumn& sortedCol : columns) {
-
-            if(sortedCol.column == static_cast<size_t>(logicalindex)) {
-                sortedCol.direction = (sortedCol.direction == sqlb::Ascending ? sqlb::Descending : sqlb::Ascending);
+        for(sqlb::OrderBy& sortedCol : columns) {
+            if(sortedCol.expr == column) {
+                sortedCol.direction = (sortedCol.direction == sqlb::OrderBy::Ascending ? sqlb::OrderBy::Descending : sqlb::OrderBy::Ascending);
                 present = true;
                 break;
             }
         }
         if(!present)
-            columns.emplace_back(logicalindex, sqlb::Ascending);
+            columns.emplace_back(column, sqlb::OrderBy::Ascending);
     } else {
         // Single column sorting
 
         // If we have exactly one sort column and it is the column which was just clicked, change its sort order.
         // If not, clear the list of sorting columns and replace it by a single new sort column.
-        if(columns.size() == 1 && columns.front().column == static_cast<size_t>(logicalindex))
+        if(columns.size() == 1 && columns.front().expr == column)
         {
-            columns.front().direction = (columns.front().direction == sqlb::Ascending ? sqlb::Descending : sqlb::Ascending);
+            columns.front().direction = (columns.front().direction == sqlb::OrderBy::Ascending ? sqlb::OrderBy::Descending : sqlb::OrderBy::Ascending);
         } else {
             columns.clear();
-            columns.emplace_back(logicalindex, sqlb::Ascending);
+            columns.emplace_back(column, sqlb::OrderBy::Ascending);
         }
     }
 

src/TableBrowser.h

@@ -27,7 +27,7 @@ class TableBrowser;
 struct BrowseDataTableSettings
 {
     using CondFormatMap = std::map<size_t, std::vector<CondFormat>>;
-    std::vector<sqlb::SortedColumn> sortColumns;
+    std::vector<sqlb::OrderBy> sortColumns;
     std::map<int, int> columnWidths;
     std::map<size_t, QString> filterValues;
     CondFormatMap condFormats;

src/sql/Query.cpp

@@ -114,11 +114,7 @@ std::string Query::buildQuery(bool withRowid) const
     // Sorting
     std::string order_by;
     for(const auto& sorted_column : m_sort)
-    {
-        if(sorted_column.column < m_column_names.size())
-            order_by += sqlb::escapeIdentifier(m_column_names.at(sorted_column.column)) + " "
-                    + (sorted_column.direction == sqlb::Ascending ? "ASC" : "DESC") + ",";
-    }
+        order_by += sorted_column.toSql() + ",";
     if(order_by.size())
     {
         order_by.pop_back();

src/sql/Query.h

@@ -10,25 +10,35 @@
 namespace sqlb
 {
 
-enum SortDirection
+struct OrderBy
 {
-    Ascending,
-    Descending
-};
+    enum SortDirection
+    {
+        Ascending,
+        Descending
+    };
 
-struct SortedColumn
-{
-    SortedColumn(size_t column_, SortDirection direction_) :
-        column(column_),
+    OrderBy(const std::string& expr_, SortDirection direction_) :
+        expr(expr_),
+        is_expression(false),
         direction(direction_)
     {}
 
-    bool operator==(const SortedColumn& rhs) const
+    bool operator==(const OrderBy& rhs) const
     {
-        return column == rhs.column && direction == rhs.direction;
+        return direction == rhs.direction && is_expression == rhs.is_expression && expr == rhs.expr;
     }
 
-    size_t column;
+    std::string toSql() const
+    {
+        if(is_expression)
+            return sqlb::escapeIdentifier(expr) + (direction == Ascending ? " ASC" : " DESC");
+        else
+            return expr + (direction == Ascending ? " ASC" : " DESC");
+    }
+
+    std::string expr;
+    bool is_expression;
     SortDirection direction;
 };
 
@@ -76,9 +86,9 @@ public:
     std::vector<std::string>& globalWhere() { return m_global_where; }
     void setGlobalWhere(const std::vector<std::string>& w) { m_global_where = w; }
 
-    const std::vector<SortedColumn>& orderBy() const { return m_sort; }
-    std::vector<SortedColumn>& orderBy() { return m_sort; }
-    void setOrderBy(const std::vector<SortedColumn>& columns) { m_sort = columns; }
+    const std::vector<OrderBy>& orderBy() const { return m_sort; }
+    std::vector<OrderBy>& orderBy() { return m_sort; }
+    void setOrderBy(const std::vector<OrderBy>& columns) { m_sort = columns; }
 
 private:
     std::vector<std::string> m_column_names;
@@ -87,7 +97,7 @@ private:
     std::vector<SelectedColumn> m_selected_columns;
     std::unordered_map<size_t, std::string> m_where;    // TODO The two where variables should be merged into a single variable which ...
     std::vector<std::string> m_global_where;            // ... holds some sort of general tree structure for all sorts of where conditions.
-    std::vector<SortedColumn> m_sort;
+    std::vector<OrderBy> m_sort;
 
     std::vector<SelectedColumn>::iterator findSelectedColumnByName(const std::string& name);
     std::vector<SelectedColumn>::const_iterator findSelectedColumnByName(const std::string& name) const;

src/sql/sqlitetypes.h

@@ -671,6 +671,26 @@ bool removeField(T& object, const std::string& name)
     return removeField(&object, name);
 }
 
+/**
+ * @brief getFieldNumber returns the number of the field with the given name in an object. This is supposed to be a temporary helper function only.
+ * @param object
+ * @param name
+ * @return number of the field
+ *
+ * TODO Remove this function. Whereever it is used we make the assumption that the queried columns are exactly equal to the columns of the table or view.
+ *      For more complex queries this is not true and in fact it already is a dubious assumption because we also select the rowid column.
+ */
+inline size_t getFieldNumber(ObjectPtr object, const std::string& name)
+{
+    const auto object_fields = object->fieldInformation();
+    for(size_t i=0;i<object_fields.size();i++)
+    {
+        if(object_fields[i].name == name)
+            return i;
+    }
+    return 0;
+}
+
 } //namespace sqlb
 
 #endif

src/sqlitetablemodel.cpp

@@ -221,22 +221,22 @@ QVariant SqliteTableModel::headerData(int section, Qt::Orientation orientation,
     {
         // if we have a VIRTUAL table the model will not be valid, with no header data
         if(static_cast<size_t>(section) < m_headers.size()) {
-            const QString plainHeader = QString::fromStdString(m_headers.at(static_cast<size_t>(section)));
+            const std::string plainHeader = m_headers.at(static_cast<size_t>(section));
             // In the edit role, return a plain column name, but in the display role, add the sort indicator.
             if (role == Qt::EditRole)
-                return plainHeader;
+                return QString::fromStdString(plainHeader);
             else {
                 QString sortIndicator;
                 for(size_t i = 0; i < m_query.orderBy().size(); i++) {
-                    const sqlb::SortedColumn sortedColumn = m_query.orderBy()[i];
+                    const sqlb::OrderBy sortedColumn = m_query.orderBy()[i];
                     // Append sort indicator with direction and ordinal number in superscript style
-                    if (sortedColumn.column == static_cast<size_t>(section)) {
-                        sortIndicator = sortedColumn.direction == sqlb::Ascending ? " ▾" : " ▴";
+                    if (sortedColumn.expr == plainHeader) {
+                        sortIndicator = sortedColumn.direction == sqlb::OrderBy::Ascending ? " ▾" : " ▴";
                         sortIndicator.append(toSuperScript(i+1));
                         break;
                     }
                 }
-                return plainHeader + sortIndicator;
+                return QString::fromStdString(plainHeader) + sortIndicator;
             }
         }
         return QString::number(section + 1);
@@ -570,12 +570,12 @@ Qt::ItemFlags SqliteTableModel::flags(const QModelIndex& index) const
 void SqliteTableModel::sort(int column, Qt::SortOrder order)
 {
     // Construct a sort order list from this item and forward it to the function to sort by lists
-    std::vector<sqlb::SortedColumn> list;
-    list.emplace_back(column, order == Qt::AscendingOrder ? sqlb::Ascending : sqlb::Descending);
+    std::vector<sqlb::OrderBy> list;
+    list.emplace_back(m_headers.at(static_cast<size_t>(column)), order == Qt::AscendingOrder ? sqlb::OrderBy::Ascending : sqlb::OrderBy::Descending);
     sort(list);
 }
 
-void SqliteTableModel::sort(const std::vector<sqlb::SortedColumn>& columns)
+void SqliteTableModel::sort(const std::vector<sqlb::OrderBy>& columns)
 {
     // Don't do anything when the sort order hasn't changed
     if(m_query.orderBy() == columns)

src/sqlitetablemodel.h

@@ -89,7 +89,7 @@ public:
     void setQuery(const sqlb::Query& query);
 
     void sort(int column, Qt::SortOrder order = Qt::AscendingOrder) override;
-    void sort(const std::vector<sqlb::SortedColumn>& columns);
+    void sort(const std::vector<sqlb::OrderBy>& columns);
     sqlb::ObjectIdentifier currentTableName() const { return m_query.table(); }
 
     Qt::ItemFlags flags(const QModelIndex& index) const override;