CrudModel::removeRows now works for count > 1

This allows for efficient removal of blocks of rows. If you would remove
row by row the repeated beginRemoveRows and endRemoveRows calls will generate
additional overhead.

pglab/CrudModel.cpp

@@ -15,6 +15,7 @@
 #include "Pgsql_oids.h"
 #include "Pgsql_Params.h"
 #include <string>
+#include "ScopeGuard.h"
 
 CrudModel::CrudModel(ASyncWindow *async_window)
 	: m_asyncWindow(async_window)
@@ -272,13 +273,13 @@ CrudModel::PKeyValues CrudModel::getPKeyForRow(int row) const
 	auto mod_row = getModifiedRow(row);
 	if (mod_row){
 		for (auto attnum : m_primaryKey->key) {
-			int col = attnum - 1; // Assume column ordering matches table, also we assume know special columns like oid are shown
+			const int col = attNumToCol(attnum);
 			values.push_back(*(mod_row->data()[col]));
 		}
 	}
 	else if (row < m_roData->rows()){
 		for (auto attnum : m_primaryKey->key) {
-			int col = attnum - 1; // Assume column ordering matches table, also we assume know special columns like oid are shown
+			int col = attNumToCol(attnum);
 			values.push_back(m_roData->get(col, row).c_str());
 		}
 	}
@@ -286,6 +287,26 @@ CrudModel::PKeyValues CrudModel::getPKeyForRow(int row) const
 	return values;
 }
 
+Pgsql::Params CrudModel::getPKeyParamsForRow(int row) const
+{
+	Pgsql::Params params;
+	auto mod_row = getModifiedRow(row);
+	for (auto attnum : m_primaryKey->key) {
+		const int col = attNumToCol(attnum);
+		Oid t = getType(col);
+		std::string s;
+		if (mod_row){
+			s = *(mod_row->data()[col]);
+		}
+		else if (row < m_roData->rows()){
+			s = m_roData->get(col, row).c_str();
+		}
+		params.add(s, t);
+	}
+	return params;
+}
+
+
 QString CrudModel::columnName(int col) const
 {
 	return m_roData->getColName(col);
@@ -312,7 +333,7 @@ std::tuple<QString, Pgsql::Params> CrudModel::createUpdateQuery(const PKeyValues
 	q << "\nWHERE ";
 	int i = 0;
 	for (auto attnum : m_primaryKey->key) {
-		int col = attnum - 1; // Assume column ordering matches table, also we assume know special columns like oid are shown
+		int col = attNumToCol(attnum);
 		if (i > 0)
 			q << " AND ";
 		q << quoteIdent(columnName(col)) << "=$" << ++param;
@@ -362,23 +383,34 @@ std::tuple<QString, Pgsql::Params> CrudModel::createInsertQuery(const PendingRow
 std::tuple<QString, Pgsql::Params> CrudModel::createDeleteStatement(const PKeyValues &pkey_values)
 {
 	Pgsql::Params params;
-	QString table_name = m_table->fullyQualifiedQuotedObjectName(); // genFQTableName(*m_database->catalog(), *m_table);
+	size_t i = 0;
-	QString buffer;
-	QTextStream q(&buffer);
-	q << "DELETE FROM " << table_name;
-	q << "\nWHERE ";
-	int i = 0, param = 0;
 	for (auto attnum : m_primaryKey->key) {
-		int col = attnum - 1; // Assume column ordering matches table, also we assume know special columns like oid are shown
+		const int col = attNumToCol(attnum);
-		if (i > 0)
-			q << " AND ";
-		q << quoteIdent(columnName(col)) << "=$" << ++param;
 		params.add(pkey_values[i].c_str(), getType(col));
 		++i;
 	}
 
+	return { createDeleteStatement(), params };
+}
+
+QString CrudModel::createDeleteStatement() const
+{
+	Pgsql::Params params;
+	QString table_name = m_table->fullyQualifiedQuotedObjectName();
+	QString buffer;
+	QTextStream q(&buffer);
+	q << "DELETE FROM " << table_name;
+	q << "\nWHERE ";
+	int i = 0;
+	for (auto attnum : m_primaryKey->key) {
+		const int col = attNumToCol(attnum);
+		if (i > 0)
+			q << " AND ";
+		q << quoteIdent(columnName(col)) << "=$" << ++i;
+	}
+
 	q.flush();
-	return { buffer, params };
+	return buffer;
 }
 
 
@@ -485,7 +517,6 @@ void CrudModel::removeRows()
 
 bool CrudModel::removeRows(int row, int count, const QModelIndex &parent)
 {
-	if (count > 1) return false;
 	if (m_rowMapping.empty()) return false;
 	// When removing rows there is no direct mapping anymore between the rows in the grid
 	// and the rows in m_roData
@@ -496,39 +527,42 @@ bool CrudModel::removeRows(int row, int count, const QModelIndex &parent)
 
 	// 1. Get PKEY and remove that row from table
 
+	Pgsql::Connection db_update_conn;
+	auto dbconfig = m_database->config();
+	bool res = db_update_conn.connect(dbconfig.getKeywords(), dbconfig.getValues(), false);
+	if (!res) {
+		return false;
+	}
 
-	auto mapping = m_rowMapping[row];
+	// First delete rows in table
-	// Get PKey for row
+	QString delete_statement = createDeleteStatement();
-	auto pkey_values = getPKeyForRow(mapping.rowKey);
+	db_update_conn.query("BEGIN;");
-	if (!pkey_values.empty()) {
+	for (int current_row = row; current_row < row + count; ++current_row) {
-		// Generate DELETE
+		auto&& mapping = m_rowMapping[static_cast<size_t>(current_row)];
-	//	QString buffer;
+		auto params = getPKeyParamsForRow(mapping.rowKey);
-	//	Pgsql::Params params;
+		if (!params.empty()) {
-		auto [buffer, params] = createDeleteStatement(pkey_values);
+			// Execute DELETE
-		// Execute DELETE
+			auto result = db_update_conn.queryParam(delete_statement, params);
-		Pgsql::Connection db_update_conn;
-		auto dbconfig = m_database->config();
-		bool res = db_update_conn.connect(dbconfig.getKeywords(), dbconfig.getValues(), false);
-		if (res) {
-			auto result = db_update_conn.queryParam(buffer, params);
-			if (result) {
-
-				beginRemoveRows(parent, row, row);
-
-				// 2. remove that row from m_rowMapping
-				m_rowMapping.erase(m_rowMapping.begin() + row);
-
-
-				// 3. if the row is in modified it can be removed from modified
-				if (mapping.modified) {
-					m_modifiedRowList.erase(mapping.rowKey);
-				}
-				// 4. can it be pending? should be removed if it is.
-
-				endRemoveRows();
-				return true;
-			}
 		}
 	}
-	return false;
+	db_update_conn.query("COMMIT;");
+	// Then from model
+	{
+		beginRemoveRows(parent, row, row);
+		SCOPE_EXIT { endRemoveRows(); };
+		for (int current_row = row; current_row < row + count; ++current_row) {
+			auto&& mapping = m_rowMapping[static_cast<size_t>(current_row)];
+
+			// if the row is in modified it can be removed from modified
+			if (mapping.modified) {
+				m_modifiedRowList.erase(mapping.rowKey);
+			}
+			/// \todo can it be pending? should be removed if it is.
+
+		}
+		// remove the rows from m_rowMapping
+		auto first = m_rowMapping.begin() + row;
+		m_rowMapping.erase(first, first + count);
+	}
+	return true;
 }

pglab/CrudModel.h

@@ -297,18 +297,25 @@ private:
 	/// \return The values of the primary key column. If this is a new row it will return an empty list
 	///
 	PKeyValues getPKeyForRow(int row) const;
+	Pgsql::Params getPKeyParamsForRow(int row) const;
 
 	bool savePendingChanges();
 
 	std::tuple<QString, Pgsql::Params> createUpdateQuery(const PKeyValues &pkey_values, const PendingRow &pending_row);
 	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, ModifiedRow> updateRow(const PendingRow &pending_row);
 
 	void appendNewRow();
 
 	int lastRowKey = 0;
 	int allocNewRowKey() { return ++lastRowKey; }
+	/// Convert an attnum from the database catalog to the corresponding column in the model
+	///
+	/// \todo still assumes columns are in order, all being shown and no special column like oid shown.
+	int attNumToCol(int attnum) const { return attnum - 1; }
 private slots:
 
 	void connectionStateChanged(ASyncDBConnection::State state);