Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 9 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down
15 changes: 15 additions & 0 deletions Plugins/SQLExportPlugin/SQLExportPlugin.swift
Original file line number Diff line number Diff line change
Expand Up @@ -488,6 +488,11 @@
&& 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 {
Expand All @@ -496,11 +501,14 @@
switch element {
case .header(let header):
columns = header.columns
columnTypeNames = header.columnTypeNames ?? []

Check warning on line 504 in Plugins/SQLExportPlugin/SQLExportPlugin.swift

View workflow job for this annotation

GitHub Actions / Build for testing

left side of nil coalescing operator '??' has non-optional type '[String]', so the right side is never used
case .rows(let rows):
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,
Expand All @@ -521,6 +529,9 @@
}

if !rowBatch.isEmpty {
if needsIdentityInsert, !wroteAnyRows {
try fileHandle.write(contentsOf: "SET IDENTITY_INSERT \(tableRef) ON;\n".toUTF8Data())
}
try writeInsertStatements(
tableRef: tableRef,
columns: columns,
Expand All @@ -536,6 +547,10 @@
wroteAnyRows = true
}

if wroteAnyRows, needsIdentityInsert {
try fileHandle.write(contentsOf: "SET IDENTITY_INSERT \(tableRef) OFF;\n".toUTF8Data())
}

if wroteAnyRows {
try fileHandle.write(contentsOf: "\n".toUTF8Data())
}
Expand Down
21 changes: 21 additions & 0 deletions TablePro/Core/ChangeTracking/DataChangeManager.swift
Original file line number Diff line number Diff line change
Expand Up @@ -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(
Expand All @@ -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,
Expand Down
64 changes: 44 additions & 20 deletions TablePro/Core/Coordinators/QueryExecutionCoordinator+Helpers.swift
Original file line number Diff line number Diff line change
Expand Up @@ -103,15 +103,19 @@ extension QueryExecutionCoordinator {
var columnComments: [String: String] = [:]
var columnIdentity: [String: IdentityKind] = [:]
var generatedColumns: Set<String> = []
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,
Expand All @@ -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 {
Expand Down Expand Up @@ -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,
Expand All @@ -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
Expand Down Expand Up @@ -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(
Expand Down Expand Up @@ -536,7 +559,8 @@ extension QueryExecutionCoordinator {
columnNullable: parsed.columnNullable,
columnComments: parsed.columnComments,
columnIdentity: parsed.columnIdentity,
generatedColumns: parsed.generatedColumns
generatedColumns: parsed.generatedColumns,
hasAuthoritativeSchema: parsed.isAuthoritative
)
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -172,7 +177,7 @@ extension QueryExecutionCoordinator {
await applyParameterizedResult(
tabId: tabId,
fetchResult: fetchResult,
inlineMetadata: inlineMeta,
inlineMetadata: inlineMeta ?? cachedMetadata,
tableName: tableName,
isEditable: isEditable,
sql: sql,
Expand Down
6 changes: 5 additions & 1 deletion TablePro/Core/Coordinators/RowEditingCoordinator.swift
Original file line number Diff line number Diff line change
Expand Up @@ -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()

Expand Down Expand Up @@ -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)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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(_:)):
Expand Down Expand Up @@ -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,
Expand Down
38 changes: 35 additions & 3 deletions TablePro/Core/Services/Query/QueryExecutor.swift
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -227,30 +251,38 @@ final class QueryExecutor {
columnIdentity: identity,
approximateRowCount: schema.approximateRowCount,
columnEnumValues: enumValues,
columnComments: comments
columnComments: comments,
isAuthoritative: true
)
}

static func inlineMetadata(from meta: [ResultColumnMeta]?, columns: [String]) -> ParsedSchemaMetadata? {
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: [:],
columnForeignKeys: nil,
columnNullable: nullable,
primaryKeyColumns: primaryKeys,
generatedColumns: [],
columnIdentity: [:],
columnIdentity: identity,
approximateRowCount: nil,
columnEnumValues: [:],
columnComments: [:]
columnComments: [:],
isAuthoritative: false
)
}

Expand Down
12 changes: 11 additions & 1 deletion TablePro/Core/Services/Query/RowOperationsManager.swift
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
Loading
Loading