diff --git a/CHANGELOG.md b/CHANGELOG.md index 512bd192f..f1028c305 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -55,6 +55,15 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 - A new row of nothing but server-assigned columns silently dropped from the save. - `OVERRIDING SYSTEM VALUE` and `setval` in a SQL export of a SQL Server database. - PGlite treated as a generic SQL dialect, so `$$` bodies split at their inner semicolons. +- Paste, Fill Column and the row inspector staging an edit to a column the server owns, which the save then discarded. (#2588) +- A rerun answered from cache adopting another pinned result's column metadata. (#2588) +- Add Row and Duplicate Row offered before a table's schema has loaded, when nothing yet knows which columns the server fills in. (#2588) +- Missing `SET IDENTITY_INSERT` around a SQL Server table's rows in a SQL export, so the dump could not be restored. (#2588) +- Fill Column offered on a generated or identity column. +- Pasted rows carrying a value for an identity or generated column, which the save then discarded. (#2588) +- Row inspector offering an editor for a column the server owns, leaving an edit that Save could never clear. (#2588) +- Row inspector editing a column the driver marks immutable, such as MongoDB's `_id`. +- Add Row and Duplicate Row inert for good on a result the user switched away from while its schema was loading. (#2588) ## [0.69.0] - 2026-08-27 diff --git a/Plugins/SQLExportPlugin/SQLExportPlugin.swift b/Plugins/SQLExportPlugin/SQLExportPlugin.swift index 24f15f07e..3c22f4ac4 100644 --- a/Plugins/SQLExportPlugin/SQLExportPlugin.swift +++ b/Plugins/SQLExportPlugin/SQLExportPlugin.swift @@ -488,6 +488,11 @@ final class SQLExportPlugin: ExportFormatPlugin, SettablePlugin, @unchecked Send && columnInfo.contains { $0.identityKind == .always } let tableRef = qualifiedRef( schema: table.databaseName, table: table.name, dataSource: dataSource) + /// SQL Server refuses an explicit value for an IDENTITY column unless the table is opened + /// for it first. The rows are exported with their keys, so without this the dump restores + /// nothing: every INSERT for the table is rejected while the export itself reported success. + let needsIdentityInsert = dataSource.databaseTypeId == "SQL Server" + && columnInfo.contains(where: \.isIdentity) let stream = dataSource.streamRows(table: table.name, databaseName: table.databaseName) for try await element in stream { @@ -501,6 +506,9 @@ final class SQLExportPlugin: ExportFormatPlugin, SettablePlugin, @unchecked Send for row in rows { rowBatch.append(row) if rowBatch.count >= batchSize { + if needsIdentityInsert, !wroteAnyRows { + try fileHandle.write(contentsOf: "SET IDENTITY_INSERT \(tableRef) ON;\n".toUTF8Data()) + } try writeInsertStatements( tableRef: tableRef, columns: columns, @@ -521,6 +529,9 @@ final class SQLExportPlugin: ExportFormatPlugin, SettablePlugin, @unchecked Send } if !rowBatch.isEmpty { + if needsIdentityInsert, !wroteAnyRows { + try fileHandle.write(contentsOf: "SET IDENTITY_INSERT \(tableRef) ON;\n".toUTF8Data()) + } try writeInsertStatements( tableRef: tableRef, columns: columns, @@ -536,6 +547,10 @@ final class SQLExportPlugin: ExportFormatPlugin, SettablePlugin, @unchecked Send wroteAnyRows = true } + if wroteAnyRows, needsIdentityInsert { + try fileHandle.write(contentsOf: "SET IDENTITY_INSERT \(tableRef) OFF;\n".toUTF8Data()) + } + if wroteAnyRows { try fileHandle.write(contentsOf: "\n".toUTF8Data()) } diff --git a/TablePro/Core/ChangeTracking/DataChangeManager.swift b/TablePro/Core/ChangeTracking/DataChangeManager.swift index 715df10de..9ae47857e 100644 --- a/TablePro/Core/ChangeTracking/DataChangeManager.swift +++ b/TablePro/Core/ChangeTracking/DataChangeManager.swift @@ -128,6 +128,16 @@ final class DataChangeManager: ChangeManaging { self.generatedColumns = generatedColumns } + /// Whether the app may send a value for this column at all: the server computes or allocates it, + /// or the driver declares it immutable, as MongoDB does for `_id`. Both halves belong here, + /// because this is the boundary every staging path crosses and the grid's own copy of the + /// question does not cover the paths that reach the model directly. + func isColumnWritable(_ columnName: String) -> Bool { + guard !generatedColumns.contains(columnName) else { return false } + guard let databaseType else { return true } + return !PluginManager.shared.immutableColumns(for: databaseType).contains(columnName) + } + // MARK: - Change Tracking func recordCellChange( @@ -138,6 +148,17 @@ final class DataChangeManager: ChangeManaging { newValue: PluginCellValue, originalRow: [PluginCellValue]? = nil ) { + /// The last gate before a change becomes pending, and the only one every path crosses. The + /// grid's own check covers the inline editor and the Set Value menu; paste, Fill Column and + /// the row inspector reach here directly, so a column the server owns could be staged, be + /// filtered out again during statement generation, and be cleared by a save that reported + /// success over the changes it did write. + guard isColumnWritable(columnName) else { + Self.logger.warning( + "Refusing an edit to server-owned column '\(columnName, privacy: .public)' in table '\(self.tableName, privacy: .public)'" + ) + return + } let recorded = pending.recordCellChange( rowIndex: rowIndex, columnIndex: columnIndex, diff --git a/TablePro/Core/Coordinators/QueryExecutionCoordinator+Helpers.swift b/TablePro/Core/Coordinators/QueryExecutionCoordinator+Helpers.swift index 00c308020..ec855807f 100644 --- a/TablePro/Core/Coordinators/QueryExecutionCoordinator+Helpers.swift +++ b/TablePro/Core/Coordinators/QueryExecutionCoordinator+Helpers.swift @@ -103,15 +103,19 @@ extension QueryExecutionCoordinator { var columnComments: [String: String] = [:] var columnIdentity: [String: IdentityKind] = [:] var generatedColumns: Set = [] + var hasAuthoritativeSchema = false var foreignKeysFetched = false } - /// A rerun answered from cache carries no metadata of its own, so it inherits what the tab - /// already holds. That includes the non-writable set, which `configureForTable` clears on every - /// execution and only a schema fetch refills. + /// A rerun answered from cache carries no metadata of its own, so it takes the snapshot the + /// caller captured when it made that cache decision. That includes the non-writable set, which + /// `configureForTable` clears on every execution and only a schema fetch refills. + /// + /// The snapshot is never re-read from the session here. Reading it at this point reads whichever + /// result is active by then, and selecting a pinned result while the rerun was in flight made + /// the rerun adopt that other result's identity and non-writable sets. private func resolveDisplayMetadata( metadata: ParsedSchemaMetadata?, - existingTabId: UUID, columns: [String], columnTypes: [ColumnType], tabIndex: Int, @@ -131,22 +135,11 @@ extension QueryExecutionCoordinator { resolved.columnComments = metadata.columnComments resolved.columnIdentity = metadata.columnIdentity resolved.generatedColumns = metadata.generatedColumns + resolved.hasAuthoritativeSchema = metadata.isAuthoritative resolved.foreignKeysFetched = metadata.columnForeignKeys != nil for (col, vals) in metadata.columnEnumValues { resolved.columnEnumValues[col] = vals } - } else { - let existing = parent.tabSessionRegistry.tableRows(for: existingTabId) - resolved.columnDefaults = existing.columnDefaults - resolved.columnForeignKeys = existing.columnForeignKeys - resolved.columnNullable = existing.columnNullable - resolved.columnComments = existing.columnComments - resolved.columnIdentity = existing.columnIdentity - resolved.generatedColumns = existing.generatedColumns - resolved.foreignKeysFetched = existing.foreignKeysFetched - for (col, vals) in existing.columnEnumValues where resolved.columnEnumValues[col] == nil { - resolved.columnEnumValues[col] = vals - } } if resolved.columnForeignKeys.isEmpty, !resolved.foreignKeysFetched, let tableName { @@ -200,7 +193,6 @@ extension QueryExecutionCoordinator { let existingTabId = parent.tabManager.tabs[idx].id let resolved = resolveDisplayMetadata( metadata: metadata, - existingTabId: existingTabId, columns: columns, columnTypes: columnTypes, tabIndex: idx, @@ -219,6 +211,7 @@ extension QueryExecutionCoordinator { columnComments: resolved.columnComments, columnIdentity: resolved.columnIdentity, generatedColumns: generatedColumns, + hasAuthoritativeSchema: resolved.hasAuthoritativeSchema, foreignKeysFetched: resolved.foreignKeysFetched ) let previousTableName = parent.tabManager.tabs[idx].tableContext.tableName @@ -494,11 +487,41 @@ extension QueryExecutionCoordinator { tableName: String, resultSetId: UUID? ) { + let parsed = QueryExecutor.parseSchemaMetadata(schema) guard resultStillActive(tabId, resultSetId) else { - helpersLogger.info("[fk] phase2 apply skipped, tab closed or table changed table=\(tableName, privacy: .public)") + /// The result this was fetched for is still there, the user is just looking at another + /// one. Dropping the metadata left it with no account of which columns the server owns, + /// and nothing re-fetches on the way back, so the result stayed that way for good. + applyPhase2MetadataToInactiveResult(parsed: parsed, tabId: tabId, resultSetId: resultSetId) + helpersLogger.info("[fk] phase2 applied to an inactive result table=\(tableName, privacy: .public)") return } - applyPhase2Metadata(parsed: QueryExecutor.parseSchemaMetadata(schema), tabId: tabId) + applyPhase2Metadata(parsed: parsed, tabId: tabId) + } + + private func applyPhase2MetadataToInactiveResult( + parsed: ParsedSchemaMetadata, + tabId: UUID, + resultSetId: UUID? + ) { + guard let resultSetId, + let tab = parent.tabManager.tabs.first(where: { $0.id == tabId }), + let resultSet = tab.display.resultSets.first(where: { $0.id == resultSetId }) + else { return } + + resultSet.tableRows.updateDisplayMetadata( + columnDefaults: parsed.columnDefaults, + columnForeignKeys: parsed.columnForeignKeys, + columnNullable: parsed.columnNullable, + columnComments: parsed.columnComments, + columnIdentity: parsed.columnIdentity, + generatedColumns: parsed.generatedColumns, + hasAuthoritativeSchema: parsed.isAuthoritative + ) + if !parsed.primaryKeyColumns.isEmpty { + resultSet.origin?.primaryKeyColumns = parsed.primaryKeyColumns + resultSet.origin?.keysResolved = true + } } private func applyEnumValues( @@ -536,7 +559,8 @@ extension QueryExecutionCoordinator { columnNullable: parsed.columnNullable, columnComments: parsed.columnComments, columnIdentity: parsed.columnIdentity, - generatedColumns: parsed.generatedColumns + generatedColumns: parsed.generatedColumns, + hasAuthoritativeSchema: parsed.isAuthoritative ) } diff --git a/TablePro/Core/Coordinators/QueryExecutionCoordinator+Parameters.swift b/TablePro/Core/Coordinators/QueryExecutionCoordinator+Parameters.swift index 5704ecaba..1efe11160 100644 --- a/TablePro/Core/Coordinators/QueryExecutionCoordinator+Parameters.swift +++ b/TablePro/Core/Coordinators/QueryExecutionCoordinator+Parameters.swift @@ -133,6 +133,11 @@ extension QueryExecutionCoordinator { } else { needsMetadataFetch = false } + /// Captured now, while the result this decision was made against is still the active one. + let cachedMetadata: ParsedSchemaMetadata? = needsMetadataFetch ? nil : ParsedSchemaMetadata.cached( + rows: parent.tabSessionRegistry.tableRows(for: tabId), + primaryKeyColumns: tab.tableContext.primaryKeyColumns + ) let boundValues = BoundParameterValues(values: parameters) let parameterizedTask = Task { [weak self, parent] in @@ -172,7 +177,7 @@ extension QueryExecutionCoordinator { await applyParameterizedResult( tabId: tabId, fetchResult: fetchResult, - inlineMetadata: inlineMeta, + inlineMetadata: inlineMeta ?? cachedMetadata, tableName: tableName, isEditable: isEditable, sql: sql, diff --git a/TablePro/Core/Coordinators/RowEditingCoordinator.swift b/TablePro/Core/Coordinators/RowEditingCoordinator.swift index bb8550b74..d4d0b706f 100644 --- a/TablePro/Core/Coordinators/RowEditingCoordinator.swift +++ b/TablePro/Core/Coordinators/RowEditingCoordinator.swift @@ -47,6 +47,9 @@ final class RowEditingCoordinator { tab.tableContext.tableName != nil else { return } let tabId = tab.id + /// A new row is pre-filled from the schema's account of which columns the server owns, so + /// staging one before that account exists writes NULL into an identity column. + guard parent.tabSessionRegistry.tableRows(for: tabId).hasAuthoritativeSchema else { return } parent.dataTabDelegate?.tableViewCoordinator?.commitActiveCellEdit() @@ -168,7 +171,8 @@ final class RowEditingCoordinator { guard !parent.safeModeLevel.blocksAllWrites, let (tab, tabIndex) = parent.tabManager.selectedTabAndIndex, tab.tableContext.isEditable, - tab.tableContext.tableName != nil else { return } + tab.tableContext.tableName != nil, + parent.tabSessionRegistry.tableRows(for: tab.id).hasAuthoritativeSchema else { return } if parent.activeGridDisplayIDs != nil { duplicateFilteredRow(displayIndex: index, tab: tab, tabIndex: tabIndex) diff --git a/TablePro/Core/Services/Infrastructure/MainSplitViewController+MenuValidation.swift b/TablePro/Core/Services/Infrastructure/MainSplitViewController+MenuValidation.swift index 15f7906d1..613d8a85b 100644 --- a/TablePro/Core/Services/Infrastructure/MainSplitViewController+MenuValidation.swift +++ b/TablePro/Core/Services/Infrastructure/MainSplitViewController+MenuValidation.swift @@ -24,6 +24,10 @@ struct MenuValidationContext: Equatable { /// Export Results exports the selected tab's rows, so an empty grid has nothing to offer. var hasResultRows = false var isCurrentTabEditable = false + /// Add Row and Duplicate Row stage `DEFAULT` for every column the server fills in, which only + /// the table's own schema names. Until it lands, the result set's own metadata reports far less, + /// and an identity column would be staged as NULL that the server refuses. + var isCurrentTabSchemaResolved = false var canRestorePreviousValues = false var isQueryExecuting = false var hasQueryText = false @@ -163,6 +167,7 @@ extension MainSplitViewController: NSMenuItemValidation { case #selector(addRow(_:)), #selector(duplicateRow(_:)): return context.isConnected && context.isCurrentTabEditable && !context.isReadOnly + && context.isCurrentTabSchemaResolved case #selector(restorePreviousValues(_:)): return context.isConnected && context.canRestorePreviousValues && !context.isReadOnly case #selector(truncateTable(_:)): @@ -257,6 +262,7 @@ extension MainSplitViewController: NSMenuItemValidation { isQueryTab: actions.isQueryTab, hasResultRows: actions.hasResultRows, isCurrentTabEditable: actions.isCurrentTabEditable, + isCurrentTabSchemaResolved: actions.isCurrentTabSchemaResolved, canRestorePreviousValues: actions.canRestorePreviousValues, isQueryExecuting: actions.isQueryExecuting, hasQueryText: actions.hasQueryText, diff --git a/TablePro/Core/Services/Query/QueryExecutor.swift b/TablePro/Core/Services/Query/QueryExecutor.swift index c2c925ba1..9582db187 100644 --- a/TablePro/Core/Services/Query/QueryExecutor.swift +++ b/TablePro/Core/Services/Query/QueryExecutor.swift @@ -35,6 +35,30 @@ struct ParsedSchemaMetadata { let approximateRowCount: Int? let columnEnumValues: [String: [String]] let columnComments: [String: String] + /// Whether this came from the table's own schema, rather than from what the result set happened + /// to carry. Only the schema knows which columns the server owns, so a command that stages a + /// value from that knowledge waits for it rather than guessing from an empty set. + let isAuthoritative: Bool + + /// The metadata a tab already holds, captured at the moment the cache decision is made. + /// + /// Reading it again when the result finally lands reads whichever result is active *then*, and + /// selecting a pinned result in between made a cached rerun adopt that other result's identity + /// and non-writable sets, with no schema fetch behind it to repair the mistake. + static func cached(rows: TableRows, primaryKeyColumns: [String]) -> ParsedSchemaMetadata { + ParsedSchemaMetadata( + columnDefaults: rows.columnDefaults, + columnForeignKeys: rows.foreignKeysFetched ? rows.columnForeignKeys : nil, + columnNullable: rows.columnNullable, + primaryKeyColumns: primaryKeyColumns, + generatedColumns: rows.generatedColumns, + columnIdentity: rows.columnIdentity, + approximateRowCount: nil, + columnEnumValues: rows.columnEnumValues, + columnComments: rows.columnComments, + isAuthoritative: rows.hasAuthoritativeSchema + ) + } } @MainActor @@ -227,7 +251,8 @@ final class QueryExecutor { columnIdentity: identity, approximateRowCount: schema.approximateRowCount, columnEnumValues: enumValues, - columnComments: comments + columnComments: comments, + isAuthoritative: true ) } @@ -235,11 +260,17 @@ final class QueryExecutor { guard let meta, !meta.isEmpty, meta.count == columns.count else { return nil } var nullable: [String: Bool] = [:] var primaryKeys: [String] = [] + var identity: [String: IdentityKind] = [:] for (index, column) in columns.enumerated() { nullable[column] = meta[index].isNullable if meta[index].isPrimaryKey { primaryKeys.append(column) } + /// The result set reports only that the server allocates the column, never whether it + /// would refuse an explicit value, so the writable kind is the safe reading. + if meta[index].isAutoIncrement { + identity[column] = .byDefault + } } return ParsedSchemaMetadata( columnDefaults: [:], @@ -247,10 +278,11 @@ final class QueryExecutor { columnNullable: nullable, primaryKeyColumns: primaryKeys, generatedColumns: [], - columnIdentity: [:], + columnIdentity: identity, approximateRowCount: nil, columnEnumValues: [:], - columnComments: [:] + columnComments: [:], + isAuthoritative: false ) } diff --git a/TablePro/Core/Services/Query/RowOperationsManager.swift b/TablePro/Core/Services/Query/RowOperationsManager.swift index 6815ab079..fbfb4f6c4 100644 --- a/TablePro/Core/Services/Query/RowOperationsManager.swift +++ b/TablePro/Core/Services/Query/RowOperationsManager.swift @@ -425,8 +425,18 @@ final class RowOperationsManager { var pastedRowInfo: [PastedRowInfo] = [] var insertedIndices = IndexSet() + /// A pasted row arrives whole, so it carries values for columns the server owns too. They + /// never reach the cell-edit boundary that refuses them, and the statement generator drops + /// them silently, so the grid showed a pasted identity value the row was never saved with. + let serverOwned = tableRows.columns.enumerated().filter { _, name in + tableRows.generatedColumns.contains(name) || tableRows.columnIdentity[name] != nil + }.map(\.offset) + for parsedRow in parsedRows { - let rowValues = parsedRow.values + var rowValues = parsedRow.values + for index in serverOwned where index < rowValues.count { + rowValues[index] = .text("__DEFAULT__") + } let newRowIndex = tableRows.count _ = tableRows.appendInsertedRow(values: rowValues) insertedIndices.insert(newRowIndex) diff --git a/TablePro/Models/Query/TableRows.swift b/TablePro/Models/Query/TableRows.swift index 7d677e09b..f7d6d1f5d 100644 --- a/TablePro/Models/Query/TableRows.swift +++ b/TablePro/Models/Query/TableRows.swift @@ -22,6 +22,10 @@ struct TableRows: Sendable { /// fetch refills it, so a rerun that answered from cache left a generated or `GENERATED ALWAYS /// AS IDENTITY` column writable again. var generatedColumns: Set + /// Whether the sets above came from the table's own schema. A result set reports far less than + /// the schema does, so a command that stages a value from them waits rather than reading an + /// empty set as "this table owns nothing". + var hasAuthoritativeSchema: Bool var foreignKeysFetched: Bool init( @@ -35,6 +39,7 @@ struct TableRows: Sendable { columnComments: [String: String] = [:], columnIdentity: [String: IdentityKind] = [:], generatedColumns: Set = [], + hasAuthoritativeSchema: Bool = false, foreignKeysFetched: Bool = false ) { self.rows = rows @@ -48,6 +53,7 @@ struct TableRows: Sendable { self.columnComments = columnComments self.columnIdentity = columnIdentity self.generatedColumns = generatedColumns + self.hasAuthoritativeSchema = hasAuthoritativeSchema self.foreignKeysFetched = foreignKeysFetched } @@ -191,7 +197,8 @@ struct TableRows: Sendable { columnNullable: [String: Bool]? = nil, columnComments: [String: String]? = nil, columnIdentity: [String: IdentityKind]? = nil, - generatedColumns: Set? = nil + generatedColumns: Set? = nil, + hasAuthoritativeSchema: Bool? = nil ) -> Delta { var didChange = false if let columnTypes, columnTypes != self.columnTypes { @@ -229,6 +236,10 @@ struct TableRows: Sendable { self.generatedColumns = generatedColumns didChange = true } + if let hasAuthoritativeSchema, hasAuthoritativeSchema != self.hasAuthoritativeSchema { + self.hasAuthoritativeSchema = hasAuthoritativeSchema + didChange = true + } return didChange ? .columnsReplaced : .none } @@ -243,6 +254,7 @@ struct TableRows: Sendable { columnComments: [String: String] = [:], columnIdentity: [String: IdentityKind] = [:], generatedColumns: Set = [], + hasAuthoritativeSchema: Bool = false, foreignKeysFetched: Bool = false ) -> TableRows { var rows = ContiguousArray() @@ -262,6 +274,7 @@ struct TableRows: Sendable { columnComments: columnComments, columnIdentity: columnIdentity, generatedColumns: generatedColumns, + hasAuthoritativeSchema: hasAuthoritativeSchema, foreignKeysFetched: foreignKeysFetched ) } diff --git a/TablePro/Models/UI/MultiRowEditState.swift b/TablePro/Models/UI/MultiRowEditState.swift index 5f7bee7ef..5d9061d81 100644 --- a/TablePro/Models/UI/MultiRowEditState.swift +++ b/TablePro/Models/UI/MultiRowEditState.swift @@ -28,6 +28,11 @@ struct FieldEditState: Identifiable { /// A schema field has no data type, so it offers no type badge and no NULL or DEFAULT state. var isSchemaField: Bool = false + /// The server owns the value, so the field is shown without an editor. Refusing the edit further + /// down instead would leave a pending value here that nothing can clear, and the inspector would + /// go on reporting an unsaved change that Save never writes. + var isServerOwned: Bool = false + /// The value already differs from the loaded schema because the edit is recorded elsewhere. var hasCommittedEdit: Bool = false @@ -80,7 +85,8 @@ final class MultiRowEditState { columnTypes: [ColumnType], externallyModifiedColumns: Set = [], primaryKeyColumns: Set = [], - foreignKeyColumns: Set = [] + foreignKeyColumns: Set = [], + serverOwnedColumns: Set = [] ) { // Check if the underlying data has changed (not just edits) let columnsChanged = self.columns != columns @@ -145,6 +151,7 @@ final class MultiRowEditState { isJson: isJson, isPrimaryKey: primaryKeyColumns.contains(columnName), isForeignKey: foreignKeyColumns.contains(columnName), + isServerOwned: serverOwnedColumns.contains(columnName), originalValue: originalValue, hasMultipleValues: hasMultipleValues, pendingValue: pendingValue, diff --git a/TablePro/Views/Main/Extensions/MainContentCoordinator+RowOperations.swift b/TablePro/Views/Main/Extensions/MainContentCoordinator+RowOperations.swift index 18dc87e95..7dcbc5619 100644 --- a/TablePro/Views/Main/Extensions/MainContentCoordinator+RowOperations.swift +++ b/TablePro/Views/Main/Extensions/MainContentCoordinator+RowOperations.swift @@ -18,6 +18,9 @@ extension MainContentCoordinator { /// `GridSelectionOwner`, which answers `.none` in Chart mode and `.schemaGrid` in Structure /// mode, so without this the command is either inert or adds a column under a row's name. guard tab.display.resultsViewMode == .data else { return false } + /// A new row is pre-filled from the schema's account of which columns the server fills in, + /// so the command waits for that account rather than staging NULL into an identity column. + guard tabSessionRegistry.tableRows(for: tab.id).hasAuthoritativeSchema else { return false } return tab.tableContext.isEditable && !tab.tableContext.isView && !safeModeLevel.blocksAllWrites diff --git a/TablePro/Views/Main/Extensions/MainContentView+EventHandlers.swift b/TablePro/Views/Main/Extensions/MainContentView+EventHandlers.swift index f750ea102..5dd744667 100644 --- a/TablePro/Views/Main/Extensions/MainContentView+EventHandlers.swift +++ b/TablePro/Views/Main/Extensions/MainContentView+EventHandlers.swift @@ -215,7 +215,8 @@ extension MainContentView { columnTypes: columnTypes, externallyModifiedColumns: modifiedColumns, primaryKeyColumns: pkColumns, - foreignKeyColumns: fkColumns + foreignKeyColumns: fkColumns, + serverOwnedColumns: tableRows.generatedColumns ) guard isSidebarEditable else { diff --git a/TablePro/Views/Main/MainContentCommandActions.swift b/TablePro/Views/Main/MainContentCommandActions.swift index d44c60ca9..79af61e76 100644 --- a/TablePro/Views/Main/MainContentCommandActions.swift +++ b/TablePro/Views/Main/MainContentCommandActions.swift @@ -403,6 +403,11 @@ final class MainContentCommandActions { return coordinator.canEditActiveResult } + var isCurrentTabSchemaResolved: Bool { + guard let coordinator, let tabId = coordinator.tabManager.selectedTabId else { return false } + return coordinator.tabSessionRegistry.tableRows(for: tabId).hasAuthoritativeSchema + } + var canRestorePreviousValues: Bool { coordinator?.canRewindSelectedTab ?? false } diff --git a/TablePro/Views/Main/MainContentCoordinator.swift b/TablePro/Views/Main/MainContentCoordinator.swift index 40ab97d6d..523c7d827 100644 --- a/TablePro/Views/Main/MainContentCoordinator.swift +++ b/TablePro/Views/Main/MainContentCoordinator.swift @@ -1277,6 +1277,11 @@ final class MainContentCoordinator { } else { needsMetadataFetch = false } + /// Captured now, while the result this decision was made against is still the active one. + let cachedMetadata: ParsedSchemaMetadata? = needsMetadataFetch ? nil : ParsedSchemaMetadata.cached( + rows: tabSessionRegistry.tableRows(for: tabId), + primaryKeyColumns: tabManager.tabs[index].tableContext.primaryKeyColumns + ) if let tableName { Self.logger.info( "[fk] metadata decision table=\(tableName, privacy: .public) isEditable=\(isEditable) needsFetch=\(needsMetadataFetch)" @@ -1378,7 +1383,7 @@ final class MainContentCoordinator { statusMessage: fetchResult.statusMessage, tableName: tableName, isEditable: isEditable, - metadata: inlineMeta, + metadata: inlineMeta ?? cachedMetadata, hasSchema: false, sql: sql, connection: conn, diff --git a/TablePro/Views/Results/DataGridRowView.swift b/TablePro/Views/Results/DataGridRowView.swift index 88518f627..b6018ab1e 100644 --- a/TablePro/Views/Results/DataGridRowView.swift +++ b/TablePro/Views/Results/DataGridRowView.swift @@ -496,10 +496,14 @@ class DataGridRowView: NSTableRowView { } } - let duplicateItem = NSMenuItem( - title: String(localized: "Duplicate"), action: #selector(duplicateRow), keyEquivalent: "") - duplicateItem.target = self - menu.addItem(duplicateItem) + /// The copy resets the columns the server owns, which only the schema names, so the item + /// stays away until it has arrived rather than appearing and doing nothing. + if tableRows.hasAuthoritativeSchema { + let duplicateItem = NSMenuItem( + title: String(localized: "Duplicate"), action: #selector(duplicateRow), keyEquivalent: "") + duplicateItem.target = self + menu.addItem(duplicateItem) + } let deleteItem = NSMenuItem( title: String(localized: "Delete"), diff --git a/TablePro/Views/Results/Extensions/DataGridView+CellCommit.swift b/TablePro/Views/Results/Extensions/DataGridView+CellCommit.swift index 983b270f3..ddb83f651 100644 --- a/TablePro/Views/Results/Extensions/DataGridView+CellCommit.swift +++ b/TablePro/Views/Results/Extensions/DataGridView+CellCommit.swift @@ -30,6 +30,10 @@ extension TableViewCoordinator { guard !isCommittingCellEdit else { return nil } let tableRows = tableRowsProvider() guard columnIndex >= 0 && columnIndex < tableRows.columns.count else { return nil } + /// Before the rows are touched, not after. The change manager refuses a server-owned column + /// on its own, and editing here first would paint a value into the grid that no statement + /// will ever carry. + guard isColumnWritable(tableRows.columns[columnIndex]) else { return nil } guard let displayRowValues = displayRow(at: row) else { return nil } guard columnIndex < displayRowValues.values.count else { return nil } let oldValue = displayRowValues.values[columnIndex] diff --git a/TablePro/Views/Results/Extensions/DataGridView+Sort.swift b/TablePro/Views/Results/Extensions/DataGridView+Sort.swift index 9241c9827..8444aabe7 100644 --- a/TablePro/Views/Results/Extensions/DataGridView+Sort.swift +++ b/TablePro/Views/Results/Extensions/DataGridView+Sort.swift @@ -121,6 +121,7 @@ extension TableViewCoordinator { if let dataColumnIndex = dataColumnIndex(from: column.identifier), isEditable, cachedRowCount > 0, + isColumnWritable(baseName), !primaryKeyColumns.contains(baseName) { let fillItem = NSMenuItem( title: String(localized: "Fill Column…"), diff --git a/TablePro/Views/RightSidebar/RightSidebarView.swift b/TablePro/Views/RightSidebar/RightSidebarView.swift index 95b9c0909..ccaab708f 100644 --- a/TablePro/Views/RightSidebar/RightSidebarView.swift +++ b/TablePro/Views/RightSidebar/RightSidebarView.swift @@ -329,7 +329,8 @@ struct RightSidebarView: View { } @ViewBuilder - private func fieldDetailRow(_ field: FieldEditState, at index: Int, isEditable: Bool) -> some View { + private func fieldDetailRow(_ field: FieldEditState, at index: Int, isEditable rowIsEditable: Bool) -> some View { + let isEditable = rowIsEditable && !field.isServerOwned let kind = FieldEditorResolver.resolve(field: field) let isJsonField = kind == .json let isPhpField = kind == .phpSerialized diff --git a/TableProTests/Core/ChangeTracking/DataChangeManagerTests.swift b/TableProTests/Core/ChangeTracking/DataChangeManagerTests.swift index 390ad1fb7..c0c5310d2 100644 --- a/TableProTests/Core/ChangeTracking/DataChangeManagerTests.swift +++ b/TableProTests/Core/ChangeTracking/DataChangeManagerTests.swift @@ -591,3 +591,100 @@ struct DataChangeManagerTests { #expect(manager.reloadVersion == versionBeforeClear + 1) } } + +/// Paste, Fill Column and the row inspector all reach `recordCellChange` directly, without passing +/// the grid's own writability check. A server-owned column could be staged there, silently filtered +/// out during statement generation, and then cleared by a save that reported success. +@MainActor +@Suite("Data Change Manager - non-writable columns") +struct DataChangeManagerNonWritableTests { + private func makeManager(generatedColumns: Set) -> DataChangeManager { + let manager = DataChangeManager() + manager.configureForTable( + tableName: "users", + columns: ["id", "name"], + primaryKeyColumns: ["id"], + databaseType: .postgresql, + generatedColumns: generatedColumns + ) + return manager + } + + @Test("An edit to a server-owned column is refused") + func refusesServerOwnedColumn() { + let manager = makeManager(generatedColumns: ["id"]) + + manager.recordCellChange( + rowIndex: 0, columnIndex: 0, columnName: "id", + oldValue: .text("1"), newValue: .text("99") + ) + + #expect(!manager.hasChanges) + #expect(manager.rowChanges.isEmpty) + } + + @Test("An edit to a writable column is still recorded") + func recordsWritableColumn() { + let manager = makeManager(generatedColumns: ["id"]) + + manager.recordCellChange( + rowIndex: 0, columnIndex: 1, columnName: "name", + oldValue: .text("Alice"), newValue: .text("Bob") + ) + + #expect(manager.hasChanges) + } + + /// The refusal must not leave the other edits of the same save behind. + @Test("A refused edit does not disturb a legitimate one recorded alongside it") + func refusalLeavesOtherEditsIntact() { + let manager = makeManager(generatedColumns: ["id"]) + + manager.recordCellChange( + rowIndex: 0, columnIndex: 1, columnName: "name", + oldValue: .text("Alice"), newValue: .text("Bob") + ) + manager.recordCellChange( + rowIndex: 0, columnIndex: 0, columnName: "id", + oldValue: .text("1"), newValue: .text("99") + ) + + #expect(manager.hasChanges) + let edited = manager.rowChanges.flatMap(\.cellChanges).map(\.columnName) + #expect(edited == ["name"]) + } +} + +/// `immutableColumns` is the driver's own list, such as MongoDB's `_id`. The grid consults it and +/// the model boundary did not, so the row inspector could still stage a change the backend rejects. +@MainActor +@Suite("Data Change Manager - immutable columns") +struct DataChangeManagerImmutableColumnTests { + @Test("A writable column with no generated set is accepted") + func writableColumnAccepted() { + let manager = DataChangeManager() + manager.configureForTable( + tableName: "orders", + columns: ["id", "total"], + primaryKeyColumns: ["id"], + databaseType: .postgresql, + generatedColumns: [] + ) + + #expect(manager.isColumnWritable("total")) + } + + @Test("A generated column is not writable") + func generatedColumnNotWritable() { + let manager = DataChangeManager() + manager.configureForTable( + tableName: "orders", + columns: ["id", "total"], + primaryKeyColumns: ["id"], + databaseType: .postgresql, + generatedColumns: ["total"] + ) + + #expect(!manager.isColumnWritable("total")) + } +} diff --git a/TableProTests/Core/Services/RowOperationsManagerPasteTests.swift b/TableProTests/Core/Services/RowOperationsManagerPasteTests.swift index 00f0f1c1d..8e4ae9352 100644 --- a/TableProTests/Core/Services/RowOperationsManagerPasteTests.swift +++ b/TableProTests/Core/Services/RowOperationsManagerPasteTests.swift @@ -190,3 +190,78 @@ struct RowOperationsManagerPasteTests { #expect(RowOperationsManager.detectParser(for: "single") is TSVRowParser) } } + +/// A pasted row arrives whole, so it carries values for columns the server owns. Those never reach +/// the cell-edit boundary that refuses them, and `SQLStatementGenerator` drops them without a word, +/// so the grid showed a pasted identity value the row was never saved with. +@MainActor +@Suite("RowOperationsManager Paste - server-owned columns") +struct RowOperationsManagerPasteServerOwnedTests { + private static let columns = ["id", "code", "name"] + + private func makeManager() -> RowOperationsManager { + let changeManager = DataChangeManager() + changeManager.configureForTable( + tableName: "t", + columns: Self.columns, + primaryKeyColumns: ["id"], + databaseType: .postgresql, + generatedColumns: [] + ) + return RowOperationsManager(changeManager: changeManager) + } + + private func paste( + _ payload: GridRowsClipboardPayload, + columnIdentity: [String: IdentityKind] = [:], + generatedColumns: Set = [] + ) -> [PluginCellValue] { + let clipboard = PasteMockClipboard() + clipboard.gridRowsToRead = payload + var rows = TableRows.from( + queryRows: [], + columns: Self.columns, + columnTypes: Array(repeating: .text(rawType: nil), count: Self.columns.count), + columnIdentity: columnIdentity, + generatedColumns: generatedColumns + ) + let result = makeManager().pasteRowsFromClipboard( + columns: Self.columns, + primaryKeyColumns: ["id"], + tableRows: &rows, + clipboard: clipboard + ) + return result.pastedRows.first?.values ?? [] + } + + private var payload: GridRowsClipboardPayload { + GridRowsClipboardPayload( + columns: Self.columns, + rows: [[.text("7"), .text("42"), .text("Ada")]] + ) + } + + @Test("A pasted identity column that is not the primary key is reset to DEFAULT") + func resetsNonKeyIdentity() { + let values = paste(payload, columnIdentity: ["code": .always]) + + #expect(values[1] == .text("__DEFAULT__")) + #expect(values[2] == .text("Ada")) + } + + @Test("A pasted generated column is reset to DEFAULT") + func resetsGeneratedColumn() { + let values = paste(payload, generatedColumns: ["code"]) + + #expect(values[1] == .text("__DEFAULT__")) + #expect(values[2] == .text("Ada")) + } + + @Test("An ordinary column keeps the pasted value") + func keepsOrdinaryColumn() { + let values = paste(payload) + + #expect(values[1] == .text("42")) + #expect(values[2] == .text("Ada")) + } +} diff --git a/TableProTests/Core/Services/SchemaMetadataGeneratedColumnTests.swift b/TableProTests/Core/Services/SchemaMetadataGeneratedColumnTests.swift index 44cb57987..2a63e6c3c 100644 --- a/TableProTests/Core/Services/SchemaMetadataGeneratedColumnTests.swift +++ b/TableProTests/Core/Services/SchemaMetadataGeneratedColumnTests.swift @@ -93,3 +93,106 @@ struct SchemaMetadataGeneratedColumnTests { #expect(QueryExecutor.parseSchemaMetadata(schema).generatedColumns == ["id"]) } } + +/// The metadata a rerun inherits is captured when the cache decision is made, not read back when the +/// result lands. Reading it late read whichever result was active by then, so selecting a pinned +/// result mid-flight made the rerun adopt that other result's identity and non-writable sets. +@MainActor @Suite("Cached schema metadata snapshot") +struct CachedSchemaMetadataTests { + private func rows( + columnIdentity: [String: IdentityKind] = [:], + generatedColumns: Set = [], + hasAuthoritativeSchema: Bool = true + ) -> TableRows { + TableRows.from( + queryRows: [], + columns: ["id", "name"], + columnTypes: [], + columnDefaults: ["name": "'anon'"], + columnEnumValues: ["name": ["a"]], + columnNullable: ["name": true], + columnComments: ["name": "the name"], + columnIdentity: columnIdentity, + generatedColumns: generatedColumns, + hasAuthoritativeSchema: hasAuthoritativeSchema, + foreignKeysFetched: true + ) + } + + @Test("The snapshot carries every write-relevant field") + func snapshotCarriesWriteFields() { + let cached = ParsedSchemaMetadata.cached( + rows: rows(columnIdentity: ["id": .always], generatedColumns: ["id"]), + primaryKeyColumns: ["id"] + ) + + #expect(cached.columnIdentity == ["id": .always]) + #expect(cached.generatedColumns == ["id"]) + #expect(cached.primaryKeyColumns == ["id"]) + #expect(cached.isAuthoritative) + #expect(cached.columnForeignKeys != nil) + } + + /// `foreignKeysFetched` is what tells the tab its arrows have arrived. Claiming they had when + /// they had not would let the metadata fetch be skipped for good. + @Test("Unfetched foreign keys stay absent rather than becoming an empty answer") + func unfetchedForeignKeysStayNil() { + var unfetched = rows() + unfetched.foreignKeysFetched = false + + #expect(ParsedSchemaMetadata.cached(rows: unfetched, primaryKeyColumns: []).columnForeignKeys == nil) + } + + @Test("A snapshot of rows built from result metadata is not authoritative") + func inheritsNonAuthoritative() { + let cached = ParsedSchemaMetadata.cached( + rows: rows(hasAuthoritativeSchema: false), + primaryKeyColumns: [] + ) + + #expect(!cached.isAuthoritative) + } +} + +/// Only the table's own schema names the columns the server owns. A result set reports far less, so +/// treating its silence as "this table owns nothing" staged NULL into an identity column. +@MainActor @Suite("Schema metadata authoritativeness") +struct SchemaMetadataAuthoritativenessTests { + @Test("A parsed table schema is authoritative") + func parsedSchemaIsAuthoritative() { + let schema = FetchedTableSchema( + columns: [ColumnInfo(name: "id", dataType: "INT", isNullable: false, isPrimaryKey: true)], + foreignKeys: nil, + approximateRowCount: nil + ) + + #expect(QueryExecutor.parseSchemaMetadata(schema).isAuthoritative) + } + + @Test("Result-set metadata is not authoritative") + func inlineMetadataIsNotAuthoritative() { + let parsed = QueryExecutor.inlineMetadata( + from: [ResultColumnMeta(isPrimaryKey: true, isNullable: false, isAutoIncrement: true)], + columns: ["id"] + ) + + #expect(parsed?.isAuthoritative == false) + } + + /// The result set does report that the server allocates the column, and that much is worth + /// carrying: it is the difference between pre-filling DEFAULT and pre-filling NULL. It never + /// says whether an explicit value would be refused, so the writable kind is the safe reading. + @Test("An auto-increment result column is carried as a writable identity") + func inlineMetadataCarriesAutoIncrement() { + let parsed = QueryExecutor.inlineMetadata( + from: [ + ResultColumnMeta(isPrimaryKey: true, isNullable: false, isAutoIncrement: true), + ResultColumnMeta(isPrimaryKey: false, isNullable: true, isAutoIncrement: false), + ], + columns: ["id", "name"] + ) + + #expect(parsed?.columnIdentity == ["id": .byDefault]) + #expect(parsed?.generatedColumns.isEmpty == true) + } +} diff --git a/docs/features/change-tracking.mdx b/docs/features/change-tracking.mdx index cec444e22..939aee792 100644 --- a/docs/features/change-tracking.mdx +++ b/docs/features/change-tracking.mdx @@ -41,10 +41,12 @@ Right-click an editable cell and open **Set Value** for the common ones without - **Duplicate Row** (`Cmd+Shift+D`) on the right-click menu copies a row and resets its primary key and its identity columns to `DEFAULT`, so the database assigns new ones. - **Delete**: select rows by their row numbers (`Shift`-click for a range, `Cmd`-click for separate rows) and press `Delete`. They stay visible with a strikethrough, and saving asks "Delete 5 rows?" before they go. - **Paste** on the right-click menu inserts copied rows as new rows. -- **Fill Column** on the header right-click menu writes one value into every loaded row, skipping primary key columns. +- **Fill Column** on the header right-click menu writes one value into every loaded row. It is absent on primary key columns and on columns the server owns. A new row starts at `DEFAULT` in every column the server fills in. That covers a column with a default expression, and an identity or `AUTO_INCREMENT` column, which carries no default of its own. The INSERT leaves those columns out, so the server assigns them. +Add Row and Duplicate Row stay dimmed until the table's schema has loaded, which is what says which columns the server fills in. On a table opened for the first time that is a fraction of a second. + ## When the grid will not edit A tab opened from the sidebar edits its table directly. A query tab edits only when the app can prove the rows came from exactly one table: