From 94023a55e836b279f070ada862432480b962c511 Mon Sep 17 00:00:00 2001 From: Martin Kleusberg Date: Thu, 28 Jan 2021 18:17:54 +0100 Subject: [PATCH] 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 | 8 +++----- src/ExtendedTableWidget.h | 2 +- src/MainWindow.cpp | 21 ++++++++++++------- src/TableBrowser.cpp | 18 +++++++++-------- src/TableBrowser.h | 2 +- src/sql/Query.cpp | 6 +----- src/sql/Query.h | 40 +++++++++++++++++++++++-------------- src/sql/sqlitetypes.h | 20 +++++++++++++++++++ src/sqlitetablemodel.cpp | 18 ++++++++--------- src/sqlitetablemodel.h | 2 +- 10 files changed, 85 insertions(+), 52 deletions(-) diff --git a/src/ExtendedTableWidget.cpp b/src/ExtendedTableWidget.cpp index bc841ee0..12474415 100644 --- a/src/ExtendedTableWidget.cpp +++ b/src/ExtendedTableWidget.cpp @@ -1143,17 +1143,15 @@ void ExtendedTableWidget::openPrintDialog() delete mimeData; } -void ExtendedTableWidget::sortByColumns(const std::vector& columns) +void ExtendedTableWidget::sortByColumns(const std::vector& 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(model()); - if(sqlite_model == nullptr) - model()->sort(static_cast(columns.front().column), columns.front().direction == sqlb::Ascending ? Qt::AscendingOrder : Qt::DescendingOrder); - else + if(sqlite_model) sqlite_model->sort(columns); } diff --git a/src/ExtendedTableWidget.h b/src/ExtendedTableWidget.h index b3a3d99d..298ec425 100644 --- a/src/ExtendedTableWidget.h +++ b/src/ExtendedTableWidget.h @@ -64,7 +64,7 @@ public: int numVisibleRows() const; - void sortByColumns(const std::vector& columns); + void sortByColumns(const std::vector& columns); void setFrozenColumns(size_t count); diff --git a/src/MainWindow.cpp b/src/MainWindow.cpp index 38de4eda..be5b3e33 100644 --- a/src/MainWindow.cpp +++ b/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(index) < field_information.size()) + settings.sortColumns.emplace_back(field_information.at(static_cast(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(); } // @@ -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); } } diff --git a/src/TableBrowser.cpp b/src/TableBrowser.cpp index 30184017..5d57340c 100644 --- a/src/TableBrowser.cpp +++ b/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(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(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); } } diff --git a/src/TableBrowser.h b/src/TableBrowser.h index 05d9b0b3..a00623d4 100644 --- a/src/TableBrowser.h +++ b/src/TableBrowser.h @@ -27,7 +27,7 @@ class TableBrowser; struct BrowseDataTableSettings { using CondFormatMap = std::map>; - std::vector sortColumns; + std::vector sortColumns; std::map columnWidths; std::map filterValues; CondFormatMap condFormats; diff --git a/src/sql/Query.cpp b/src/sql/Query.cpp index e7b28f19..e64ed43f 100644 --- a/src/sql/Query.cpp +++ b/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(); diff --git a/src/sql/Query.h b/src/sql/Query.h index 112f6758..4f2fb957 100644 --- a/src/sql/Query.h +++ b/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& globalWhere() { return m_global_where; } void setGlobalWhere(const std::vector& w) { m_global_where = w; } - const std::vector& orderBy() const { return m_sort; } - std::vector& orderBy() { return m_sort; } - void setOrderBy(const std::vector& columns) { m_sort = columns; } + const std::vector& orderBy() const { return m_sort; } + std::vector& orderBy() { return m_sort; } + void setOrderBy(const std::vector& columns) { m_sort = columns; } private: std::vector m_column_names; @@ -87,7 +97,7 @@ private: std::vector m_selected_columns; std::unordered_map m_where; // TODO The two where variables should be merged into a single variable which ... std::vector m_global_where; // ... holds some sort of general tree structure for all sorts of where conditions. - std::vector m_sort; + std::vector m_sort; std::vector::iterator findSelectedColumnByName(const std::string& name); std::vector::const_iterator findSelectedColumnByName(const std::string& name) const; diff --git a/src/sql/sqlitetypes.h b/src/sql/sqlitetypes.h index 355047de..bcc10c2a 100644 --- a/src/sql/sqlitetypes.h +++ b/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(section) < m_headers.size()) { - const QString plainHeader = QString::fromStdString(m_headers.at(static_cast(section))); + const std::string plainHeader = m_headers.at(static_cast(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(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 list; - list.emplace_back(column, order == Qt::AscendingOrder ? sqlb::Ascending : sqlb::Descending); + std::vector list; + list.emplace_back(m_headers.at(static_cast(column)), order == Qt::AscendingOrder ? sqlb::OrderBy::Ascending : sqlb::OrderBy::Descending); sort(list); } -void SqliteTableModel::sort(const std::vector& columns) +void SqliteTableModel::sort(const std::vector& columns) { // Don't do anything when the sort order hasn't changed if(m_query.orderBy() == columns) diff --git a/src/sqlitetablemodel.h b/src/sqlitetablemodel.h index abe62eec..3010695a 100644 --- a/src/sqlitetablemodel.h +++ b/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& columns); + void sort(const std::vector& columns); sqlb::ObjectIdentifier currentTableName() const { return m_query.table(); } Qt::ItemFlags flags(const QModelIndex& index) const override;