From 35eac713fff51bde8026c843b8ba45f0819e78a8 Mon Sep 17 00:00:00 2001 From: eelke Date: Sun, 18 Oct 2026 07:48:33 +0200 Subject: [PATCH 1/5] Implement ResultSourceMapping --- pglablib/CMakeLists.txt | 2 + pglablib/ResultSourceMapping.cpp | 139 +++++++++++++++ pglablib/ResultSourceMapping.h | 61 +++++++ tests/pglabtests/CMakeLists.txt | 1 + tests/pglabtests/ResultSourceMappingTest.cpp | 168 +++++++++++++++++++ 5 files changed, 371 insertions(+) create mode 100644 pglablib/ResultSourceMapping.cpp create mode 100644 pglablib/ResultSourceMapping.h create mode 100644 tests/pglabtests/ResultSourceMappingTest.cpp diff --git a/pglablib/CMakeLists.txt b/pglablib/CMakeLists.txt index a3c759d..1734d09 100644 --- a/pglablib/CMakeLists.txt +++ b/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 diff --git a/pglablib/ResultSourceMapping.cpp b/pglablib/ResultSourceMapping.cpp new file mode 100644 index 0000000..d597504 --- /dev/null +++ b/pglablib/ResultSourceMapping.cpp @@ -0,0 +1,139 @@ +#include "ResultSourceMapping.h" + +#include "Pgsql_Result.h" +#include "catalog/PgDatabaseCatalog.h" +#include "catalog/PgConstraintContainer.h" + +#include +#include + +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 &columns, Oid tableOid, const std::vector &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 columns, const std::vector &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(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(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 ResultSourceMapping::primaryKeyColumns(Oid tableOid) const +{ + const TableInfo *info = findTable(tableOid); + return info ? info->pkColumns : std::vector{}; +} + +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; +} + +ResultSourceMapping createResultSourceMapping(const Pgsql::Result &result, const PgDatabaseCatalog &catalog) +{ + std::vector columns; + std::vector 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; + 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); +} diff --git a/pglablib/ResultSourceMapping.h b/pglablib/ResultSourceMapping.h new file mode 100644 index 0000000..b12f162 --- /dev/null +++ b/pglablib/ResultSourceMapping.h @@ -0,0 +1,61 @@ +#ifndef RESULTSOURCEMAPPING_H +#define RESULTSOURCEMAPPING_H + +#include + +#include + +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 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 columns, const std::vector &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 primaryKeyColumns(Oid tableOid) const; + +private: + struct TableInfo { + Oid tableOid = InvalidOid; + bool editable = false; + std::vector pkColumns; + }; + + std::vector m_columns; + std::vector 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 diff --git a/tests/pglabtests/CMakeLists.txt b/tests/pglabtests/CMakeLists.txt index f13dc29..d9f30de 100644 --- a/tests/pglabtests/CMakeLists.txt +++ b/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 diff --git a/tests/pglabtests/ResultSourceMappingTest.cpp b/tests/pglabtests/ResultSourceMappingTest.cpp new file mode 100644 index 0000000..74565e8 --- /dev/null +++ b/tests/pglabtests/ResultSourceMappingTest.cpp @@ -0,0 +1,168 @@ +#include +#include + +#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 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()); +} From bbe52c9db8e36fd6a90b3f88b3cf1a4c8050434a Mon Sep 17 00:00:00 2001 From: eelke Date: Sun, 18 Oct 2026 10:17:38 +0200 Subject: [PATCH 2/5] Generalise CrudModel to mapping-based editing --- pglab/crud/CrudModel.cpp | 235 ++++++++++++++++++++++++++------------- pglab/crud/CrudModel.h | 30 +++-- 2 files changed, 174 insertions(+), 91 deletions(-) diff --git a/pglab/crud/CrudModel.cpp b/pglab/crud/CrudModel.cpp index 8367f32..8a8ce00 100644 --- a/pglab/crud/CrudModel.cpp +++ b/pglab/crud/CrudModel.cpp @@ -4,6 +4,7 @@ #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" @@ -39,12 +40,23 @@ void CrudModel::setConfig(std::shared_ptr 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 db, std::shared_ptr 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) @@ -232,9 +244,11 @@ void CrudModel::loadIntoModel(std::shared_ptr data) 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(); } @@ -277,7 +291,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; @@ -312,22 +326,17 @@ bool CrudModel::setData(const QModelIndex &index, const QVariant &value, int rol return false; } -std::tuple> CrudModel::saveRow(const PendingRow &pending_row) +std::tuple> 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); - else - { - Pgsql::Params pkey_params = getPKeyParamsForRow(pending_row.row()); - std::tie(buffer, params) = createUpdateQuery(pkey_params, pending_row); - } + std::tie(buffer, params) = createInsertQuery(pending_row); Pgsql::Connection db_update_conn; auto dbconfig = m_database->config(); @@ -356,59 +365,120 @@ std::tuple> CrudModel::saveRow(const Pending msgBox.setText(ex.what()); msgBox.exec(); } + return { false, {} }; + } + + return saveUpdate(pending_row, row_mapping); +} + +std::tuple> CrudModel::saveUpdate(const PendingRow &pending_row, const RowMapping &row_mapping) +{ + std::set 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 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; - for (auto attnum : m_primaryKey->key) - { - const int col = attNumToCol(attnum); - Oid t = getType(col); - auto s = getSavedData(col, row); - params.add(s, t); - } - return params; + auto table = m_database->catalog()->classes()->getByKey(table_oid); + return table ? table->fullyQualifiedQuotedObjectName() : QString(); } -QString CrudModel::columnName(int col) const +std::vector CrudModel::resultColumnsOfTable(Oid table_oid) const { - return m_roData->getColName(col); + std::vector 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 CrudModel::createUpdateQuery(const Pgsql::Params &pkey_params, const PendingRow &pending_row) +QString CrudModel::attributeName(int col) const { - Pgsql::Params params; - auto data = pending_row.data(); - QString table_name = m_table->fullyQualifiedQuotedObjectName(); - 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)); - } + if (col < static_cast(columnList.size()) && columnList[col]) + return columnList[col]->name; + return QString::fromUtf8(m_roData->getColName(col)); +} - q << "\nWHERE "; - int i = 0; - for (auto attnum : m_primaryKey->key) +Pgsql::Params CrudModel::getPKeyParamsForRow(const RowMapping &row_mapping, Oid table_oid) const +{ + Pgsql::Params params; + for (int col : m_mapping->primaryKeyColumns(table_oid)) { - int col = attNumToCol(attnum); - if (i > 0) - q << " AND "; - q << quoteIdent(columnName(col)) << "=$" << ++param; - ++i; - } - params.addParams(pkey_params); - q << "\nRETURNING *"; - q.flush(); - return { buffer, params }; + auto s = getSavedData(row_mapping, col); + params.add(s, getType(col)); + } + return params; +} + +std::tuple CrudModel::createUpdateQuery(Oid table_oid, const PendingRow &pending_row, const RowMapping &row_mapping) +{ + Pgsql::Params 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 CrudModel::createInsertQuery(const PendingRow &pending_row) @@ -424,12 +494,11 @@ std::tuple 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 +511,6 @@ std::tuple CrudModel::createInsertQuery(const PendingRow return { buffer, params }; } -std::tuple 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 +521,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 +538,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 +577,13 @@ bool CrudModel::columnIsReadOnly(int column_index) const if (m_roData == nullptr) return true; - auto&& column = columnList[column_index - PreColumnCount]; - return column.getIdentity() == PgAttribute::Identity::Always - || column.getGenerated() != PgAttribute::Generated::None; + int col = column_index - PreColumnCount; + if (!m_mapping || !m_mapping->isEditable(col)) + return true; + + const auto &column = columnList[col]; + return !column || column->getIdentity() == PgAttribute::Identity::Always + || column->getGenerated() != PgAttribute::Generated::None; } void CrudModel::initializeColumnList() @@ -526,13 +591,20 @@ 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()); - columnList.push_back(*find_result); + if (find_result != columns.end()) + columnList.emplace_back(*find_result); + else + columnList.emplace_back(); } } @@ -548,6 +620,9 @@ void CrudModel::revert() void CrudModel::appendNewRow() { + if (!m_wholeTable) + return; + int row = static_cast(m_rowMapping.size()); beginInsertRows(QModelIndex(), row, row); appendNewRowInternal(); @@ -565,6 +640,8 @@ std::tuple CrudModel::removeRows(const std::set return { true, "" }; if (row_ranges.rbegin()->end() > static_cast(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 +666,7 @@ std::tuple CrudModel::removeRows(const std::set for (int current_row = range.start(); current_row < range.end(); ++current_row) { auto&& mapping = m_rowMapping[static_cast(current_row)]; - auto params = getPKeyParamsForRow(mapping.rowKey); + auto params = getPKeyParamsForRow(mapping, m_table->oid()); if (!params.empty()) db_update_conn.queryParam(delete_statement, params); } diff --git a/pglab/crud/CrudModel.h b/pglab/crud/CrudModel.h index 5a6beb3..d05b6c0 100644 --- a/pglab/crud/CrudModel.h +++ b/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 #include #include @@ -26,6 +28,8 @@ public: ~CrudModel() override; void setConfig(std::shared_ptr 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 db, std::shared_ptr result); QVariant headerData(int section, Qt::Orientation orientation, int role = Qt::DisplayRole) const override; @@ -50,7 +54,6 @@ public slots: virtual void revert() override; private: - using PKeyValues = std::vector; const int PreColumnCount = 1; @@ -76,6 +79,7 @@ private: res += " NULLS FIRST"; else if (nulls == NullSorting::Last) res += " NULLS LAST"; + return res; } }; @@ -205,10 +209,11 @@ private: std::shared_ptr m_database; std::optional m_table; - std::optional m_primaryKey; + std::optional 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 m_roData; @@ -218,7 +223,7 @@ private: // identity allways or generated }; - std::vector columnList; // list of columnMeta 1 to 1 with columns in m_roData. + std::vector> columnList; // attribute per column in m_roData, empty for expression columns PendingRowList m_pendingRowList; @@ -238,29 +243,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 createUpdateQuery(const Pgsql::Params &pkey_params, const PendingRow &pending_row); + std::tuple createUpdateQuery(Oid table_oid, const PendingRow &pending_row, const RowMapping &row_mapping); std::tuple createInsertQuery(const PendingRow &pending_row); - std::tuple createDeleteStatement(const PKeyValues &pkey_values); QString createDeleteStatement() const; - std::tuple> saveRow(const PendingRow &pending_row); + std::tuple> saveRow(const PendingRow &pending_row, const RowMapping &row_mapping); + std::tuple> 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 - /// - /// \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; } + /// Name of the table with the given oid, quoted and schema qualified + QString tableName(Oid table_oid) const; + /// Result columns that are user columns (attnum > 0) of the given table, in result order + std::vector 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> &row_ranges); void RemoveRangeOfRowsFromModel(IntegerRange row_range); From 5e7fb68c55b308332f92915636f692cbbe88602e Mon Sep 17 00:00:00 2001 From: eelke Date: Sun, 18 Oct 2026 10:29:24 +0200 Subject: [PATCH 3/5] QueryTool now uses the same DataGrid as the CRUD tool. --- pglab/CMakeLists.txt | 2 +- pglab/crud/CrudModel.cpp | 3 + pglab/crud/CrudTab.cpp | 38 ++++----- pglab/crud/CrudTab.h | 4 +- pglab/crud/CrudTab.ui | 10 +-- pglab/crud/DataGridWidget.cpp | 34 ++++++++ pglab/crud/DataGridWidget.h | 28 ++++++ pglab/querytool/QueryResultModel.cpp | 85 ------------------- pglab/querytool/QueryResultModel.h | 31 ------- pglab/querytool/QueryTool.cpp | 4 +- pglab/querytool/QueryTool.h | 2 - pglab/tuplesresultwidget.cpp | 35 +++++--- pglab/tuplesresultwidget.h | 19 +++-- pglab/tuplesresultwidget.ui | 31 ------- ...esults-editable-grid-b265a193a39f3245.yaml | 12 +++ 15 files changed, 135 insertions(+), 203 deletions(-) create mode 100644 pglab/crud/DataGridWidget.cpp create mode 100644 pglab/crud/DataGridWidget.h delete mode 100644 pglab/querytool/QueryResultModel.cpp delete mode 100644 pglab/querytool/QueryResultModel.h create mode 100644 releasenotes/notes/query-results-editable-grid-b265a193a39f3245.yaml diff --git a/pglab/CMakeLists.txt b/pglab/CMakeLists.txt index ab4dda5..fb3d3f2 100644 --- a/pglab/CMakeLists.txt +++ b/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 diff --git a/pglab/crud/CrudModel.cpp b/pglab/crud/CrudModel.cpp index 8a8ce00..a8bcae1 100644 --- a/pglab/crud/CrudModel.cpp +++ b/pglab/crud/CrudModel.cpp @@ -10,6 +10,7 @@ #include "CustomDataRole.h" #include #include +#include #include #include "Pgsql_oids.h" #include "Pgsql_PgException.h" @@ -217,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 {}; } diff --git a/pglab/crud/CrudTab.cpp b/pglab/crud/CrudTab.cpp index a776879..88b0ac2 100644 --- a/pglab/crud/CrudTab.cpp +++ b/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 +#include #include +#include #include #include #include +#include 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_SortFilterProxy->setSourceModel(m_crudModel); - m_SortFilterProxy->setSortRole(CustomSortRole); - ui->tableView->setModel(m_SortFilterProxy); - ui->tableView->setSortingEnabled(true); - m_SortFilterProxy->sort(0, Qt::AscendingOrder); + m_grid = new DataGridWidget(this); + ui->verticalLayout->addWidget(m_grid); + m_grid->setSourceModel(m_crudModel); + m_grid->view()->verticalHeader()->setVisible(false); - ui->tableView->setSelectionMode(QAbstractItemView::ExtendedSelection); - ui->tableView->setSelectionBehavior(QAbstractItemView::SelectRows); - ui->tableView->addAction(ui->actionRemove_rows); + m_grid->view()->setSelectionMode(QAbstractItemView::ExtendedSelection); + m_grid->view()->setSelectionBehavior(QAbstractItemView::SelectRows); + 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); @@ -70,7 +66,7 @@ void CrudTab::refresh() void CrudTab::on_actionRemove_rows_triggered() { std::set> 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 +106,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 +118,9 @@ void CrudTab::gotoColumn(QString column) auto name = m_crudModel->headerData(i, Qt::Horizontal).toString(); if (name == column) { - auto index = ui->tableView->currentIndex(); - index = ui->tableView->model()->index(index.row(), i); - ui->tableView->setCurrentIndex(index); + auto index = m_grid->view()->currentIndex(); + index = m_grid->view()->model()->index(index.row(), i); + m_grid->view()->setCurrentIndex(index); break; } } diff --git a/pglab/crud/CrudTab.h b/pglab/crud/CrudTab.h index ef91325..a6bf0fc 100644 --- a/pglab/crud/CrudTab.h +++ b/pglab/crud/CrudTab.h @@ -1,7 +1,6 @@ #ifndef CRUDTAB_H #define CRUDTAB_H -#include #include "catalog/PgClass.h" #include "IDatabaseWindow.h" #include @@ -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 m_table; CrudModel *m_crudModel = nullptr; - QSortFilterProxyModel *m_SortFilterProxy = nullptr; + DataGridWidget *m_grid = nullptr; QMenu *headerContextMenu = nullptr; diff --git a/pglab/crud/CrudTab.ui b/pglab/crud/CrudTab.ui index 8c58abd..f3118a4 100644 --- a/pglab/crud/CrudTab.ui +++ b/pglab/crud/CrudTab.ui @@ -13,15 +13,7 @@ Form - - - - - false - - - - + Remove rows diff --git a/pglab/crud/DataGridWidget.cpp b/pglab/crud/DataGridWidget.cpp new file mode 100644 index 0000000..64f19a8 --- /dev/null +++ b/pglab/crud/DataGridWidget.cpp @@ -0,0 +1,34 @@ +#include "DataGridWidget.h" +#include "CustomDataRole.h" +#include "ResultTableModelUtil.h" +#include "util/PgLabItemDelegate.h" +#include +#include +#include + + +DataGridWidget::DataGridWidget(QWidget *parent) + : QWidget(parent) +{ + auto layout = new QVBoxLayout(this); + layout->setContentsMargins(0, 0, 0, 0); + + 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); + } + m_proxy->setSourceModel(model); + m_view->setModel(m_proxy); + m_view->setSortingEnabled(true); + m_proxy->sort(0, Qt::AscendingOrder); +} diff --git a/pglab/crud/DataGridWidget.h b/pglab/crud/DataGridWidget.h new file mode 100644 index 0000000..11d6745 --- /dev/null +++ b/pglab/crud/DataGridWidget.h @@ -0,0 +1,28 @@ +#ifndef DATAGRIDWIDGET_H +#define DATAGRIDWIDGET_H + +#include + +class QAbstractItemModel; +class QSortFilterProxyModel; +class QTableView; + +/** 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: + QTableView *m_view = nullptr; + QSortFilterProxyModel *m_proxy = nullptr; +}; + +#endif // DATAGRIDWIDGET_H diff --git a/pglab/querytool/QueryResultModel.cpp b/pglab/querytool/QueryResultModel.cpp deleted file mode 100644 index 360a00c..0000000 --- a/pglab/querytool/QueryResultModel.cpp +++ /dev/null @@ -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 -#include - -using namespace Pgsql; - -QueryResultModel::QueryResultModel(QObject *parent, std::shared_ptr r - , std::shared_ptr 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; -} diff --git a/pglab/querytool/QueryResultModel.h b/pglab/querytool/QueryResultModel.h deleted file mode 100644 index 7de2f70..0000000 --- a/pglab/querytool/QueryResultModel.h +++ /dev/null @@ -1,31 +0,0 @@ -#ifndef QUERYRESULTMODEL_H -#define QUERYRESULTMODEL_H - -#include -#include "catalog/models/BaseTableModel.h" -#include "Pgsql_Connection.h" - -class PgDatabaseCatalog; - -class QueryResultModel : public QAbstractTableModel -{ - Q_OBJECT -public: - QueryResultModel(QObject *parent, std::shared_ptr r, std::shared_ptr 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 GetPgsqlResult() const { return result; } -protected: -// virtual Oid getType(int column) const override; -// virtual QVariant getData(const QModelIndex &index) const override; -private: - std::shared_ptr result; - std::shared_ptr catalog; -}; - -#endif // QUERYRESULTMODEL_H diff --git a/pglab/querytool/QueryTool.cpp b/pglab/querytool/QueryTool.cpp index 153b19c..f808335 100644 --- a/pglab/querytool/QueryTool.cpp +++ b/pglab/querytool/QueryTool.cpp @@ -476,10 +476,8 @@ void QueryTool::query_ready(std::shared_ptr dbres, qint64 elapsed //int n_rows = dbres->getRows(); //QString rowcount_str = QString("rows: %1").arg(dbres->getRows()); - auto result_model = std::make_shared(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) diff --git a/pglab/querytool/QueryTool.h b/pglab/querytool/QueryTool.h index 5cb98f0..1274642 100644 --- a/pglab/querytool/QueryTool.h +++ b/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; diff --git a/pglab/tuplesresultwidget.cpp b/pglab/tuplesresultwidget.cpp index 1e2dd5b..21b090a 100644 --- a/pglab/tuplesresultwidget.cpp +++ b/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 #include -#include "util/PgLabItemDelegate.h" +#include +#include TuplesResultWidget::TuplesResultWidget(QWidget *parent) : @@ -15,9 +19,13 @@ TuplesResultWidget::TuplesResultWidget(QWidget *parent) : ui->setupUi(this); ui->lblRowCount->setText(QString()); - SetTableViewDefault(ui->ResultView); - auto delegate = new PgLabItemDelegate(ui->ResultView); - ui->ResultView->setItemDelegate(delegate); + m_grid = new DataGridWidget(this); + ui->verticalLayout->insertWidget(0, m_grid); + 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 res, float ms) +void TuplesResultWidget::setResult(std::shared_ptr db, std::shared_ptr res, float ms) { - resultModel = res; - ui->ResultView->setModel(resultModel.get()); + m_result = res; - 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); } } - - diff --git a/pglab/tuplesresultwidget.h b/pglab/tuplesresultwidget.h index 89a55d9..19fc083 100644 --- a/pglab/tuplesresultwidget.h +++ b/pglab/tuplesresultwidget.h @@ -1,13 +1,21 @@ #ifndef TUPLESRESULTWIDGET_H #define TUPLESRESULTWIDGET_H -#include "querytool/QueryResultModel.h" #include +#include 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 res, float ms); + void setResult(std::shared_ptr db, std::shared_ptr res, float ms); void exportData(const QString &file_name) const; - std::shared_ptr GetPgsqlResult() const { return resultModel->GetPgsqlResult(); } + std::shared_ptr GetPgsqlResult() const { return m_result; } private: Ui::TuplesResultWidget *ui; - - std::shared_ptr resultModel; + DataGridWidget *m_grid = nullptr; + CrudModel *m_model = nullptr; + std::shared_ptr m_result; }; #endif // TUPLESRESULTWIDGET_H diff --git a/pglab/tuplesresultwidget.ui b/pglab/tuplesresultwidget.ui index 677a464..b13909e 100644 --- a/pglab/tuplesresultwidget.ui +++ b/pglab/tuplesresultwidget.ui @@ -29,37 +29,6 @@ 2 - - - - - Source Sans Pro - 10 - - - - QAbstractItemView::DoubleClicked|QAbstractItemView::EditKeyPressed - - - true - - - QAbstractItemView::ScrollPerPixel - - - QAbstractItemView::ScrollPerPixel - - - false - - - 21 - - - 16 - - - diff --git a/releasenotes/notes/query-results-editable-grid-b265a193a39f3245.yaml b/releasenotes/notes/query-results-editable-grid-b265a193a39f3245.yaml new file mode 100644 index 0000000..14be544 --- /dev/null +++ b/releasenotes/notes/query-results-editable-grid-b265a193a39f3245.yaml @@ -0,0 +1,12 @@ +--- +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. +other: + - | + Read-only cells in query results, such as expression columns or columns of + tables without a primary key in the result, are now shown in grey so they + can be told apart from editable cells. From 66267334c82fce58d5afb6508e3984901df8dc9e Mon Sep 17 00:00:00 2001 From: eelke Date: Sun, 18 Oct 2026 10:35:39 +0200 Subject: [PATCH 4/5] We should not allow editing of system catalogs. --- pglablib/ResultSourceMapping.cpp | 26 ++++++++++++++++--- ...esults-editable-grid-b265a193a39f3245.yaml | 9 ++++--- 2 files changed, 28 insertions(+), 7 deletions(-) diff --git a/pglablib/ResultSourceMapping.cpp b/pglablib/ResultSourceMapping.cpp index d597504..8197133 100644 --- a/pglablib/ResultSourceMapping.cpp +++ b/pglablib/ResultSourceMapping.cpp @@ -1,8 +1,12 @@ #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 #include @@ -107,6 +111,20 @@ const ResultSourceMapping::TableInfo *ResultSourceMapping::findTable(Oid tableOi 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 columns; @@ -129,9 +147,11 @@ ResultSourceMapping createResultSourceMapping(const Pgsql::Result &result, const TablePrimaryKey pk; pk.tableOid = source.tableOid; - auto constraint = catalog.constraints()->getPrimaryForRelation(source.tableOid); - if (constraint) - pk.attnums.assign(constraint->key.begin(), constraint->key.end()); + 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); } diff --git a/releasenotes/notes/query-results-editable-grid-b265a193a39f3245.yaml b/releasenotes/notes/query-results-editable-grid-b265a193a39f3245.yaml index 14be544..a54b9f5 100644 --- a/releasenotes/notes/query-results-editable-grid-b265a193a39f3245.yaml +++ b/releasenotes/notes/query-results-editable-grid-b265a193a39f3245.yaml @@ -4,9 +4,10 @@ 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. + 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 or columns of - tables without a primary key in the result, are now shown in grey so they - can be told apart from editable cells. + 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. From ee1a8ba48dea2ea96ada9e75bfb05d78150889c4 Mon Sep 17 00:00:00 2001 From: eelke Date: Sun, 18 Oct 2026 11:34:00 +0200 Subject: [PATCH 5/5] Add save and revert button to datagrid no implicit saves any more. --- .idea/QtSettings.xml | 6 ++ .idea/misc.xml | 7 ++ .idea/pgLab.iml | 8 +-- pglab/crud/CrudModel.cpp | 30 ++++++++- pglab/crud/CrudModel.h | 13 +++- pglab/crud/CrudTab.cpp | 7 ++ pglab/crud/DataGridWidget.cpp | 67 +++++++++++++++++++ pglab/crud/DataGridWidget.h | 14 ++++ ...-save-revert-toolbar-3f9c1a7e5b2d8604.yaml | 13 ++++ 9 files changed, 153 insertions(+), 12 deletions(-) create mode 100644 .idea/QtSettings.xml create mode 100644 .idea/misc.xml create mode 100644 releasenotes/notes/datagrid-save-revert-toolbar-3f9c1a7e5b2d8604.yaml diff --git a/.idea/QtSettings.xml b/.idea/QtSettings.xml new file mode 100644 index 0000000..0248d0c --- /dev/null +++ b/.idea/QtSettings.xml @@ -0,0 +1,6 @@ + + + + + \ No newline at end of file diff --git a/.idea/misc.xml b/.idea/misc.xml new file mode 100644 index 0000000..0b76fe5 --- /dev/null +++ b/.idea/misc.xml @@ -0,0 +1,7 @@ + + + + + + \ No newline at end of file diff --git a/.idea/pgLab.iml b/.idea/pgLab.iml index 6102194..4c94235 100644 --- a/.idea/pgLab.iml +++ b/.idea/pgLab.iml @@ -1,8 +1,2 @@ - - - - - - - \ No newline at end of file + \ No newline at end of file diff --git a/pglab/crud/CrudModel.cpp b/pglab/crud/CrudModel.cpp index a8bcae1..c30a1bc 100644 --- a/pglab/crud/CrudModel.cpp +++ b/pglab/crud/CrudModel.cpp @@ -244,6 +244,7 @@ void CrudModel::loadData() void CrudModel::loadIntoModel(std::shared_ptr data) { beginResetModel(); + bool was_dirty = hasPendingChanges(); m_pendingRowList.clear(); m_roData = data; initializeColumnList(); @@ -253,6 +254,8 @@ void CrudModel::loadIntoModel(std::shared_ptr data) if (m_wholeTable) appendNewRowInternal(); endResetModel(); + if (was_dirty) + emit pendingChangesChanged(false); } void CrudModel::initRowMapping() @@ -321,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() << role); return true; @@ -611,14 +617,27 @@ void CrudModel::initializeColumnList() } } -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() @@ -678,7 +697,10 @@ std::tuple CrudModel::removeRows(const std::set // 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) @@ -713,6 +735,8 @@ void CrudModel::RemoveRangeOfRowsFromModel(IntegerRange 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()); } diff --git a/pglab/crud/CrudModel.h b/pglab/crud/CrudModel.h index d05b6c0..485b6c2 100644 --- a/pglab/crud/CrudModel.h +++ b/pglab/crud/CrudModel.h @@ -46,12 +46,21 @@ 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 removeRows(const std::set> &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: diff --git a/pglab/crud/CrudTab.cpp b/pglab/crud/CrudTab.cpp index 88b0ac2..8bb44b0 100644 --- a/pglab/crud/CrudTab.cpp +++ b/pglab/crud/CrudTab.cpp @@ -60,6 +60,13 @@ void CrudTab::setConfig(Oid oid) //std::shared_ptr 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(); } diff --git a/pglab/crud/DataGridWidget.cpp b/pglab/crud/DataGridWidget.cpp index 64f19a8..65708f7 100644 --- a/pglab/crud/DataGridWidget.cpp +++ b/pglab/crud/DataGridWidget.cpp @@ -1,9 +1,13 @@ #include "DataGridWidget.h" +#include "CrudModel.h" #include "CustomDataRole.h" #include "ResultTableModelUtil.h" #include "util/PgLabItemDelegate.h" +#include #include +#include #include +#include #include @@ -13,6 +17,25 @@ DataGridWidget::DataGridWidget(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); @@ -27,8 +50,52 @@ void DataGridWidget::setSourceModel(QAbstractItemModel *model) 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(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(m_proxy->sourceModel()); + if (!crud) + return; + + commitOpenEditor(); + crud->saveChanges(); +} + +void DataGridWidget::onRevert() +{ + auto crud = qobject_cast(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(); } diff --git a/pglab/crud/DataGridWidget.h b/pglab/crud/DataGridWidget.h index 11d6745..d605053 100644 --- a/pglab/crud/DataGridWidget.h +++ b/pglab/crud/DataGridWidget.h @@ -4,8 +4,10 @@ #include class QAbstractItemModel; +class QAction; class QSortFilterProxyModel; class QTableView; +class QToolBar; /** Sortable and editable grid shared by the CRUD tab and the query result tab. */ @@ -20,9 +22,21 @@ public: 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 diff --git a/releasenotes/notes/datagrid-save-revert-toolbar-3f9c1a7e5b2d8604.yaml b/releasenotes/notes/datagrid-save-revert-toolbar-3f9c1a7e5b2d8604.yaml new file mode 100644 index 0000000..ebfdb20 --- /dev/null +++ b/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.