Merge pull request 'unify-grid' (#88) from unify-grid into main

Reviewed-on: #88

.idea/QtSettings.xml

@@ -0,0 +1,6 @@
+<?xml version="1.0" encoding="UTF-8"?>
+<project version="4">
+  <component name="QtSettings">
+    <option name="myCurrentProfile" value="Debug" />
+  </component>
+</project>

.idea/misc.xml

@@ -0,0 +1,7 @@
+<?xml version="1.0" encoding="UTF-8"?>
+<project version="4">
+  <component name="CMakePythonSetting">
+    <option name="pythonIntegrationState" value="YES" />
+  </component>
+  <component name="CMakeWorkspace" PROJECT_DIR="$PROJECT_DIR$" />
+</project>

.idea/pgLab.iml

@@ -1,8 +1,2 @@
 <?xml version="1.0" encoding="UTF-8"?>
-<module type="EMPTY_MODULE" version="4">
+<module classpath="CIDR" type="CPP_MODULE" version="4" />
-  <component name="NewModuleRootManager">
-    <content url="file://$MODULE_DIR$" />
-    <orderEntry type="inheritedJdk" />
-    <orderEntry type="sourceFolder" forTests="false" />
-  </component>
-</module>

pglab/CMakeLists.txt

@@ -30,9 +30,9 @@ set(PGLAB_SOURCES
     codeeditor/GutterPainter.cpp
     crud/CrudModel.cpp
     crud/CrudTab.cpp
+    crud/DataGridWidget.cpp
     querytool/QueryExplainModel.cpp
     querytool/QueryParamListController.cpp
-    querytool/QueryResultModel.cpp
     querytool/QueryTool.cpp
     serverinspector/DatabasesPage.cpp
     serverinspector/RolesPage.cpp

pglab/crud/CrudModel.cpp

@@ -4,11 +4,13 @@
 #include "catalog/PgAttribute.h"
 #include "catalog/PgAttributeContainer.h"
 #include "catalog/PgConstraintContainer.h"
+#include "catalog/PgClassContainer.h"
 #include "SqlFormattingUtils.h"
 #include "Pgsql_oids.h"
 #include "CustomDataRole.h"
 #include <QtConcurrent>
 #include <QFuture>
+#include <QBrush>
 #include <QMessageBox>
 #include "Pgsql_oids.h"
 #include "Pgsql_PgException.h"
@@ -39,12 +41,23 @@ void CrudModel::setConfig(std::shared_ptr<OpenDatabase> db, const PgClass &table
 {
 	m_database = db;
 	m_table = table;
-	m_primaryKey = db->catalog()->constraints()->getPrimaryForRelation(table.oid());
+	m_wholeTable = true;
 	callLoadData = true;
 	auto dbconfig = m_database->config();
 	m_dbConn.setupConnection(dbconfig);
 }
 
+void CrudModel::setResult(std::shared_ptr<OpenDatabase> db, std::shared_ptr<Pgsql::Result> result)
+{
+	m_database = db;
+	m_table.reset();
+	m_wholeTable = false;
+	callLoadData = false;
+	auto dbconfig = m_database->config();
+	m_dbConn.setupConnection(dbconfig);
+	loadIntoModel(result);
+}
+
 QVariant CrudModel::headerData(int section, Qt::Orientation orientation, int role) const
 {
     if (section == 0)
@@ -205,6 +218,8 @@ QVariant CrudModel::data(const QModelIndex &index, int role) const
         if (value)
             return StringToVariant(value->c_str(), typ);
     }
+    else if (role == Qt::ForegroundRole && !m_wholeTable && columnIsReadOnly(index.column()))
+        return QBrush(Qt::gray);
 
     return {};
 }
@@ -229,13 +244,18 @@ void CrudModel::loadData()
 void CrudModel::loadIntoModel(std::shared_ptr<Pgsql::Result> data)
 {
 	beginResetModel();
+	bool was_dirty = hasPendingChanges();
 	m_pendingRowList.clear();
 	m_roData = data;
     initializeColumnList();
+    m_mapping.emplace(createResultSourceMapping(*m_roData, *m_database->catalog()));
     lastRowKey = data->rows() - 1;
 	initRowMapping();
-    appendNewRowInternal();
+    if (m_wholeTable)
+        appendNewRowInternal();
 	endResetModel();
+	if (was_dirty)
+		emit pendingChangesChanged(false);
 }
 
 void CrudModel::initRowMapping()
@@ -277,7 +297,7 @@ Qt::ItemFlags CrudModel::flags(const QModelIndex &index) const
     if (index.column() < PreColumnCount)
         return flags;
 
-    if (m_primaryKey && !columnIsReadOnly(index.column()))
+    if (!columnIsReadOnly(index.column()))
 		flags |= Qt::ItemIsEditable;
 
 	return flags;
@@ -304,7 +324,10 @@ bool CrudModel::setData(const QModelIndex &index, const QVariant &value, int rol
 				s.clear();
 			val = s;
 		}
+		bool was_dirty = hasPendingChanges();
 		m_pendingRowList.setValue(col, row_mapping.rowKey, val);
+		if (!was_dirty)
+			emit pendingChangesChanged(true);
 
 		emit dataChanged(index, index, QVector<int>() << role);
 		return true;
@@ -312,22 +335,17 @@ bool CrudModel::setData(const QModelIndex &index, const QVariant &value, int rol
 	return false;
 }
 
-std::tuple<bool, std::vector<CrudModel::Value>> CrudModel::saveRow(const PendingRow &pending_row)
+std::tuple<bool, std::vector<CrudModel::Value>> CrudModel::saveRow(const PendingRow &pending_row, const RowMapping &row_mapping)
 {
     auto data = pending_row.data();
-    RowMapping& rowmapping = m_rowMapping[pending_row.row()];
+    if (data.empty())
+        return { true, {} };
 
-    if (!data.empty())
+    if (row_mapping.isNew())
     {
         QString buffer;
         Pgsql::Params params;
-        if (rowmapping.isNew())
+        std::tie(buffer, params) = createInsertQuery(pending_row);
-            std::tie(buffer, params) = createInsertQuery(pending_row);
-        else
-        {
-            Pgsql::Params pkey_params = getPKeyParamsForRow(pending_row.row());
-            std::tie(buffer, params) = createUpdateQuery(pkey_params, pending_row);
-        }
 
         Pgsql::Connection db_update_conn;
         auto dbconfig = m_database->config();
@@ -356,59 +374,120 @@ std::tuple<bool, std::vector<CrudModel::Value>> CrudModel::saveRow(const Pending
             msgBox.setText(ex.what());
             msgBox.exec();
         }
+        return { false, {} };
+    }
+
+    return saveUpdate(pending_row, row_mapping);
+}
+
+std::tuple<bool, std::vector<CrudModel::Value>> CrudModel::saveUpdate(const PendingRow &pending_row, const RowMapping &row_mapping)
+{
+    std::set<Oid> tables;
+    for (const auto &e : pending_row.data())
+        tables.insert(m_mapping->tableOid(e.first));
+
+    try
+    {
+        Pgsql::Connection db_update_conn;
+        auto dbconfig = m_database->config();
+        db_update_conn.connect(dbconfig.connectionString());
+        auto tx = Pgsql::Transaction::startTransaction(db_update_conn);
+        for (Oid table_oid : tables)
+        {
+            QString buffer;
+            Pgsql::Params params;
+            std::tie(buffer, params) = createUpdateQuery(table_oid, pending_row, row_mapping);
+            auto result = tx.queryParam(buffer, params);
+            if (!result || result.tuplesAffected() != 1)
+            {
+                QMessageBox::warning(nullptr, tr("Update failed"), tr("The change did not update exactly one row in %1, the row may have changed or is no longer unique.").arg(tableName(table_oid)));
+                return { false, {} };
+            }
+        }
+        tx.commit();
+
+        std::vector<Value> values;
+        values.reserve(m_roData->cols());
+        for (int col = 0; col < m_roData->cols(); ++col)
+        {
+            auto &&pending = pending_row.data();
+            auto found = pending.find(col);
+            values.push_back(found != pending.end() ? found->second : getSavedData(row_mapping, col));
+        }
+        return { true, values };
+    }
+    catch (const Pgsql::PgResultError &ex)
+    {
+        QMessageBox msgBox;
+        msgBox.setText(ex.what());
+        msgBox.exec();
     }
     return { false, {} };
 }
 
-Pgsql::Params CrudModel::getPKeyParamsForRow(int row) const
+QString CrudModel::tableName(Oid table_oid) const
 {
-	Pgsql::Params params;
+    auto table = m_database->catalog()->classes()->getByKey(table_oid);
-    for (auto attnum : m_primaryKey->key)
+    return table ? table->fullyQualifiedQuotedObjectName() : QString();
-    {
-		const int col = attNumToCol(attnum);
-		Oid t = getType(col);
-        auto s = getSavedData(col, row);
-        params.add(s, t);
-	}
-	return params;
 }
 
-QString CrudModel::columnName(int col) const
+std::vector<int> CrudModel::resultColumnsOfTable(Oid table_oid) const
 {
-	return m_roData->getColName(col);
+    std::vector<int> cols;
+    for (int col = 0; col < m_roData->cols(); ++col)
+        if (m_roData->ftable(col) == table_oid && m_roData->ftableCol(col) > 0)
+            cols.push_back(col);
+    return cols;
 }
 
-std::tuple<QString, Pgsql::Params> CrudModel::createUpdateQuery(const Pgsql::Params &pkey_params, const PendingRow &pending_row)
+QString CrudModel::attributeName(int col) const
 {
-	Pgsql::Params params;
+    if (col < static_cast<int>(columnList.size()) && columnList[col])
-	auto data = pending_row.data();
+        return columnList[col]->name;
-    QString table_name = m_table->fullyQualifiedQuotedObjectName();
+    return QString::fromUtf8(m_roData->getColName(col));
-	QString buffer;
+}
-	QTextStream q(&buffer);
-	q << "UPDATE " << table_name << " AS d\n  SET ";
-	int param = 0;
-    for (auto& e : data)
-    {
-		if (param > 0)
-			q << ",";
-		q << quoteIdent(columnName(e.first)) << "=$" << ++param;
-		params.add(e.second, getType(e.first));
-	}
 
-	q << "\nWHERE ";
+Pgsql::Params CrudModel::getPKeyParamsForRow(const RowMapping &row_mapping, Oid table_oid) const
-	int i = 0;
+{
-    for (auto attnum : m_primaryKey->key)
+    Pgsql::Params params;
+    for (int col : m_mapping->primaryKeyColumns(table_oid))
     {
-		int col = attNumToCol(attnum);
+        auto s = getSavedData(row_mapping, col);
-		if (i > 0)
+        params.add(s, getType(col));
-			q << " AND ";
+    }
-		q << quoteIdent(columnName(col)) << "=$" << ++param;
+    return params;
-		++i;
+}
-	}
+
-    params.addParams(pkey_params);
+std::tuple<QString, Pgsql::Params> CrudModel::createUpdateQuery(Oid table_oid, const PendingRow &pending_row, const RowMapping &row_mapping)
-	q << "\nRETURNING *";
+{
-	q.flush();
+    Pgsql::Params params;
-	return { buffer, params };
+    QString table_name = tableName(table_oid);
+    QString buffer;
+    QTextStream q(&buffer);
+    q << "UPDATE " << table_name << " AS d\n  SET ";
+    int param = 0;
+    for (auto& e : pending_row.data())
+    {
+        if (m_mapping->tableOid(e.first) != table_oid)
+            continue;
+        if (param > 0)
+            q << ",";
+        q << quoteIdent(attributeName(e.first)) << "=$" << ++param;
+        params.add(e.second, getType(e.first));
+    }
+
+    q << "\nWHERE ";
+    int i = 0;
+    for (int col : m_mapping->primaryKeyColumns(table_oid))
+    {
+        if (i > 0)
+            q << " AND ";
+        q << quoteIdent(attributeName(col)) << "=$" << ++param;
+        ++i;
+    }
+    params.addParams(getPKeyParamsForRow(row_mapping, table_oid));
+    q.flush();
+    return { buffer, params };
 }
 
 std::tuple<QString, Pgsql::Params> CrudModel::createInsertQuery(const PendingRow &pending_row)
@@ -424,12 +503,11 @@ std::tuple<QString, Pgsql::Params> CrudModel::createInsertQuery(const PendingRow
     for (const auto& e : data)
     {
         int num = e.first;
-        auto&& column = columnList[num];
         if (first)
             first = false;
         else
             q << ",";
-        q << quoteIdent(column.name);
+        q << quoteIdent(attributeName(num));
 	}
 	q << ") VALUES ($1";
 	for (size_t p = 2; p <= data.size(); ++p)
@@ -442,19 +520,6 @@ std::tuple<QString, Pgsql::Params> CrudModel::createInsertQuery(const PendingRow
 	return { buffer, params };
 }
 
-std::tuple<QString, Pgsql::Params> CrudModel::createDeleteStatement(const PKeyValues &pkey_values)
-{
-	Pgsql::Params params;
-	size_t i = 0;
-    for (auto attnum : m_primaryKey->key)
-    {
-		const int col = attNumToCol(attnum);
-		params.add(pkey_values[i].c_str(), getType(col));
-		++i;
-	}
-
-	return { createDeleteStatement(), params };
-}
 
 QString CrudModel::createDeleteStatement() const
 {
@@ -465,12 +530,11 @@ QString CrudModel::createDeleteStatement() const
 	q << "DELETE FROM " << table_name;
 	q << "\nWHERE ";
 	int i = 0;
-    for (auto attnum : m_primaryKey->key)
+    for (int col : m_mapping->primaryKeyColumns(m_table->oid()))
     {
-		const int col = attNumToCol(attnum);
 		if (i > 0)
 			q << " AND ";
-		q << quoteIdent(columnName(col)) << "=$" << ++i;
+		q << quoteIdent(attributeName(col)) << "=$" << ++i;
 	}
 
 	q.flush();
@@ -483,7 +547,13 @@ bool CrudModel::savePendingChanges()
     while (!m_pendingRowList.m_rows.empty())
     {
 		auto iter = m_pendingRowList.m_rows.begin();
-        auto [ok, modified_row] = saveRow(iter->second);
+        int pendingKey = iter->first;
+        auto mapping_iter = std::find_if(m_rowMapping.begin(), m_rowMapping.end(),
+                                         [pendingKey](const RowMapping &rhs) -> bool { return rhs.rowKey == pendingKey; });
+        if (mapping_iter == m_rowMapping.end())
+            return false;
+
+        auto [ok, modified_row] = saveRow(iter->second, *mapping_iter);
         if (ok)
         {
 			int rowKey = iter->first;
@@ -516,9 +586,13 @@ bool CrudModel::columnIsReadOnly(int column_index) const
     if (m_roData == nullptr)
         return true;
 
-    auto&& column = columnList[column_index - PreColumnCount];
+    int col = column_index - PreColumnCount;
-    return column.getIdentity() == PgAttribute::Identity::Always
+    if (!m_mapping || !m_mapping->isEditable(col))
-            || column.getGenerated() != PgAttribute::Generated::None;
+        return true;
+
+    const auto &column = columnList[col];
+    return !column || column->getIdentity() == PgAttribute::Identity::Always
+            || column->getGenerated() != PgAttribute::Generated::None;
 }
 
 void CrudModel::initializeColumnList()
@@ -526,28 +600,51 @@ void CrudModel::initializeColumnList()
     columnList.clear();
     columnList.reserve(m_roData->cols());
     auto atts = m_database->catalog()->attributes();
-    auto columns = atts->getColumnsForRelation(m_table->oid());
     for (int col = 0; col < m_roData->cols(); ++col)
     {
         int attnum = m_roData->ftableCol(col);
+        if (attnum <= 0)
+        {
+            columnList.emplace_back();
+            continue;
+        }
+        auto columns = atts->getColumnsForRelation(m_roData->ftable(col));
         auto find_result = std::find_if(columns.begin(), columns.end(), [attnum](const PgAttribute &att) { return att.num == attnum; });
-        assert(find_result != columns.end());
+        if (find_result != columns.end())
-        columnList.push_back(*find_result);
+            columnList.emplace_back(*find_result);
+        else
+            columnList.emplace_back();
     }
 }
 
-bool CrudModel::submit()
+bool CrudModel::saveChanges()
 {
-	return savePendingChanges();
+    bool was_dirty = hasPendingChanges();
+    bool ok = savePendingChanges();
+    if (was_dirty != hasPendingChanges())
+        emit pendingChangesChanged(hasPendingChanges());
+    return ok;
 }
 
-void CrudModel::revert()
+void CrudModel::revertChanges()
 {
+    if (!hasPendingChanges())
+        return;
 
+    m_pendingRowList.clear();
+    for (auto &row_mapping : m_rowMapping)
+        row_mapping.pending = false;
+
+    if (rowCount() > 0)
+        emit dataChanged(index(0, 0), index(rowCount() - 1, columnCount() - 1));
+    emit pendingChangesChanged(false);
 }
 
 void CrudModel::appendNewRow()
 {
+    if (!m_wholeTable)
+        return;
+
     int row = static_cast<int>(m_rowMapping.size());
 	beginInsertRows(QModelIndex(), row, row);
     appendNewRowInternal();
@@ -565,6 +662,8 @@ std::tuple<bool, QString> CrudModel::removeRows(const std::set<IntegerRange<int>
         return { true, "" };
     if (row_ranges.rbegin()->end() > static_cast<int>(m_rowMapping.size()))
         return { false, "Range error" };
+    if (!m_wholeTable || !m_table || m_mapping->primaryKeyColumns(m_table->oid()).empty())
+        return { false, "Rows can only be removed from a table with a primary key" };
 	// When removing rows there is no direct mapping anymore between the rows in the grid
 	// and the rows in m_roData
 
@@ -589,7 +688,7 @@ std::tuple<bool, QString> CrudModel::removeRows(const std::set<IntegerRange<int>
                 for (int current_row = range.start(); current_row < range.end(); ++current_row)
                 {
 					auto&& mapping = m_rowMapping[static_cast<size_t>(current_row)];
-					auto params = getPKeyParamsForRow(mapping.rowKey);
+					auto params = getPKeyParamsForRow(mapping, m_table->oid());
                     if (!params.empty())
 						db_update_conn.queryParam(delete_statement, params);
 				}
@@ -598,7 +697,10 @@ std::tuple<bool, QString> CrudModel::removeRows(const std::set<IntegerRange<int>
 			// If something goes wrong after this commit we should reload contents of model
 		}
 		// Then from model
+        bool was_dirty = hasPendingChanges();
         RemoveRangesOfRowsFromModel(row_ranges);
+        if (was_dirty != hasPendingChanges())
+            emit pendingChangesChanged(hasPendingChanges());
 		return  { true, "" };
     }
     catch (const Pgsql::PgResultError &error)
@@ -633,6 +735,8 @@ void CrudModel::RemoveRangeOfRowsFromModel(IntegerRange<int> range)
     beginRemoveRows(QModelIndex(), range.start(), range.end() - 1);
     SCOPE_EXIT { endRemoveRows(); };
     auto first = m_rowMapping.begin() + range.start();
+    for (auto iter = first; iter != first + range.length(); ++iter)
+        m_pendingRowList.m_rows.erase(iter->rowKey);
     m_rowMapping.erase(first, first + range.length());
 }
 

pglab/crud/CrudModel.h

@@ -5,8 +5,10 @@
 #include "Pgsql_Connection.h"
 
 #include "IntegerRange.h"
+#include "ResultSourceMapping.h"
 #include "catalog/PgClass.h"
 #include "catalog/PgConstraint.h"
+#include "catalog/PgAttribute.h"
 #include <QAbstractTableModel>
 #include <map>
 #include <memory>
@@ -26,6 +28,8 @@ public:
 	~CrudModel() override;
 
 	void setConfig(std::shared_ptr<OpenDatabase> db, const PgClass &table);
+	/// Show an arbitrary query result, cells are editable where the result maps them to a table with a primary key
+	void setResult(std::shared_ptr<OpenDatabase> db, std::shared_ptr<Pgsql::Result> result);
 
 
 	QVariant headerData(int section, Qt::Orientation orientation, int role = Qt::DisplayRole) const override;
@@ -42,15 +46,23 @@ public:
 
 	void loadData();
 
+	/// True when there are edits that have not been saved to the database
+	bool hasPendingChanges() const { return !m_pendingRowList.m_rows.empty(); }
+	/// Writes all pending rows to the database, rows that fail stay pending
+	bool saveChanges();
+	/// Discards all pending edits, the grid shows the last saved values again
+	void revertChanges();
+
 	std::tuple<bool, QString> removeRows(const std::set<IntegerRange<int>> &row_ranges);
 	bool removeRows(int row, int count, const QModelIndex &parent = QModelIndex()) override;
 
+signals:
+	/// Emitted when the model goes from clean to having pending changes or back
+	void pendingChangesChanged(bool hasChanges);
+
 public slots:
-	virtual bool submit() override;
-	virtual void revert() override;
 
 private:
-	using PKeyValues = std::vector<std::string>;
 
     const int PreColumnCount = 1;
 
@@ -76,6 +88,7 @@ private:
 				res += " NULLS FIRST";
 			else if (nulls == NullSorting::Last)
 				res += " NULLS LAST";
+			return res;
 		}
 
 	};
@@ -205,10 +218,11 @@ private:
 
 	std::shared_ptr<OpenDatabase> m_database;
 	std::optional<PgClass> m_table;
-	std::optional<PgConstraint> m_primaryKey;
+	std::optional<ResultSourceMapping> m_mapping;
 	ASyncDBConnection m_dbConn;
 
 	bool callLoadData = false;
+	bool m_wholeTable = false; ///< true when showing a whole table (CRUD), false for an arbitrary query result
 
 	std::shared_ptr<Pgsql::Result> m_roData;
 
@@ -218,7 +232,7 @@ private:
         // identity allways or generated
     };
 
-    std::vector<PgAttribute> columnList; // list of columnMeta 1 to 1 with columns in m_roData.
+    std::vector<std::optional<PgAttribute>> columnList; // attribute per column in m_roData, empty for expression columns
 
 	PendingRowList m_pendingRowList;
 
@@ -238,29 +252,30 @@ private:
 
 	Oid getType(int column) const;
 
-	QString columnName(int col) const;
 
     /// Get the PKey values from when the row was last restored or retrieved.
-    Pgsql::Params getPKeyParamsForRow(int row) const;
+    Pgsql::Params getPKeyParamsForRow(const RowMapping &row_mapping, Oid table_oid) const;
 
 	bool savePendingChanges();
 
-    std::tuple<QString, Pgsql::Params> createUpdateQuery(const Pgsql::Params &pkey_params, const PendingRow &pending_row);
+    std::tuple<QString, Pgsql::Params> createUpdateQuery(Oid table_oid, const PendingRow &pending_row, const RowMapping &row_mapping);
 	std::tuple<QString, Pgsql::Params> createInsertQuery(const PendingRow &pending_row);
-	std::tuple<QString, Pgsql::Params> createDeleteStatement(const PKeyValues &pkey_values);
 
 	QString createDeleteStatement() const;
-    std::tuple<bool, std::vector<Value>> saveRow(const PendingRow &pending_row);
+    std::tuple<bool, std::vector<Value>> saveRow(const PendingRow &pending_row, const RowMapping &row_mapping);
+    std::tuple<bool, std::vector<Value>> saveUpdate(const PendingRow &pending_row, const RowMapping &row_mapping);
 
 	void appendNewRow();
     void appendNewRowInternal();
 
     int lastRowKey = -1;
 	int allocNewRowKey() { return ++lastRowKey; }
-	/// Convert an attnum from the database catalog to the corresponding column in the model
+	/// Name of the table with the given oid, quoted and schema qualified
-	///
+	QString tableName(Oid table_oid) const;
-	/// \todo still assumes columns are in order, all being shown and no special column like oid shown.
+	/// Result columns that are user columns (attnum > 0) of the given table, in result order
-	int attNumToCol(int attnum) const { return attnum - 1; }
+	std::vector<int> resultColumnsOfTable(Oid table_oid) const;
+	/// Catalog name of the attribute behind a result column, falls back to the result column name
+	QString attributeName(int col) const;
 
     void RemoveRangesOfRowsFromModel(const std::set<IntegerRange<int>> &row_ranges);
     void RemoveRangeOfRowsFromModel(IntegerRange<int> row_range);

pglab/crud/CrudTab.cpp

@@ -3,16 +3,19 @@
 #include "CrudModel.h"
 #include "CustomDataRole.h"
 #include "ResultTableModelUtil.h"
-#include "util/PgLabItemDelegate.h"
+#include "DataGridWidget.h"
 #include "IntegerRange.h"
 #include "OpenDatabase.h"
 #include "catalog/PgClassContainer.h"
 #include "catalog/PgDatabaseCatalog.h"
 #include <QDebug>
+#include <QHeaderView>
 #include <QMenu>
+#include <QSortFilterProxyModel>
 #include <QMessageBox>
 #include <iterator>
 #include <set>
+#include <QTableView>
 
 
 CrudTab::CrudTab(IDatabaseWindow *context, QWidget *parent)
@@ -24,25 +27,18 @@ CrudTab::CrudTab(IDatabaseWindow *context, QWidget *parent)
 
 	m_db = context->openDatabase();
 
-	SetTableViewDefault(ui->tableView);
-
-	auto delegate = new PgLabItemDelegate(ui->tableView);
-	ui->tableView->setItemDelegate(delegate);
-
     m_crudModel = new CrudModel(nullptr);
 
-    m_SortFilterProxy = new QSortFilterProxyModel(this);
+	m_grid = new DataGridWidget(this);
-    m_SortFilterProxy->setSourceModel(m_crudModel);
+	ui->verticalLayout->addWidget(m_grid);
-    m_SortFilterProxy->setSortRole(CustomSortRole);
+	m_grid->setSourceModel(m_crudModel);
-    ui->tableView->setModel(m_SortFilterProxy);
+	m_grid->view()->verticalHeader()->setVisible(false);
-    ui->tableView->setSortingEnabled(true);
-    m_SortFilterProxy->sort(0, Qt::AscendingOrder);
 
-	ui->tableView->setSelectionMode(QAbstractItemView::ExtendedSelection);
+	m_grid->view()->setSelectionMode(QAbstractItemView::ExtendedSelection);
-	ui->tableView->setSelectionBehavior(QAbstractItemView::SelectRows);
+	m_grid->view()->setSelectionBehavior(QAbstractItemView::SelectRows);
-	ui->tableView->addAction(ui->actionRemove_rows);
+	m_grid->view()->addAction(ui->actionRemove_rows);
 
-	auto horizontal_header = ui->tableView->horizontalHeader();
+	auto horizontal_header = m_grid->view()->horizontalHeader();
 	horizontal_header->setContextMenuPolicy(Qt::ContextMenuPolicy::CustomContextMenu);
 	connect(horizontal_header, &QHeaderView::customContextMenuRequested,
 			this, &CrudTab::headerCustomContextMenu);
@@ -64,13 +60,20 @@ void CrudTab::setConfig(Oid oid) //std::shared_ptr<OpenDatabase> db, const PgCla
 
 void CrudTab::refresh()
 {
+	if (m_crudModel->hasPendingChanges()) {
+		auto res = QMessageBox::question(this, "pgLab",
+			tr("There are unsaved changes. Refreshing discards them. Do you want to continue?"),
+			QMessageBox::Yes, QMessageBox::No);
+		if (res != QMessageBox::Yes)
+			return;
+	}
 	m_crudModel->loadData();
 }
 
 void CrudTab::on_actionRemove_rows_triggered()
 {
 	std::set<IntegerRange<int>> row_ranges;
-    auto selection = m_SortFilterProxy->mapSelectionToSource(ui->tableView->selectionModel()->selection());
+    auto selection = m_grid->proxy()->mapSelectionToSource(m_grid->view()->selectionModel()->selection());
     for (auto& range : selection) {
 		row_ranges.emplace(range.top(), range.height());
 	}
@@ -110,7 +113,7 @@ void CrudTab::headerCustomContextMenu(const QPoint &pos)
 
     if (headerContextMenu)
     {
-        auto horizontal_header = ui->tableView->horizontalHeader();
+        auto horizontal_header = m_grid->view()->horizontalHeader();
         headerContextMenu->popup(horizontal_header->mapToGlobal(pos));
     }
 }
@@ -122,9 +125,9 @@ void CrudTab::gotoColumn(QString column)
         auto name = m_crudModel->headerData(i, Qt::Horizontal).toString();
         if (name == column)
         {
-            auto index = ui->tableView->currentIndex();
+            auto index = m_grid->view()->currentIndex();
-            index = ui->tableView->model()->index(index.row(), i);
+            index = m_grid->view()->model()->index(index.row(), i);
-            ui->tableView->setCurrentIndex(index);
+            m_grid->view()->setCurrentIndex(index);
             break;
         }
     }

pglab/crud/CrudTab.h

@@ -1,7 +1,6 @@
 #ifndef CRUDTAB_H
 #define CRUDTAB_H
 
-#include <QSortFilterProxyModel>
 #include "catalog/PgClass.h"
 #include "IDatabaseWindow.h"
 #include <QWidget>
@@ -14,6 +13,7 @@ namespace Ui {
 
 class OpenDatabase;
 class CrudModel;
+class DataGridWidget;
 class QMenu;
 
 class CrudTab : public QWidget {
@@ -33,7 +33,7 @@ private:
 	std::optional<PgClass> m_table;
 
 	CrudModel *m_crudModel = nullptr;
-    QSortFilterProxyModel *m_SortFilterProxy = nullptr;
+	DataGridWidget *m_grid = nullptr;
 
     QMenu *headerContextMenu = nullptr;
 

pglab/crud/CrudTab.ui

@@ -13,15 +13,7 @@
   <property name="windowTitle">
    <string>Form</string>
   </property>
-  <layout class="QVBoxLayout" name="verticalLayout">
+  <layout class="QVBoxLayout" name="verticalLayout"/>
-   <item>
-    <widget class="QTableView" name="tableView">
-     <attribute name="verticalHeaderVisible">
-      <bool>false</bool>
-     </attribute>
-    </widget>
-   </item>
-  </layout>
   <action name="actionRemove_rows">
    <property name="text">
     <string>Remove rows</string>

pglab/crud/DataGridWidget.cpp

@@ -0,0 +1,101 @@
+#include "DataGridWidget.h"
+#include "CrudModel.h"
+#include "CustomDataRole.h"
+#include "ResultTableModelUtil.h"
+#include "util/PgLabItemDelegate.h"
+#include <QAction>
+#include <QSortFilterProxyModel>
+#include <QStyle>
+#include <QTableView>
+#include <QToolBar>
+#include <QVBoxLayout>
+
+
+DataGridWidget::DataGridWidget(QWidget *parent)
+	: QWidget(parent)
+{
+	auto layout = new QVBoxLayout(this);
+	layout->setContentsMargins(0, 0, 0, 0);
+
+	m_toolbar = new QToolBar(this);
+	m_toolbar->setToolButtonStyle(Qt::ToolButtonTextBesideIcon);
+	layout->addWidget(m_toolbar);
+
+	m_actSave = new QAction(style()->standardIcon(QStyle::SP_DialogSaveButton), tr("Save"), this);
+	m_actSave->setShortcut(QKeySequence::Save);
+	m_actSave->setShortcutContext(Qt::WidgetWithChildrenShortcut);
+	m_actSave->setToolTip(tr("Save all pending changes to the database (Ctrl+S)"));
+	m_actSave->setEnabled(false);
+	addAction(m_actSave);
+	m_toolbar->addAction(m_actSave);
+	connect(m_actSave, &QAction::triggered, this, &DataGridWidget::onSave);
+
+	m_actRevert = new QAction(style()->standardIcon(QStyle::SP_DialogResetButton), tr("Revert"), this);
+	m_actRevert->setToolTip(tr("Discard all pending changes"));
+	m_actRevert->setEnabled(false);
+	m_toolbar->addAction(m_actRevert);
+	connect(m_actRevert, &QAction::triggered, this, &DataGridWidget::onRevert);
+
+	m_view = new QTableView(this);
+	layout->addWidget(m_view);
+
+	SetTableViewDefault(m_view);
+	auto delegate = new PgLabItemDelegate(m_view);
+	m_view->setItemDelegate(delegate);
+}
+
+void DataGridWidget::setSourceModel(QAbstractItemModel *model)
+{
+	if (!m_proxy) {
+		m_proxy = new QSortFilterProxyModel(this);
+		m_proxy->setSortRole(CustomSortRole);
+	}
+	disconnect(m_pendingChangesConnection);
+	m_proxy->setSourceModel(model);
+	m_view->setModel(m_proxy);
+	m_view->setSortingEnabled(true);
+	m_proxy->sort(0, Qt::AscendingOrder);
+
+	auto crud = qobject_cast<CrudModel *>(model);
+	m_toolbar->setVisible(crud != nullptr);
+	if (crud) {
+		m_pendingChangesConnection = connect(crud, &CrudModel::pendingChangesChanged, this, &DataGridWidget::onPendingChangesChanged);
+		onPendingChangesChanged(crud->hasPendingChanges());
+	}
+	else {
+		onPendingChangesChanged(false);
+	}
+}
+
+void DataGridWidget::onPendingChangesChanged(bool hasChanges)
+{
+	m_actSave->setEnabled(hasChanges);
+	m_actRevert->setEnabled(hasChanges);
+}
+
+void DataGridWidget::onSave()
+{
+	auto crud = qobject_cast<CrudModel *>(m_proxy->sourceModel());
+	if (!crud)
+		return;
+
+	commitOpenEditor();
+	crud->saveChanges();
+}
+
+void DataGridWidget::onRevert()
+{
+	auto crud = qobject_cast<CrudModel *>(m_proxy->sourceModel());
+	if (!crud)
+		return;
+
+	commitOpenEditor();
+	crud->revertChanges();
+}
+
+void DataGridWidget::commitOpenEditor()
+{
+	// Moving focus away from the editor makes the delegate commit its value, a toolbar click does not take focus
+	if (m_view->indexWidget(m_view->currentIndex()))
+		m_view->setFocus();
+}

pglab/crud/DataGridWidget.h

@@ -0,0 +1,42 @@
+#ifndef DATAGRIDWIDGET_H
+#define DATAGRIDWIDGET_H
+
+#include <QWidget>
+
+class QAbstractItemModel;
+class QAction;
+class QSortFilterProxyModel;
+class QTableView;
+class QToolBar;
+
+/** Sortable and editable grid shared by the CRUD tab and the query result tab.
+ */
+class DataGridWidget : public QWidget {
+	Q_OBJECT
+public:
+	explicit DataGridWidget(QWidget *parent = nullptr);
+
+	/// Sets the model to show, sorting is done by a proxy that uses CustomSortRole
+	void setSourceModel(QAbstractItemModel *model);
+
+	QTableView *view() const { return m_view; }
+	QSortFilterProxyModel *proxy() const { return m_proxy; }
+
+private slots:
+	void onPendingChangesChanged(bool hasChanges);
+	void onSave();
+	void onRevert();
+
+private:
+	/// Closes an open cell editor so its value is written to the model before saving or reverting
+	void commitOpenEditor();
+
+	QTableView *m_view = nullptr;
+	QSortFilterProxyModel *m_proxy = nullptr;
+	QToolBar *m_toolbar = nullptr;
+	QAction *m_actSave = nullptr;
+	QAction *m_actRevert = nullptr;
+	QMetaObject::Connection m_pendingChangesConnection;
+};
+
+#endif // DATAGRIDWIDGET_H

pglab/querytool/QueryResultModel.cpp

@@ -1,85 +0,0 @@
-#include "QueryResultModel.h"
-#include "ResultTableModelUtil.h"
-#include "Pgsql_declare.h"
-#include "Pgsql_oids.h"
-#include "catalog/PgDatabaseCatalog.h"
-#include "CustomDataRole.h"
-#include <QBrush>
-#include <QColor>
-
-using namespace Pgsql;
-
-QueryResultModel::QueryResultModel(QObject *parent, std::shared_ptr<Pgsql::Result> r
-			, std::shared_ptr<PgDatabaseCatalog> cat)
-	: QAbstractTableModel(parent)
-	, result(std::move(r))
-	, catalog(cat)
-{}
-
-
-int QueryResultModel::rowCount(const QModelIndex &) const
-{
-	int r = result->rows();
-	return r;
-}
-
-int QueryResultModel::columnCount(const QModelIndex &) const
-{
-	int r = result->cols();
-	return r;
-}
-
-QVariant QueryResultModel::data(const QModelIndex &index, int role) const
-{
-//	static const QString null_str("null");
-	QVariant v;
-
-	const int col = index.column();
-	const int rij = index.row();
-	const Oid oid = result->type(col);
-	if (role == Qt::DisplayRole || role == Qt::EditRole) {
-		if (result->null(col, rij)) {
-			//v = nullptr;
-		}
-		else {
-			Value value = result->get(col, rij);
-			if (oid == bool_oid) {
-				bool b = value;
-				v = b;
-			}
-			else {
-				QString s = value;
-				v = s;
-			}
-		}
-	}
-    else if (role == CustomDataTypeRole) {
-		v = oid;
-	}
-
-
-	return v;
-}
-
-QVariant QueryResultModel::headerData(int section, Qt::Orientation orientation, int role) const
-{
-	QVariant r;
-	if (role == Qt::DisplayRole) {
-		if (orientation == Qt::Horizontal) {
-			QString s(result->getColName(section));
-			s += "\n";
-			s += getTypeDisplayString(*catalog, result->type(section));
-			r = s;
-		}
-		else {
-			r = QString::number(section + 1);
-		}
-	}
-	Qt::ItemFlags f;
-	return r;
-}
-
-Qt::ItemFlags QueryResultModel::flags(const QModelIndex &) const
-{
-	return Qt::ItemIsSelectable | Qt::ItemIsEnabled | Qt::ItemIsEditable;
-}

pglab/querytool/QueryResultModel.h

@@ -1,31 +0,0 @@
-#ifndef QUERYRESULTMODEL_H
-#define QUERYRESULTMODEL_H
-
-#include <QAbstractTableModel>
-#include "catalog/models/BaseTableModel.h"
-#include "Pgsql_Connection.h"
-
-class PgDatabaseCatalog;
-
-class QueryResultModel : public QAbstractTableModel
-{
-	Q_OBJECT
-public:
-	QueryResultModel(QObject *parent, std::shared_ptr<Pgsql::Result> r, std::shared_ptr<PgDatabaseCatalog> cat);
-
-	int rowCount(const QModelIndex &parent = QModelIndex()) const override;
-	int columnCount(const QModelIndex &parent = QModelIndex()) const override;
-	QVariant data(const QModelIndex &index, int role = Qt::DisplayRole) const override;
-	virtual QVariant headerData(int section, Qt::Orientation orientation, int role = Qt::DisplayRole) const override;
-	virtual Qt::ItemFlags flags(const QModelIndex &index) const override;
-
-	std::shared_ptr<const Pgsql::Result> GetPgsqlResult() const { return result; }
-protected:
-//	virtual Oid getType(int column) const override;
-//	virtual QVariant getData(const QModelIndex &index) const override;
-private:
-	std::shared_ptr<Pgsql::Result> result;
-	std::shared_ptr<PgDatabaseCatalog> catalog;
-};
-
-#endif // QUERYRESULTMODEL_H

pglab/querytool/QueryTool.cpp

@@ -476,10 +476,8 @@ void QueryTool::query_ready(std::shared_ptr<Pgsql::Result> dbres, qint64 elapsed
         //int n_rows = dbres->getRows();
         //QString rowcount_str = QString("rows: %1").arg(dbres->getRows());
 
-        auto result_model = std::make_shared<QueryResultModel>(nullptr , dbres,
-                                                    m_catalog);
         TuplesResultWidget *trw = new TuplesResultWidget;
-        trw->setResult(result_model, elapsedms);
+        trw->setResult(m_context->openDatabase(), dbres, elapsedms);
         resultList.push_back(trw);
         ui->tabWidget->addTab(trw, "Data");
         if (resultList.size() == 1)

pglab/querytool/QueryTool.h

@@ -3,7 +3,6 @@
 
 #include "ManagedPage.h"
 #include "ASyncDBConnection.h"
-#include "QueryResultModel.h"
 #include "QueryExplainModel.h"
 #include "stopwatch.h"
 #include "tuplesresultwidget.h"
@@ -26,7 +25,6 @@ class QTableView;
 class QTabWidget;
 class SqlSyntaxHighlighter;
 class ExplainRoot;
-class QueryResultModel;
 class QueryExplainModel;
 class PgTypeContainer;
 class OpenDatabase;

pglab/tuplesresultwidget.cpp

@@ -1,11 +1,15 @@
-#include "tuplesresultwidget.h"
+#include "tuplesresultwidget.h"
 #include "ui_tuplesresultwidget.h"
 #include "ResultTableModelUtil.h"
 #include "util.h"
+#include "crud/CrudModel.h"
+#include "crud/DataGridWidget.h"
+#include "OpenDatabase.h"
 
 #include <QFile>
 #include <QTextStream>
-#include "util/PgLabItemDelegate.h"
+#include <QTableView>
+#include <QHeaderView>
 
 
 TuplesResultWidget::TuplesResultWidget(QWidget *parent) :
@@ -15,9 +19,13 @@ TuplesResultWidget::TuplesResultWidget(QWidget *parent) :
 	ui->setupUi(this);
 	ui->lblRowCount->setText(QString());
 
-	SetTableViewDefault(ui->ResultView);
+	m_grid = new DataGridWidget(this);
-	auto delegate = new PgLabItemDelegate(ui->ResultView);
+	ui->verticalLayout->insertWidget(0, m_grid);
-	ui->ResultView->setItemDelegate(delegate);
+	m_grid->view()->setFont(QFont("Source Sans Pro", 10));
+	m_grid->view()->setAlternatingRowColors(true);
+	m_grid->view()->setWordWrap(false);
+	m_grid->view()->verticalHeader()->setDefaultSectionSize(21);
+	m_grid->view()->verticalHeader()->setMinimumSectionSize(16);
 }
 
 TuplesResultWidget::~TuplesResultWidget()
@@ -25,14 +33,17 @@ TuplesResultWidget::~TuplesResultWidget()
 	delete ui;
 }
 
-void TuplesResultWidget::setResult(std::shared_ptr<QueryResultModel> res, float ms)
+void TuplesResultWidget::setResult(std::shared_ptr<OpenDatabase> db, std::shared_ptr<Pgsql::Result> res, float ms)
 {
-	resultModel = res;
+	m_result = res;
-	ui->ResultView->setModel(resultModel.get());
 
-	ui->ResultView->resizeColumnsToContents();
+	m_model = new CrudModel(m_grid);
+	m_model->setResult(db, res);
+	m_grid->setSourceModel(m_model);
 
-	QString rowcount_str = QString("rows: %1").arg(resultModel->rowCount());
+	m_grid->view()->resizeColumnsToContents();
+
+	QString rowcount_str = QString("rows: %1").arg(m_model->rowCount());
 	ui->lblRowCount->setText(rowcount_str);
 	ui->lblElapsedTime->setText(msfloatToHumanReadableString(ms));
 }
@@ -42,8 +53,6 @@ void TuplesResultWidget::exportData(const QString &file_name) const
 	QFile file(file_name);
 	if (file.open(QIODevice::WriteOnly)) {
 		QTextStream out(&file);
-		::exportTable(ui->ResultView, out);
+		::exportTable(m_grid->view(), out);
 	}
 }
-
-

pglab/tuplesresultwidget.h

@@ -1,13 +1,21 @@
 #ifndef TUPLESRESULTWIDGET_H
 #define TUPLESRESULTWIDGET_H
 
-#include "querytool/QueryResultModel.h"
 #include <QWidget>
+#include <memory>
 
 namespace Ui {
 class TuplesResultWidget;
 }
 
+namespace Pgsql {
+	class Result;
+}
+
+class OpenDatabase;
+class CrudModel;
+class DataGridWidget;
+
 class TuplesResultWidget : public QWidget
 {
 	Q_OBJECT
@@ -16,14 +24,15 @@ public:
 	explicit TuplesResultWidget(QWidget *parent = 0);
 	~TuplesResultWidget();
 
-	void setResult(std::shared_ptr<QueryResultModel> res, float ms);
+	void setResult(std::shared_ptr<OpenDatabase> db, std::shared_ptr<Pgsql::Result> res, float ms);
 	void exportData(const QString &file_name) const;
 
-	std::shared_ptr<const Pgsql::Result> GetPgsqlResult() const { return resultModel->GetPgsqlResult(); }
+	std::shared_ptr<const Pgsql::Result> GetPgsqlResult() const { return m_result; }
 private:
 	Ui::TuplesResultWidget *ui;
-
+	DataGridWidget *m_grid = nullptr;
-	std::shared_ptr<QueryResultModel> resultModel;
+	CrudModel *m_model = nullptr;
+	std::shared_ptr<Pgsql::Result> m_result;
 };
 
 #endif // TUPLESRESULTWIDGET_H

pglab/tuplesresultwidget.ui

@@ -29,37 +29,6 @@
    <property name="bottomMargin">
     <number>2</number>
    </property>
-   <item>
-    <widget class="QTableView" name="ResultView">
-     <property name="font">
-      <font>
-       <family>Source Sans Pro</family>
-       <pointsize>10</pointsize>
-      </font>
-     </property>
-     <property name="editTriggers">
-      <set>QAbstractItemView::DoubleClicked|QAbstractItemView::EditKeyPressed</set>
-     </property>
-     <property name="alternatingRowColors">
-      <bool>true</bool>
-     </property>
-     <property name="verticalScrollMode">
-      <enum>QAbstractItemView::ScrollPerPixel</enum>
-     </property>
-     <property name="horizontalScrollMode">
-      <enum>QAbstractItemView::ScrollPerPixel</enum>
-     </property>
-     <property name="wordWrap">
-      <bool>false</bool>
-     </property>
-     <attribute name="verticalHeaderDefaultSectionSize">
-      <number>21</number>
-     </attribute>
-     <attribute name="verticalHeaderMinimumSectionSize">
-      <number>16</number>
-     </attribute>
-    </widget>
-   </item>
    <item>
     <widget class="QWidget" name="widget" native="true">
      <layout class="QHBoxLayout" name="horizontalLayout">

pglablib/CMakeLists.txt

@@ -32,6 +32,7 @@ add_library(pglablib STATIC
     SqlFormattingUtils.cpp
     catalog/PgKeywordList.cpp
     QueryGenerator.cpp
+    ResultSourceMapping.cpp
     catalog/PgObject.cpp
     catalog/PgTablespace.cpp
     catalog/PgTablespaceContainer.cpp
@@ -107,6 +108,7 @@ add_library(pglablib STATIC
     catalog/PgCatalogTypes.h
     catalog/PgKeywordList.h
     QueryGenerator.h
+    ResultSourceMapping.h
     catalog/PgObject.h
     catalog/PgTablespace.h
     catalog/PgTablespaceContainer.h

pglablib/ResultSourceMapping.cpp

@@ -0,0 +1,159 @@
+#include "ResultSourceMapping.h"
+
+#include "Pgsql_Result.h"
+#include "catalog/PgClass.h"
+#include "catalog/PgDatabaseCatalog.h"
+#include "catalog/PgConstraintContainer.h"
+#include "catalog/PgNamespace.h"
+#include "catalog/PgNamespaceContainer.h"
+#include "catalog/PgClassContainer.h"
+
+#include <algorithm>
+#include <stdexcept>
+
+namespace {
+
+/// Whether a non-key column of the table is selected more than once.
+/// A table that appears twice in a join (through aliases) shows up like that, and libpq can not tell the instances apart.
+bool hasAmbiguousColumn(const std::vector<ResultColumnSource> &columns, Oid tableOid, const std::vector<int> &pkAttnums)
+{
+    for (const auto &col : columns) {
+        if (col.tableOid != tableOid || std::find(pkAttnums.begin(), pkAttnums.end(), col.attnum) != pkAttnums.end())
+            continue;
+        auto count = std::count_if(columns.begin(), columns.end(), [&](const ResultColumnSource &other) {
+            return other.tableOid == tableOid && other.attnum == col.attnum;
+        });
+        if (count > 1)
+            return true;
+    }
+    return false;
+}
+
+}
+
+ResultSourceMapping::ResultSourceMapping(std::vector<ResultColumnSource> columns, const std::vector<TablePrimaryKey> &primaryKeys)
+    : m_columns(std::move(columns))
+{
+    for (const auto &col : m_columns) {
+        if (col.tableOid == InvalidOid || findTable(col.tableOid))
+            continue;
+
+        TableInfo info;
+        info.tableOid = col.tableOid;
+
+        auto pk = std::find_if(primaryKeys.begin(), primaryKeys.end(), [&](const TablePrimaryKey &key) {
+            return key.tableOid == col.tableOid;
+        });
+        if (pk != primaryKeys.end() && !pk->attnums.empty()) {
+            bool complete = true;
+            for (int attnum : pk->attnums) {
+                auto found = std::find_if(m_columns.begin(), m_columns.end(), [&](const ResultColumnSource &source) {
+                    return source.tableOid == col.tableOid && source.attnum == attnum;
+                });
+                if (found == m_columns.end()) {
+                    complete = false;
+                    break;
+                }
+                info.pkColumns.push_back(static_cast<int>(std::distance(m_columns.begin(), found)));
+            }
+            info.editable = complete && !hasAmbiguousColumn(m_columns, col.tableOid, pk->attnums);
+        }
+        if (!info.editable)
+            info.pkColumns.clear();
+
+        m_tables.push_back(info);
+    }
+}
+
+int ResultSourceMapping::columnCount() const
+{
+    return static_cast<int>(m_columns.size());
+}
+
+Oid ResultSourceMapping::tableOid(int col) const
+{
+    if (col < 0 || col >= columnCount())
+        throw std::range_error("column index out of range");
+    return m_columns[col].tableOid;
+}
+
+int ResultSourceMapping::attnum(int col) const
+{
+    if (col < 0 || col >= columnCount())
+        throw std::range_error("column index out of range");
+    return m_columns[col].attnum;
+}
+
+bool ResultSourceMapping::isEditable(int col) const
+{
+    if (col < 0 || col >= columnCount())
+        throw std::range_error("column index out of range");
+
+    const auto &source = m_columns[col];
+    if (source.attnum <= 0)
+        return false;
+
+    const TableInfo *info = findTable(source.tableOid);
+    return info && info->editable;
+}
+
+std::vector<int> ResultSourceMapping::primaryKeyColumns(Oid tableOid) const
+{
+    const TableInfo *info = findTable(tableOid);
+    return info ? info->pkColumns : std::vector<int>{};
+}
+
+const ResultSourceMapping::TableInfo *ResultSourceMapping::findTable(Oid tableOid) const
+{
+    auto found = std::find_if(m_tables.begin(), m_tables.end(), [tableOid](const TableInfo &info) {
+        return info.tableOid == tableOid;
+    });
+    return found == m_tables.end() ? nullptr : &*found;
+}
+
+namespace {
+
+/// Catalog tables (pg_catalog, information_schema, ...) are views or system tables and must never be edited.
+bool isSystemCatalogTable(const PgDatabaseCatalog &catalog, Oid tableOid)
+{
+    const PgClass *cls = catalog.classes()->getByKey(tableOid);
+    if (!cls)
+        return false;
+    const PgNamespace *ns = catalog.namespaces()->getByKey(cls->nsOid());
+    return ns && ns->isSystemCatalog();
+}
+
+}
+
+ResultSourceMapping createResultSourceMapping(const Pgsql::Result &result, const PgDatabaseCatalog &catalog)
+{
+    std::vector<ResultColumnSource> columns;
+    std::vector<TablePrimaryKey> primaryKeys;
+
+    for (int col = 0; col < result.cols(); ++col) {
+        ResultColumnSource source;
+        source.tableOid = result.ftable(col);
+        source.attnum = result.ftableCol(col);
+        columns.push_back(source);
+
+        if (source.tableOid == InvalidOid)
+            continue;
+
+        bool known = std::any_of(primaryKeys.begin(), primaryKeys.end(), [&](const TablePrimaryKey &key) {
+            return key.tableOid == source.tableOid;
+        });
+        if (known)
+            continue;
+
+        TablePrimaryKey pk;
+        pk.tableOid = source.tableOid;
+        if (!isSystemCatalogTable(catalog, source.tableOid)) {
+            auto constraint = catalog.constraints()->getPrimaryForRelation(source.tableOid);
+            if (constraint)
+                pk.attnums.assign(constraint->key.begin(), constraint->key.end());
+        }
+        primaryKeys.push_back(pk);
+    }
+
+    return ResultSourceMapping(std::move(columns), primaryKeys);
+}

pglablib/ResultSourceMapping.h

@@ -0,0 +1,61 @@
+#ifndef RESULTSOURCEMAPPING_H
+#define RESULTSOURCEMAPPING_H
+
+#include <libpq-fe.h>
+
+#include <vector>
+
+class PgDatabaseCatalog;
+
+namespace Pgsql {
+    class Result;
+}
+
+/// The table column a result column is read from, as reported by libpq (PQftable/PQftablecol).
+/// tableOid is InvalidOid and attnum is 0 for expression columns.
+struct ResultColumnSource {
+    Oid tableOid = InvalidOid;
+    int attnum = 0;
+};
+
+/// The primary key of a table, attnums in key order.
+struct TablePrimaryKey {
+    Oid tableOid = InvalidOid;
+    std::vector<int> attnums;
+};
+
+/// Maps the columns of a query result to the table columns they come from and decides which
+/// cells can be edited. A cell is editable when it maps to a user column of a table whose full
+/// primary key is present in the result and which is not ambiguous.
+class ResultSourceMapping {
+public:
+    ResultSourceMapping(std::vector<ResultColumnSource> columns, const std::vector<TablePrimaryKey> &primaryKeys);
+
+    int columnCount() const;
+    /// Table the column is read from, InvalidOid for expression columns.
+    Oid tableOid(int col) const;
+    /// attnum of the column in its table, 0 for expression columns.
+    int attnum(int col) const;
+    bool isEditable(int col) const;
+    /// Result column indices holding the primary key of the table, in key order.
+    /// Empty when the table is not editable.
+    std::vector<int> primaryKeyColumns(Oid tableOid) const;
+
+private:
+    struct TableInfo {
+        Oid tableOid = InvalidOid;
+        bool editable = false;
+        std::vector<int> pkColumns;
+    };
+
+    std::vector<ResultColumnSource> m_columns;
+    std::vector<TableInfo> m_tables;
+
+    const TableInfo *findTable(Oid tableOid) const;
+};
+
+/// Builds the mapping for a query result, looking up the primary keys of the source tables in the catalog.
+ResultSourceMapping createResultSourceMapping(const Pgsql::Result &result, const PgDatabaseCatalog &catalog);
+
+
+#endif // RESULTSOURCEMAPPING_H

releasenotes/notes/datagrid-save-revert-toolbar-3f9c1a7e5b2d8604.yaml

@@ -0,0 +1,13 @@
+---
+features:
+  - |
+    Data grids in the table editor and in query results now have a toolbar
+    with :menuselection:`Save` and :menuselection:`Revert` buttons, Save can
+    also be triggered with :kbd:`Ctrl+S`. Both buttons are only enabled while
+    there are unsaved changes.
+other:
+  - |
+    Edits in data grids are no longer saved automatically when a cell editor
+    is closed or when :kbd:`Escape` is pressed. Use the :menuselection:`Save`
+    button to write changes to the database, or :menuselection:`Revert` to
+    discard them. Refreshing a table with unsaved changes asks for confirmation.

releasenotes/notes/query-results-editable-grid-b265a193a39f3245.yaml

@@ -0,0 +1,13 @@
+---
+features:
+  - |
+    Query results are now shown in the same grid as the table data tab. Columns
+    can be sorted by clicking their headers, and cells that belong to a table
+    with a primary key present in the result can be edited directly. Changes
+    are saved with an ``UPDATE`` by primary key. Tables in system catalog
+    schemas such as ``pg_catalog`` and ``information_schema`` are never editable.
+other:
+  - |
+    Read-only cells in query results, such as expression columns, columns of
+    tables without a primary key in the result, or columns of system catalog
+    tables, are now shown in grey so they can be told apart from editable cells.

tests/pglabtests/CMakeLists.txt

@@ -17,6 +17,7 @@ add_executable(pglabtests
     tst_NameManglingRules.cpp
     tst_TypeMappings.cpp
     tst_Range.cpp
+    ResultSourceMappingTest.cpp
     )
 
 target_link_libraries(pglabtests PRIVATE

tests/pglabtests/ResultSourceMappingTest.cpp

@@ -0,0 +1,168 @@
+#include <gtest/gtest.h>
+#include <gmock/gmock-matchers.h>
+
+#include "ResultSourceMapping.h"
+
+
+namespace {
+
+constexpr Oid tableA = 1001;
+constexpr Oid tableB = 1002;
+
+ResultColumnSource src(Oid table, int attnum)
+{
+	ResultColumnSource s;
+	s.tableOid = table;
+	s.attnum = attnum;
+	return s;
+}
+
+ResultColumnSource expression()
+{
+	return ResultColumnSource{};
+}
+
+TablePrimaryKey pk(Oid table, std::vector<int> attnums)
+{
+	TablePrimaryKey key;
+	key.tableOid = table;
+	key.attnums = std::move(attnums);
+	return key;
+}
+
+}
+
+
+TEST(ResultSourceMappingTest, single_table_with_pk_all_columns_editable)
+{
+	ResultSourceMapping m({src(tableA, 1), src(tableA, 2), src(tableA, 3)}, {pk(tableA, {1})});
+
+	EXPECT_EQ(m.columnCount(), 3);
+	EXPECT_TRUE(m.isEditable(0));
+	EXPECT_TRUE(m.isEditable(1));
+	EXPECT_TRUE(m.isEditable(2));
+	EXPECT_EQ(m.tableOid(1), tableA);
+	EXPECT_EQ(m.attnum(2), 3);
+	EXPECT_THAT(m.primaryKeyColumns(tableA), ::testing::ElementsAre(0));
+}
+
+TEST(ResultSourceMappingTest, composite_pk_maps_key_columns_in_key_order)
+{
+	ResultSourceMapping m({src(tableA, 3), src(tableA, 1), src(tableA, 2)}, {pk(tableA, {1, 2})});
+
+	EXPECT_TRUE(m.isEditable(0));
+	EXPECT_THAT(m.primaryKeyColumns(tableA), ::testing::ElementsAre(1, 2));
+}
+
+TEST(ResultSourceMappingTest, join_with_both_pks_both_tables_editable)
+{
+	ResultSourceMapping m({src(tableA, 1), src(tableA, 2), src(tableB, 1), src(tableB, 2)},
+		{pk(tableA, {1}), pk(tableB, {1})});
+
+	for (int col = 0; col < m.columnCount(); ++col) {
+		EXPECT_TRUE(m.isEditable(col)) << "column " << col;
+	}
+	EXPECT_THAT(m.primaryKeyColumns(tableA), ::testing::ElementsAre(0));
+	EXPECT_THAT(m.primaryKeyColumns(tableB), ::testing::ElementsAre(2));
+}
+
+TEST(ResultSourceMappingTest, join_where_only_one_table_has_pk_in_result)
+{
+	ResultSourceMapping m({src(tableA, 1), src(tableA, 2), src(tableB, 2)},
+		{pk(tableA, {1}), pk(tableB, {1})});
+
+	EXPECT_TRUE(m.isEditable(0));
+	EXPECT_TRUE(m.isEditable(1));
+	EXPECT_FALSE(m.isEditable(2));
+	EXPECT_TRUE(m.primaryKeyColumns(tableB).empty());
+}
+
+TEST(ResultSourceMappingTest, pk_missing_from_result_is_read_only)
+{
+	ResultSourceMapping m({src(tableA, 2), src(tableA, 3)}, {pk(tableA, {1})});
+
+	EXPECT_FALSE(m.isEditable(0));
+	EXPECT_FALSE(m.isEditable(1));
+	EXPECT_TRUE(m.primaryKeyColumns(tableA).empty());
+}
+
+TEST(ResultSourceMappingTest, composite_pk_partially_present_is_read_only)
+{
+	ResultSourceMapping m({src(tableA, 1), src(tableA, 3)}, {pk(tableA, {1, 2})});
+
+	EXPECT_FALSE(m.isEditable(0));
+	EXPECT_FALSE(m.isEditable(1));
+}
+
+TEST(ResultSourceMappingTest, table_without_pk_is_read_only)
+{
+	ResultSourceMapping m({src(tableA, 1), src(tableA, 2)}, {});
+
+	EXPECT_FALSE(m.isEditable(0));
+	EXPECT_FALSE(m.isEditable(1));
+}
+
+TEST(ResultSourceMappingTest, expression_column_is_read_only)
+{
+	ResultSourceMapping m({src(tableA, 1), expression(), src(tableA, 2)}, {pk(tableA, {1})});
+
+	EXPECT_TRUE(m.isEditable(0));
+	EXPECT_FALSE(m.isEditable(1));
+	EXPECT_EQ(m.tableOid(1), InvalidOid);
+	EXPECT_EQ(m.attnum(1), 0);
+	EXPECT_TRUE(m.isEditable(2));
+}
+
+TEST(ResultSourceMappingTest, system_column_is_read_only)
+{
+	ResultSourceMapping m({src(tableA, 1), src(tableA, -2)}, {pk(tableA, {1})});
+
+	EXPECT_TRUE(m.isEditable(0));
+	EXPECT_FALSE(m.isEditable(1));
+}
+
+TEST(ResultSourceMappingTest, duplicate_pk_column_still_editable)
+{
+	ResultSourceMapping m({src(tableA, 1), src(tableA, 1), src(tableA, 2)}, {pk(tableA, {1})});
+
+	EXPECT_TRUE(m.isEditable(0));
+	EXPECT_TRUE(m.isEditable(1));
+	EXPECT_TRUE(m.isEditable(2));
+	EXPECT_THAT(m.primaryKeyColumns(tableA), ::testing::ElementsAre(0));
+}
+
+TEST(ResultSourceMappingTest, same_non_pk_column_twice_makes_table_read_only)
+{
+	ResultSourceMapping m({src(tableA, 1), src(tableA, 2), src(tableA, 2)}, {pk(tableA, {1})});
+
+	EXPECT_FALSE(m.isEditable(0));
+	EXPECT_FALSE(m.isEditable(1));
+	EXPECT_FALSE(m.isEditable(2));
+	EXPECT_TRUE(m.primaryKeyColumns(tableA).empty());
+}
+
+TEST(ResultSourceMappingTest, self_join_with_aliased_non_pk_columns_is_read_only)
+{
+	ResultSourceMapping m({src(tableA, 1), src(tableA, 1), src(tableA, 2), src(tableA, 2)}, {pk(tableA, {1})});
+
+	EXPECT_FALSE(m.isEditable(0));
+	EXPECT_FALSE(m.isEditable(2));
+}
+
+TEST(ResultSourceMappingTest, out_of_range_column_throws)
+{
+	ResultSourceMapping m({src(tableA, 1)}, {pk(tableA, {1})});
+
+	EXPECT_THROW(m.isEditable(1), std::range_error);
+	EXPECT_THROW(m.isEditable(-1), std::range_error);
+	EXPECT_THROW(m.tableOid(1), std::range_error);
+	EXPECT_THROW(m.attnum(-1), std::range_error);
+}
+
+TEST(ResultSourceMappingTest, empty_result_has_no_columns)
+{
+	ResultSourceMapping m({}, {});
+
+	EXPECT_EQ(m.columnCount(), 0);
+	EXPECT_TRUE(m.primaryKeyColumns(tableA).empty());
+}