From ab22f9b5079f9931cc763f686aef3943bc11670a Mon Sep 17 00:00:00 2001 From: Ngo Quoc Dat Date: Tue, 29 Sep 2026 19:44:30 +0700 Subject: [PATCH 1/3] fix(plugins): refuse a table transfer that maps two source columns to one destination column --- CHANGELOG.md | 1 + .../Core/Plugins/ImportDataSinkAdapter.swift | 57 ++++-- .../Services/Export/TableColumnMatcher.swift | 75 ++++++-- .../Export/TableTransferService.swift | 23 +++ TablePro/Resources/Localizable.xcstrings | 12 ++ .../Export/TableTransferMappingEditor.swift | 107 ++++++++--- .../Views/Export/TableTransferSheet.swift | 38 ++-- .../Core/Export/TableColumnMatcherTests.swift | 119 ++++++++++++ .../Export/TableTransferServiceTests.swift | 175 ++++++++++++++++++ .../ImportDataSinkAdapterMappingTests.swift | 40 ++++ docs/features/import-export.mdx | 8 +- 11 files changed, 587 insertions(+), 68 deletions(-) create mode 100644 TableProTests/Core/Export/TableColumnMatcherTests.swift diff --git a/CHANGELOG.md b/CHANGELOG.md index 40eb3555b3..f63c5d3753 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -19,6 +19,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 - iOS row editor saving the placeholder of a long text or binary value over the full value. - Explain Analyze running write statements on Read-Only connections and skipping the Alert and Safe Mode confirmation. - **Local only** connections taking edits and deletions made on another device. +- Table Transfer emptying a destination table, then failing, when two source columns map to one column. ## [0.76.1] - 2026-09-29 diff --git a/TablePro/Core/Plugins/ImportDataSinkAdapter.swift b/TablePro/Core/Plugins/ImportDataSinkAdapter.swift index 1626b7b891..33c42a6697 100644 --- a/TablePro/Core/Plugins/ImportDataSinkAdapter.swift +++ b/TablePro/Core/Plugins/ImportDataSinkAdapter.swift @@ -16,6 +16,11 @@ final class ImportDataSinkAdapter: PluginImportDataSink, @unchecked Sendable { private let databaseType: DatabaseType private let grammar: SQLLexicalGrammar private let columnMapping: [String: String] + + /// A lowercased field name to the one mapping key that lowercases to it. A name two keys share, + /// `Name` and `name`, is left out, because a third spelling could mean either. + private let mappingKeyByFoldedName: [String: String] + private let rowGenerator: SQLStatementGenerator? /// Asked before every statement this sink sends, because one `insertRows` call is no longer one @@ -46,10 +51,9 @@ final class ImportDataSinkAdapter: PluginImportDataSink, @unchecked Sendable { self.grammar = databaseType.lexicalGrammar self.databaseTypeId = databaseType.rawValue self.targetTable = targetTable - self.columnMapping = Dictionary( - columnMapping.map { ($0.key.lowercased(), $0.value) }, - uniquingKeysWith: { _, last in last } - ) + self.columnMapping = columnMapping + self.mappingKeyByFoldedName = Dictionary(grouping: columnMapping.keys, by: { $0.lowercased() }) + .compactMapValues { $0.count == 1 ? $0.first : nil } if let targetTable { self.rowGenerator = try? SQLStatementGenerator( tableName: targetTable, @@ -98,14 +102,7 @@ final class ImportDataSinkAdapter: PluginImportDataSink, @unchecked Sendable { throw PluginImportError.importFailed("Could not resolve SQL dialect for \(targetTable)") } - var columns: [String] = [] - var bindValues: [PluginCellValue] = [] - for (field, value) in values { - guard let column = columnMapping[field.lowercased()] else { continue } - columns.append(column) - bindValues.append(value) - } - + let (columns, bindValues) = mappedColumnsAndValues(values) guard !columns.isEmpty else { guard values.isEmpty else { throw PluginImportError.importFailed(Self.unmappedRowMessage) @@ -201,16 +198,48 @@ final class ImportDataSinkAdapter: PluginImportDataSink, @unchecked Sendable { String(localized: "No values in this row matched the column mapping") } - private func mappedColumnsAndValues(_ values: [String: PluginCellValue]) -> ([String], [PluginCellValue]) { + /// Exact names first. Folding every name used to put `Name` and `name` on one key, so a + /// source table holding both wrote one column twice and the server refused the INSERT. + internal func mappedColumnsAndValues(_ values: [String: PluginCellValue]) -> ([String], [PluginCellValue]) { var pairs: [(column: String, value: PluginCellValue)] = [] + var unspelled: [(field: String, value: PluginCellValue)] = [] for (field, value) in values { - guard let column = columnMapping[field.lowercased()] else { continue } + guard let column = columnMapping[field] else { + unspelled.append((field, value)) + continue + } pairs.append((column, value)) } + if !unspelled.isEmpty { + pairs += caseFoldedPairs(unspelled, in: values) + } pairs.sort { $0.column < $1.column } return (pairs.map(\.column), pairs.map(\.value)) } + /// A field spelled like no mapping key still reaches the key it matches ignoring case, such as + /// a header cased differently from the one the mapping was made from, but only when nothing + /// else in the row answers to that name: not the key's own spelling, and not a second field. + private func caseFoldedPairs( + _ unspelled: [(field: String, value: PluginCellValue)], + in values: [String: PluginCellValue] + ) -> [(column: String, value: PluginCellValue)] { + var fieldsPerFoldedName: [String: Int] = [:] + for entry in unspelled { + fieldsPerFoldedName[entry.field.lowercased(), default: 0] += 1 + } + var pairs: [(column: String, value: PluginCellValue)] = [] + for (field, value) in unspelled { + let folded = field.lowercased() + guard fieldsPerFoldedName[folded] == 1, + let key = mappingKeyByFoldedName[folded], + values[key] == nil, + let column = columnMapping[key] else { continue } + pairs.append((column, value)) + } + return pairs + } + func deleteAllRowsFromTargetTable() async throws { guard targetTable != nil, let rowGenerator else { throw PluginImportError.importFailed("No target table configured for row import") diff --git a/TablePro/Core/Services/Export/TableColumnMatcher.swift b/TablePro/Core/Services/Export/TableColumnMatcher.swift index 642ae9daf6..d2da0eddfb 100644 --- a/TablePro/Core/Services/Export/TableColumnMatcher.swift +++ b/TablePro/Core/Services/Export/TableColumnMatcher.swift @@ -25,27 +25,31 @@ enum TableColumnMatcher { let unmatchedDestination: [String] var isEmpty: Bool { mapping.isEmpty } - } - /// Case-insensitive, because engines disagree about identifier folding and a transfer from a - /// case-folding engine to a case-preserving one would otherwise match nothing. - static func match(source: [String], destination: [String]) -> Match { - var destinationByFolded: [String: String] = [:] - for column in destination { - destinationByFolded[column.lowercased()] = column + /// Destination columns more than one source column is mapped to. The INSERT would name + /// each of them twice, which every engine refuses, so the transfer cannot run until the + /// user moves one. + var contestedDestinations: [String] { + TableColumnMatcher.contestedDestinations(in: mapping) } + } + /// Exact spelling first, then case-insensitive, because engines disagree about identifier + /// folding and a transfer from a case-folding engine to a case-preserving one would otherwise + /// match nothing. A destination column goes to one source column at most, so `Name` and `name` + /// on the source never both land on a lone `name`. + static func match(source: [String], destination: [String]) -> Match { + let targets = pair(source, with: destination) var mapping: [String: String] = [:] var unmatchedSource: [String] = [] - var claimed: Set = [] - for column in source { - guard let target = destinationByFolded[column.lowercased()] else { + for (column, target) in zip(source, targets) { + guard let target else { unmatchedSource.append(column) continue } mapping[column] = target - claimed.insert(target) } + let claimed = Set(mapping.values) return Match( mapping: mapping, unmatchedSource: unmatchedSource, @@ -53,9 +57,22 @@ enum TableColumnMatcher { ) } + /// The automatic match with the user's overrides laid over it. + static func match( + source: [String], + destination: [String], + overrides: [String: String?] + ) -> Match { + let automatic = match(source: source, destination: destination) + guard !overrides.isEmpty else { return automatic } + return applying(overrides: overrides, to: automatic, destination: destination) + } + /// Applies the user's overrides over an automatic match. An override to nil excludes the /// column, which is how a source column with no destination is deliberately dropped rather - /// than failing the transfer. + /// than failing the transfer. An override may point at a column another source column + /// already holds; that is kept and reported through `contestedDestinations`, not resolved + /// by quietly unmapping the other one. static func applying( overrides: [String: String?], to match: Match, @@ -79,4 +96,38 @@ enum TableColumnMatcher { unmatchedDestination: destination.filter { !claimed.contains($0) } ) } + + /// Destination columns named by more than one entry of `mapping`, sorted. + static func contestedDestinations(in mapping: [String: String]) -> [String] { + var sourceCount: [String: Int] = [:] + for target in mapping.values { + sourceCount[target, default: 0] += 1 + } + return sourceCount.filter { $0.value > 1 }.keys.sorted() + } + + /// Pairs each name with a candidate of the same spelling, then with a candidate left over that + /// differs only by case, earlier names first. A candidate is paired with one name at most. + private static func pair(_ names: [String], with candidates: [String]) -> [String?] { + var pairs = [String?](repeating: nil, count: names.count) + var claimed = Set() + let spelled = Set(candidates) + for (index, name) in names.enumerated() { + guard spelled.contains(name), !claimed.contains(name) else { continue } + pairs[index] = name + claimed.insert(name) + } + + var unclaimedByFolded: [String: [String]] = [:] + for candidate in candidates where !claimed.contains(candidate) { + unclaimedByFolded[candidate.lowercased(), default: []].append(candidate) + } + for (index, name) in names.enumerated() where pairs[index] == nil { + let folded = name.lowercased() + guard var remaining = unclaimedByFolded[folded], !remaining.isEmpty else { continue } + pairs[index] = remaining.removeFirst() + unclaimedByFolded[folded] = remaining + } + return pairs + } } diff --git a/TablePro/Core/Services/Export/TableTransferService.swift b/TablePro/Core/Services/Export/TableTransferService.swift index 993211a099..9491539dd7 100644 --- a/TablePro/Core/Services/Export/TableTransferService.swift +++ b/TablePro/Core/Services/Export/TableTransferService.swift @@ -14,6 +14,7 @@ enum TableTransferError: LocalizedError { case sameConnectionAndContainer case targetMissing(table: String) case noMatchingColumns(table: String) + case contestedDestination(table: String, columns: [String]) case transferFailed(String) var errorDescription: String? { @@ -30,6 +31,11 @@ enum TableTransferError: LocalizedError { return String( format: String(localized: "No column of %@ matches a column on the destination table."), table) + case .contestedDestination(let table, let columns): + return String( + format: String(localized: "%1$@: more than one source column is mapped to %2$@."), + table, + columns.joined(separator: ", ")) case .transferFailed(let message): return String(format: String(localized: "Transfer failed: %@"), message) } @@ -137,6 +143,7 @@ final class TableTransferService: ObservableObject { ) async throws { let rowObjects = request.objects.filter { $0.kind.carriesRows } guard !rowObjects.isEmpty else { throw TableTransferError.noTablesSelected } + try Self.refuseContestedMappings(request.columnMapping, for: rowObjects) /// The flag is cleared on the way out, never on the way in. A Stop pressed while the sheet /// was still reading both sides' columns arrives before this line, and clearing it here @@ -168,6 +175,22 @@ final class TableTransferService: ObservableObject { state.warnings.append(contentsOf: source.cappedTableWarnings) } + /// Checked for every table before the first is written: the INSERT would name the contested + /// column twice, and the server refuses it only after "Delete existing rows first" has already + /// emptied the table, permanently when the table is not wrapped in a transaction. + nonisolated static func refuseContestedMappings( + _ mappings: [String: [String: String]], + for objects: [ExportObjectItem] + ) throws { + for object in objects { + guard let mapping = mappings[object.name] else { continue } + let contested = TableColumnMatcher.contestedDestinations(in: mapping) + guard contested.isEmpty else { + throw TableTransferError.contestedDestination(table: object.name, columns: contested) + } + } + } + /// The sink writes by column name and skips any field the mapping does not name, so an empty /// mapping writes nothing and reports every row as unmapped. A caller that supplies no mapping /// gets one matched by name, and a table whose columns match nothing is refused by name here diff --git a/TablePro/Resources/Localizable.xcstrings b/TablePro/Resources/Localizable.xcstrings index 4732c63b49..cc567d665b 100644 --- a/TablePro/Resources/Localizable.xcstrings +++ b/TablePro/Resources/Localizable.xcstrings @@ -3470,6 +3470,9 @@ }, "%@ loses its default" : { + }, + "%@ mapped more than once" : { + }, "%@ matches" : { @@ -5930,6 +5933,9 @@ }, "%1$@: %2$@ had no column of that name on the destination." : { + }, + "%1$@: more than one source column is mapped to %2$@." : { + }, "%1$@: only the first %2$lld rows were read, the most this database returns from one query." : { @@ -19907,6 +19913,9 @@ }, "Another profile already has this name." : { + }, + "Another source column is mapped to the same destination column." : { + }, "ANTHROPIC_API_KEY and ANTHROPIC_AUTH_TOKEN are removed from the tool's environment, so replies always draw on the subscription." : { "localizations" : { @@ -57294,6 +57303,9 @@ }, "Each database is written to its own file." : { + }, + "Each destination column can be mapped from only one source column." : { + }, "Each value is split at every match. Rows with fewer pieces get empty cells." : { diff --git a/TablePro/Views/Export/TableTransferMappingEditor.swift b/TablePro/Views/Export/TableTransferMappingEditor.swift index cabf89e64b..8b4ae7898c 100644 --- a/TablePro/Views/Export/TableTransferMappingEditor.swift +++ b/TablePro/Views/Export/TableTransferMappingEditor.swift @@ -11,24 +11,45 @@ import SwiftUI /// the two schemas were renamed apart. Without this the only way to correct that would be to rename /// a column on one side. internal struct TableTransferMappingEditor: View { + internal static var contestedMappingMessage: String { + String(localized: "Each destination column can be mapped from only one source column.") + } + internal let tableName: String internal let sourceColumns: [String] internal let destinationColumns: [String] - @Binding internal var overrides: [String: String?] + internal let onChange: ([String: String?]) -> Void internal let dismiss: () -> Void - private var automatic: TableColumnMatcher.Match { - TableColumnMatcher.match(source: sourceColumns, destination: destinationColumns) + /// Held here rather than read back through the sheet: SwiftUI does not re-evaluate `.popover` + /// content when the presenting view re-renders, so a pick that only wrote the sheet's state + /// left this view drawing the mapping it opened with. + @State private var overrides: [String: String?] + + internal init( + tableName: String, + sourceColumns: [String], + destinationColumns: [String], + overrides: [String: String?], + onChange: @escaping ([String: String?]) -> Void, + dismiss: @escaping () -> Void + ) { + self.tableName = tableName + self.sourceColumns = sourceColumns + self.destinationColumns = destinationColumns + self.onChange = onChange + self.dismiss = dismiss + _overrides = State(initialValue: overrides) } private var resolved: TableColumnMatcher.Match { - overrides.isEmpty - ? automatic - : TableColumnMatcher.applying( - overrides: overrides, to: automatic, destination: destinationColumns) + TableColumnMatcher.match( + source: sourceColumns, destination: destinationColumns, overrides: overrides) } internal var body: some View { + let match = resolved + let contested = Set(match.contestedDestinations) VStack(alignment: .leading, spacing: 10) { Text(tableName) .font(.headline) @@ -42,36 +63,28 @@ internal struct TableTransferMappingEditor: View { ScrollView { VStack(alignment: .leading, spacing: 4) { ForEach(sourceColumns, id: \.self) { column in - HStack(spacing: 6) { - Text(column) - .lineLimit(1) - .truncationMode(.middle) - .frame(width: 130, alignment: .leading) - - Picker(String(format: String(localized: "Destination for %@"), column), - selection: binding(for: column)) { - Text("Skip").tag(String?.none) - ForEach(destinationColumns, id: \.self) { target in - Text(target).tag(String?.some(target)) - } - } - .labelsHidden() - .frame(width: 150) - } + row(for: column, isContested: match.mapping[column].map(contested.contains) ?? false) } } } .frame(height: 200) - if !resolved.unmatchedDestination.isEmpty { - Text(unmatchedDestinationLabel) + if !contested.isEmpty { + Text(Self.contestedMappingMessage) + .font(.caption) + .foregroundStyle(.red) + .fixedSize(horizontal: false, vertical: true) + } + + if !match.unmatchedDestination.isEmpty { + Text(unmatchedDestinationLabel(match.unmatchedDestination)) .font(.caption) .foregroundStyle(.secondary) .fixedSize(horizontal: false, vertical: true) } HStack { - Button("Match by Name") { overrides = [:] } + Button("Match by Name") { update([:]) } Spacer() Button("Done", action: dismiss) .keyboardShortcut(.defaultAction) @@ -81,19 +94,55 @@ internal struct TableTransferMappingEditor: View { .frame(width: 340) } + private func row(for column: String, isContested: Bool) -> some View { + HStack(spacing: 6) { + Text(column) + .lineLimit(1) + .truncationMode(.middle) + .frame(width: 130, alignment: .leading) + + Picker(String(format: String(localized: "Destination for %@"), column), + selection: binding(for: column)) { + Text("Skip").tag(String?.none) + ForEach(destinationColumns, id: \.self) { target in + Text(target).tag(String?.some(target)) + } + } + .labelsHidden() + .frame(width: 150) + + if isContested { + Image(systemName: "exclamationmark.triangle.fill") + .foregroundStyle(.red) + .help(String(localized: "Another source column is mapped to the same destination column.")) + .accessibilityLabel( + Text("Another source column is mapped to the same destination column.")) + } + } + } + /// A destination column nothing writes to takes its own default or null, which only fails when /// it is `NOT NULL` without one, so it is stated rather than blocked. - private var unmatchedDestinationLabel: String { + private func unmatchedDestinationLabel(_ columns: [String]) -> String { String( format: String(localized: "Not written: %@. Each takes its default or null."), - resolved.unmatchedDestination.joined(separator: ", ") + columns.joined(separator: ", ") ) } private func binding(for column: String) -> Binding { Binding( get: { resolved.mapping[column] }, - set: { overrides[column] = .some($0) } + set: { target in + var updated = overrides + updated[column] = .some(target) + update(updated) + } ) } + + private func update(_ updated: [String: String?]) { + overrides = updated + onChange(updated) + } } diff --git a/TablePro/Views/Export/TableTransferSheet.swift b/TablePro/Views/Export/TableTransferSheet.swift index 2c488a013c..652f17b728 100644 --- a/TablePro/Views/Export/TableTransferSheet.swift +++ b/TablePro/Views/Export/TableTransferSheet.swift @@ -55,7 +55,13 @@ struct TableTransferSheet: View { } private var canTransfer: Bool { - !isRunning && !selectedTables.isEmpty && destinationConnection != nil + !isRunning && !selectedTables.isEmpty && destinationConnection != nil && !hasContestedMapping + } + + /// Two source columns mapped to one destination column build an INSERT naming it twice, which + /// the server refuses only after "Delete existing rows first" has emptied the table. + private var hasContestedMapping: Bool { + selectedTables.contains { !resolvedMatch(for: $0.name).contestedDestinations.isEmpty } } var body: some View { @@ -177,6 +183,11 @@ struct TableTransferSheet: View { .foregroundStyle(.secondary) .lineLimit(1) .truncationMode(.middle) + } else if hasContestedMapping { + Text(TableTransferMappingEditor.contestedMappingMessage) + .font(.caption) + .foregroundStyle(.red) + .lineLimit(2) } } actions: { Button(isRunning ? String(localized: "Stop") : String(localized: "Cancel")) { @@ -213,6 +224,7 @@ struct TableTransferSheet: View { @ViewBuilder private func mappingSummary(for table: String) -> some View { let match = resolvedMatch(for: table) + let cannotTransfer = match.isEmpty || !match.contestedDestinations.isEmpty if isMatching, destinationColumns[table] == nil { ProgressView() .scaleEffect(0.5) @@ -228,7 +240,7 @@ struct TableTransferSheet: View { HStack(spacing: 3) { Text(mappingLabel(match)) .font(.caption) - .foregroundStyle(match.isEmpty ? Color.red : .secondary) + .foregroundStyle(cannotTransfer ? Color.red : .secondary) Image(systemName: "arrow.left.arrow.right") .font(.caption) } @@ -242,10 +254,8 @@ struct TableTransferSheet: View { tableName: table, sourceColumns: sourceColumns[table] ?? [], destinationColumns: destinationColumns[table] ?? [], - overrides: Binding( - get: { overrides[table] ?? [:] }, - set: { overrides[table] = $0 } - ), + overrides: overrides[table] ?? [:], + onChange: { overrides[table] = $0 }, dismiss: { inspectedTable = nil } ) } @@ -254,6 +264,12 @@ struct TableTransferSheet: View { private func mappingLabel(_ match: TableColumnMatcher.Match) -> String { guard !match.isEmpty else { return String(localized: "No columns match") } + let contested = match.contestedDestinations + guard contested.isEmpty else { + return String( + format: String(localized: "%@ mapped more than once"), + contested.joined(separator: ", ")) + } guard match.unmatchedSource.isEmpty else { return String( format: String(localized: "%1$lld mapped, %2$lld skipped"), @@ -264,12 +280,10 @@ struct TableTransferSheet: View { } private func resolvedMatch(for table: String) -> TableColumnMatcher.Match { - let destination = destinationColumns[table] ?? [] - let automatic = TableColumnMatcher.match( - source: sourceColumns[table] ?? [], destination: destination) - guard let tableOverrides = overrides[table], !tableOverrides.isEmpty else { return automatic } - return TableColumnMatcher.applying( - overrides: tableOverrides, to: automatic, destination: destination) + TableColumnMatcher.match( + source: sourceColumns[table] ?? [], + destination: destinationColumns[table] ?? [], + overrides: overrides[table] ?? [:]) } private func binding(for table: ExportObjectItem) -> Binding { diff --git a/TableProTests/Core/Export/TableColumnMatcherTests.swift b/TableProTests/Core/Export/TableColumnMatcherTests.swift new file mode 100644 index 0000000000..333a28ecae --- /dev/null +++ b/TableProTests/Core/Export/TableColumnMatcherTests.swift @@ -0,0 +1,119 @@ +// +// TableColumnMatcherTests.swift +// TableProTests +// + +import Foundation +import Testing + +@testable import TablePro + +struct TableColumnMatcherTests { + @Test("Columns are matched by name, and the rest are reported on both sides") + func matchesByName() { + let match = TableColumnMatcher.match( + source: ["id", "name", "legacy"], destination: ["id", "name", "created_at"]) + + #expect(match.mapping == ["id": "id", "name": "name"]) + #expect(match.unmatchedSource == ["legacy"]) + #expect(match.unmatchedDestination == ["created_at"]) + #expect(match.contestedDestinations.isEmpty) + } + + @Test("A column spelled with another case still matches") + func matchesIgnoringCase() { + let match = TableColumnMatcher.match(source: ["ID", "Email"], destination: ["id", "email"]) + + #expect(match.mapping == ["ID": "id", "Email": "email"]) + #expect(match.unmatchedSource.isEmpty) + } + + /// Both used to land on `name`, and the INSERT named it twice. + @Test("The exact spelling claims a destination column before a twin that differs only by case") + func exactSpellingWinsTheColumn() { + let match = TableColumnMatcher.match(source: ["Name", "name"], destination: ["id", "name"]) + + #expect(match.mapping == ["name": "name"]) + #expect(match.unmatchedSource == ["Name"]) + #expect(match.contestedDestinations.isEmpty) + } + + @Test("Of two twins that both differ by case from the destination, only the first is matched") + func firstTwinWinsTheColumn() { + let match = TableColumnMatcher.match(source: ["NAME", "Name"], destination: ["name"]) + + #expect(match.mapping == ["NAME": "name"]) + #expect(match.unmatchedSource == ["Name"]) + } + + @Test("Twins on both sides each reach their own column") + func twinsReachTheirOwnColumns() { + let match = TableColumnMatcher.match(source: ["Name", "name"], destination: ["name", "Name"]) + + #expect(match.mapping == ["Name": "Name", "name": "name"]) + #expect(match.unmatchedDestination.isEmpty) + } + + @Test("A destination twin left over is matched ignoring case once the exact spelling is taken") + func leftoverTwinMatchesIgnoringCase() { + let match = TableColumnMatcher.match(source: ["NAME", "Name"], destination: ["Name", "name"]) + + #expect(match.mapping == ["Name": "Name", "NAME": "name"]) + } + + @Test("An override onto a column another source column holds is kept and reported as contested") + func overrideOntoAHeldColumnIsContested() { + let match = TableColumnMatcher.match( + source: ["first_name", "last_name"], + destination: ["first_name", "last_name"], + overrides: ["last_name": "first_name"] + ) + + #expect(match.mapping == ["first_name": "first_name", "last_name": "first_name"]) + #expect(match.contestedDestinations == ["first_name"]) + #expect(match.unmatchedDestination == ["last_name"]) + } + + @Test("Skipping one of the two source columns clears the contest") + func skippingOneClearsTheContest() { + let match = TableColumnMatcher.match( + source: ["first_name", "last_name"], + destination: ["first_name", "last_name"], + overrides: ["last_name": "first_name", "first_name": nil] + ) + + #expect(match.mapping == ["last_name": "first_name"]) + #expect(match.contestedDestinations.isEmpty) + #expect(match.unmatchedSource == ["first_name"]) + } + + @Test("Repointing a case twin onto the column its twin holds is contested") + func repointedTwinIsContested() { + let match = TableColumnMatcher.match( + source: ["Name", "name"], + destination: ["name"], + overrides: ["Name": "name"] + ) + + #expect(match.contestedDestinations == ["name"]) + } + + @Test("No overrides gives the automatic match") + func noOverridesIsTheAutomaticMatch() { + let source = ["id", "Name", "name"] + let destination = ["id", "name"] + + #expect( + TableColumnMatcher.match(source: source, destination: destination, overrides: [:]) + == TableColumnMatcher.match(source: source, destination: destination) + ) + } + + @Test("Only destination columns named more than once are contested, in name order") + func contestedDestinationsAreSorted() { + let contested = TableColumnMatcher.contestedDestinations( + in: ["a": "z", "b": "z", "c": "y", "d": "x", "e": "x", "f": "x"]) + + #expect(contested == ["x", "z"]) + } +} diff --git a/TableProTests/Core/Export/TableTransferServiceTests.swift b/TableProTests/Core/Export/TableTransferServiceTests.swift index 7372ddffc6..15bd693899 100644 --- a/TableProTests/Core/Export/TableTransferServiceTests.swift +++ b/TableProTests/Core/Export/TableTransferServiceTests.swift @@ -9,7 +9,83 @@ import Testing @testable import TablePro +/// Serves one table's rows as a source, and records what reaches it as a destination. +private final class TransferStubDriver: PluginDatabaseDriver, @unchecked Sendable { + let header: [String] + let rows: [PluginRow] + let columns: [String] + private(set) var executedQueries: [String] = [] + private(set) var executedParameters: [[PluginCellValue]] = [] + + init(header: [String] = [], rows: [PluginRow] = [], columns: [String] = []) { + self.header = header + self.rows = rows + self.columns = columns + } + + var insertStatements: [String] { + executedQueries.filter { $0.uppercased().hasPrefix("INSERT") } + } + + func defaultExportQuery(table: String, schema: String?) -> String? { + "SELECT * FROM \(table)" + } + + func streamRows(query: String) -> AsyncThrowingStream { + let header = PluginStreamHeader( + columns: header, columnTypeNames: header.map { _ in "TEXT" }) + let rows = rows + return AsyncThrowingStream { continuation in + continuation.yield(.header(header)) + continuation.yield(.rows(rows)) + continuation.finish() + } + } + + func connect() async throws {} + func disconnect() {} + + func execute(query: String) async throws -> PluginQueryResult { + executedQueries.append(query) + return PluginQueryResult(columns: [], columnTypeNames: [], rows: [], rowsAffected: 0, executionTime: 0) + } + + func executeParameterized(query: String, parameters: [PluginCellValue]) async throws -> PluginQueryResult { + executedQueries.append(query) + executedParameters.append(parameters) + return PluginQueryResult(columns: [], columnTypeNames: [], rows: [], rowsAffected: 0, executionTime: 0) + } + + func fetchTables(schema: String?) async throws -> [PluginTableInfo] { [] } + + func fetchColumns(table: String, schema: String?) async throws -> [PluginColumnInfo] { + columns.map { PluginColumnInfo(name: $0, dataType: "TEXT") } + } + + func fetchIndexes(table: String, schema: String?) async throws -> [PluginIndexInfo] { [] } + func fetchForeignKeys(table: String, schema: String?) async throws -> [PluginForeignKeyInfo] { [] } + func fetchTableDDL(table: String, schema: String?) async throws -> String { "" } + func fetchViewDefinition(view: String, schema: String?) async throws -> String { "" } + + func fetchTableMetadata(table: String, schema: String?) async throws -> PluginTableMetadata { + PluginTableMetadata(tableName: table) + } + + func fetchDatabases() async throws -> [String] { [] } + + func fetchDatabaseMetadata(_ database: String) async throws -> PluginDatabaseMetadata { + PluginDatabaseMetadata(name: database) + } +} + struct TableTransferServiceTests { + private func adapter(_ driver: TransferStubDriver, type: DatabaseType) -> PluginDriverAdapter { + PluginDriverAdapter(connection: DatabaseConnection(name: "Test", type: type), pluginDriver: driver) + } + + private func occurrences(of needle: String, in text: String) -> Int { + text.components(separatedBy: needle).count - 1 + } @Test("A row is keyed by its header's column names, in order") func rowIsKeyedByHeader() { @@ -100,4 +176,103 @@ struct TableTransferServiceTests { #expect(request.wrapInTransaction) #expect(!request.deleteExistingRows) } + + @Test("A mapping that sends two source columns to one destination column is refused") + func contestedMappingIsRefused() { + let objects = [ExportObjectItem(name: "people", kind: .table)] + #expect(throws: TableTransferError.self) { + try TableTransferService.refuseContestedMappings( + ["people": ["first_name": "name", "last_name": "name"]], for: objects) + } + #expect(throws: Never.self) { + try TableTransferService.refuseContestedMappings( + ["people": ["first_name": "first_name", "last_name": "last_name"]], for: objects) + } + } + + /// The server refused the INSERT only after "Delete existing rows first" had run, and with no + /// transaction around the table the deletion stood: the destination was left empty. + @MainActor @Test("A contested mapping deletes nothing on the destination") + func contestedMappingDeletesNothing() async { + let source = TransferStubDriver(header: ["first_name", "last_name"], rows: [[.text("Ada"), .text("Lovelace")]]) + let destination = TransferStubDriver(columns: ["name"]) + let request = TableTransferService.Request( + objects: [ExportObjectItem(name: "people", kind: .table, isSelected: true)], + sourceType: .postgresql, + destinationType: .mysql, + columnMapping: ["people": ["first_name": "name", "last_name": "name"]], + deleteExistingRows: true, + wrapInTransaction: false + ) + + do { + try await TableTransferService().transfer( + request: request, + sourceDriver: adapter(source, type: .postgresql), + destinationDriver: adapter(destination, type: .mysql) + ) + Issue.record("A mapping naming one destination column twice was transferred") + } catch TableTransferError.contestedDestination(let table, let columns) { + #expect(table == "people") + #expect(columns == ["name"]) + } catch { + Issue.record("Unexpected error: \(error)") + } + #expect(destination.executedQueries.isEmpty) + } + + /// `Name` and `name` both matched a lone `name`, and even with only one of them mapped the + /// sink folded the other onto it, so the INSERT named the column twice. + @MainActor @Test("A source column that differs only by case from a mapped one is not written twice") + func caseTwinIsNotWrittenTwice() async throws { + let source = TransferStubDriver( + header: ["id", "Name", "name"], rows: [[.text("1"), .text("Display"), .text("login")]]) + let destination = TransferStubDriver(columns: ["id", "name"]) + let request = TableTransferService.Request( + objects: [ExportObjectItem(name: "people", kind: .table, isSelected: true)], + sourceType: .postgresql, + destinationType: .mysql, + sourceColumns: ["people": ["id", "Name", "name"]], + deleteExistingRows: true, + wrapInTransaction: false + ) + + let service = TableTransferService() + try await service.transfer( + request: request, + sourceDriver: adapter(source, type: .postgresql), + destinationDriver: adapter(destination, type: .mysql) + ) + + let insert = try #require(destination.insertStatements.first) + #expect(destination.insertStatements.count == 1) + #expect(occurrences(of: "`name`", in: insert) == 1) + #expect(destination.executedParameters.last?.contains(.text("login")) == true) + #expect(destination.executedParameters.last?.contains(.text("Display")) == false) + #expect(service.state.transferredRows == 1) + #expect(service.state.warnings.contains { $0.contains("Name") }) + } + + @MainActor @Test("Twins mapped by the user to their own columns each reach their own column") + func mappedTwinsReachTheirOwnColumns() async throws { + let source = TransferStubDriver( + header: ["Name", "name"], rows: [[.text("Display"), .text("login")]]) + let destination = TransferStubDriver(columns: ["display_name", "login"]) + let request = TableTransferService.Request( + objects: [ExportObjectItem(name: "people", kind: .table, isSelected: true)], + sourceType: .postgresql, + destinationType: .mysql, + columnMapping: ["people": ["Name": "display_name", "name": "login"]] + ) + + try await TableTransferService().transfer( + request: request, + sourceDriver: adapter(source, type: .postgresql), + destinationDriver: adapter(destination, type: .mysql) + ) + + let insert = try #require(destination.insertStatements.first) + #expect(occurrences(of: "`display_name`", in: insert) == 1) + #expect(occurrences(of: "`login`", in: insert) == 1) + } } diff --git a/TableProTests/Core/Plugins/ImportDataSinkAdapterMappingTests.swift b/TableProTests/Core/Plugins/ImportDataSinkAdapterMappingTests.swift index 2bf3fb7b61..c0482c8c8d 100644 --- a/TableProTests/Core/Plugins/ImportDataSinkAdapterMappingTests.swift +++ b/TableProTests/Core/Plugins/ImportDataSinkAdapterMappingTests.swift @@ -56,6 +56,46 @@ struct ImportDataSinkAdapterMappingTests { try await sink.insertRow(["NAME": .text("Ada")]) } + /// Every name used to be folded, so the twin landed on the mapped field's column and the INSERT + /// named it twice. + @Test("A field whose twin differing only by case is mapped exactly is left out") + func caseTwinOfAMappedFieldStaysUnmapped() { + let sink = adapter(mapping: ["Email": "email"]) + + let (columns, values) = sink.mappedColumnsAndValues(["Email": .text("work"), "email": .text("home")]) + + #expect(columns == ["email"]) + #expect(values == [.text("work")]) + } + + @Test("Two fields that differ only by case reach their own columns") + func caseTwinsReachTheirOwnColumns() { + let sink = adapter(mapping: ["Email": "work_email", "email": "home_email"]) + + let (columns, values) = sink.mappedColumnsAndValues(["Email": .text("work"), "email": .text("home")]) + + #expect(columns == ["home_email", "work_email"]) + #expect(values == [.text("home"), .text("work")]) + } + + @Test("A third spelling of two mapped fields that differ only by case reaches neither column") + func ambiguousMappingKeysDoNotFold() { + let sink = adapter(mapping: ["Email": "work_email", "email": "home_email"]) + + let (columns, _) = sink.mappedColumnsAndValues(["EMAIL": .text("x")]) + + #expect(columns.isEmpty) + } + + @Test("Two unknown spellings of one mapped field in a row reach neither column") + func ambiguousRowKeysDoNotFold() { + let sink = adapter(mapping: ["Name": "name"]) + + let (columns, _) = sink.mappedColumnsAndValues(["NAME": .text("a"), "name": .text("b"), "id": .text("1")]) + + #expect(columns.isEmpty) + } + /// A row carrying nothing has nothing to lose, so it passes through. Only a row holding values /// that reach no column is worth stopping for, and conflating the two would turn an empty /// object in an NDJSON file into a failed import. diff --git a/docs/features/import-export.mdx b/docs/features/import-export.mdx index 0817ca5eb0..cd029b7628 100644 --- a/docs/features/import-export.mdx +++ b/docs/features/import-export.mdx @@ -247,7 +247,13 @@ Right-click tables in the sidebar and choose **Transfer To…** to copy their ro -Rows only. The destination table has to exist and its column names have to match, because inventing DDL that crosses from one engine to another would create tables whose types quietly disagree with the data landing in them. A per-table row filter set in the export tree is not carried over; narrow the transfer by transferring fewer tables. +Rows only, into a destination table that already exists. A per-table row filter set in the export tree is not carried over; narrow the transfer by transferring fewer tables. + +### Column mapping + +Each ticked table shows how many of its columns map. Columns match by name, the same spelling first and then the same name in another case. Click the count to point a column at a different destination column, or pick **Skip** to leave it out. **Match by Name** puts every column back. + +A destination column takes one source column. Map a second column onto it and **Transfer** stays off until one of the two moves or is skipped. **Delete existing rows first** empties each destination table before writing. There is no undo. From 733ca6bcdec49bc994198542f613de633350a40c Mon Sep 17 00:00:00 2001 From: Ngo Quoc Dat Date: Tue, 29 Sep 2026 20:29:35 +0700 Subject: [PATCH 2/3] fix(plugins): keep a skipped import field out of the column its case twin maps to --- CHANGELOG.md | 1 + .../Core/Plugins/ImportDataSinkAdapter.swift | 36 ++++++++------ .../Core/Services/Export/ImportService.swift | 10 ++-- .../Services/Export/TableColumnMatcher.swift | 49 ++++++------------- .../Export/TableTransferService.swift | 1 + TablePro/Views/Import/RowImportSheet.swift | 20 ++++++-- .../Core/Export/TableColumnMatcherTests.swift | 25 ++++++++-- .../ImportDataSinkAdapterMappingTests.swift | 42 ++++++++++++++-- docs/features/import-export.mdx | 8 +-- 9 files changed, 124 insertions(+), 68 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index f63c5d3753..21e0253102 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -20,6 +20,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 - Explain Analyze running write statements on Read-Only connections and skipping the Alert and Safe Mode confirmation. - **Local only** connections taking edits and deletions made on another device. - Table Transfer emptying a destination table, then failing, when two source columns map to one column. +- File import failing, or writing a skipped field, when two fields differ only by case. ## [0.76.1] - 2026-09-29 diff --git a/TablePro/Core/Plugins/ImportDataSinkAdapter.swift b/TablePro/Core/Plugins/ImportDataSinkAdapter.swift index 33c42a6697..2ff9c7bd15 100644 --- a/TablePro/Core/Plugins/ImportDataSinkAdapter.swift +++ b/TablePro/Core/Plugins/ImportDataSinkAdapter.swift @@ -21,6 +21,11 @@ final class ImportDataSinkAdapter: PluginImportDataSink, @unchecked Sendable { /// `Name` and `name`, is left out, because a third spelling could mean either. private let mappingKeyByFoldedName: [String: String] + /// Every field the mapping was made from, the ones it leaves out included. The mapping alone + /// cannot tell a field the user skipped from a header cased differently, and folding the + /// skipped one wrote it into the column its twin was mapped to. + private let sourceFields: Set + private let rowGenerator: SQLStatementGenerator? /// Asked before every statement this sink sends, because one `insertRows` call is no longer one @@ -43,6 +48,7 @@ final class ImportDataSinkAdapter: PluginImportDataSink, @unchecked Sendable { databaseType: DatabaseType, targetTable: String? = nil, columnMapping: [String: String] = [:], + sourceFields: [String] = [], isCancelled: @escaping @Sendable () -> Bool = { false } ) { self.isCancelled = isCancelled @@ -54,6 +60,7 @@ final class ImportDataSinkAdapter: PluginImportDataSink, @unchecked Sendable { self.columnMapping = columnMapping self.mappingKeyByFoldedName = Dictionary(grouping: columnMapping.keys, by: { $0.lowercased() }) .compactMapValues { $0.count == 1 ? $0.first : nil } + self.sourceFields = Set(sourceFields) if let targetTable { self.rowGenerator = try? SQLStatementGenerator( tableName: targetTable, @@ -202,38 +209,37 @@ final class ImportDataSinkAdapter: PluginImportDataSink, @unchecked Sendable { /// source table holding both wrote one column twice and the server refused the INSERT. internal func mappedColumnsAndValues(_ values: [String: PluginCellValue]) -> ([String], [PluginCellValue]) { var pairs: [(column: String, value: PluginCellValue)] = [] - var unspelled: [(field: String, value: PluginCellValue)] = [] + var unknown: [(field: String, value: PluginCellValue)] = [] for (field, value) in values { - guard let column = columnMapping[field] else { - unspelled.append((field, value)) - continue + if let column = columnMapping[field] { + pairs.append((column, value)) + } else if !sourceFields.contains(field) { + unknown.append((field, value)) } - pairs.append((column, value)) } - if !unspelled.isEmpty { - pairs += caseFoldedPairs(unspelled, in: values) + if !unknown.isEmpty { + pairs += caseFoldedPairs(unknown, in: values) } pairs.sort { $0.column < $1.column } return (pairs.map(\.column), pairs.map(\.value)) } - /// A field spelled like no mapping key still reaches the key it matches ignoring case, such as - /// a header cased differently from the one the mapping was made from, but only when nothing - /// else in the row answers to that name: not the key's own spelling, and not a second field. + /// A field the mapping was not made from, such as a JSON key first seen past the sampled + /// documents, still reaches the key it matches ignoring case, but only when nothing else in the + /// row answers to that name: not the key's own spelling, and not a second field. private func caseFoldedPairs( - _ unspelled: [(field: String, value: PluginCellValue)], + _ unknown: [(field: String, value: PluginCellValue)], in values: [String: PluginCellValue] ) -> [(column: String, value: PluginCellValue)] { var fieldsPerFoldedName: [String: Int] = [:] - for entry in unspelled { - fieldsPerFoldedName[entry.field.lowercased(), default: 0] += 1 + for field in values.keys { + fieldsPerFoldedName[field.lowercased(), default: 0] += 1 } var pairs: [(column: String, value: PluginCellValue)] = [] - for (field, value) in unspelled { + for (field, value) in unknown { let folded = field.lowercased() guard fieldsPerFoldedName[folded] == 1, let key = mappingKeyByFoldedName[folded], - values[key] == nil, let column = columnMapping[key] else { continue } pairs.append((column, value)) } diff --git a/TablePro/Core/Services/Export/ImportService.swift b/TablePro/Core/Services/Export/ImportService.swift index f79877b44f..7891255296 100644 --- a/TablePro/Core/Services/Export/ImportService.swift +++ b/TablePro/Core/Services/Export/ImportService.swift @@ -57,7 +57,8 @@ final class ImportService: ObservableObject { ownsDecompressedFile: Bool = false, knownStatementCount: Int? = nil, targetTable: String? = nil, - columnMapping: [String: String] = [:] + columnMapping: [String: String] = [:], + sourceFields: [String] = [] ) async throws -> PluginImportResult { guard let plugin = PluginManager.shared.importPlugin(forFormat: formatId) else { throw PluginImportError.importFailed("Import format '\(formatId)' not found") @@ -138,7 +139,8 @@ final class ImportService: ObservableObject { source: source, progress: progress, targetTable: targetTable, - columnMapping: columnMapping + columnMapping: columnMapping, + sourceFields: sourceFields ) } } catch { @@ -208,13 +210,15 @@ final class ImportService: ObservableObject { source: any PluginImportSource, progress: PluginImportProgress, targetTable: String?, - columnMapping: [String: String] + columnMapping: [String: String], + sourceFields: [String] ) async throws -> PluginImportResult { let sink = ImportDataSinkAdapter( driver: driver, databaseType: connection.type, targetTable: targetTable, columnMapping: columnMapping, + sourceFields: sourceFields, isCancelled: { progress.isCancelled } ) return try await plugin.performImport(source: source, sink: sink, progress: progress) diff --git a/TablePro/Core/Services/Export/TableColumnMatcher.swift b/TablePro/Core/Services/Export/TableColumnMatcher.swift index d2da0eddfb..d196c167cf 100644 --- a/TablePro/Core/Services/Export/TableColumnMatcher.swift +++ b/TablePro/Core/Services/Export/TableColumnMatcher.swift @@ -39,60 +39,41 @@ enum TableColumnMatcher { /// match nothing. A destination column goes to one source column at most, so `Name` and `name` /// on the source never both land on a lone `name`. static func match(source: [String], destination: [String]) -> Match { - let targets = pair(source, with: destination) var mapping: [String: String] = [:] - var unmatchedSource: [String] = [] - for (column, target) in zip(source, targets) { - guard let target else { - unmatchedSource.append(column) - continue - } + for (column, target) in zip(source, pair(source, with: destination)) { + guard let target else { continue } mapping[column] = target } - let claimed = Set(mapping.values) - return Match( - mapping: mapping, - unmatchedSource: unmatchedSource, - unmatchedDestination: destination.filter { !claimed.contains($0) } - ) - } - - /// The automatic match with the user's overrides laid over it. - static func match( - source: [String], - destination: [String], - overrides: [String: String?] - ) -> Match { - let automatic = match(source: source, destination: destination) - guard !overrides.isEmpty else { return automatic } - return applying(overrides: overrides, to: automatic, destination: destination) + return resolved(mapping, source: source, destination: destination) } - /// Applies the user's overrides over an automatic match. An override to nil excludes the + /// The automatic match with the user's overrides laid over it. An override to nil excludes the /// column, which is how a source column with no destination is deliberately dropped rather /// than failing the transfer. An override may point at a column another source column /// already holds; that is kept and reported through `contestedDestinations`, not resolved /// by quietly unmapping the other one. - static func applying( - overrides: [String: String?], - to match: Match, - destination: [String] + static func match( + source: [String], + destination: [String], + overrides: [String: String?] ) -> Match { - var mapping = match.mapping - var unmatchedSource = Set(match.unmatchedSource) + var mapping = match(source: source, destination: destination).mapping for (sourceColumn, target) in overrides { guard let target, destination.contains(target) else { mapping.removeValue(forKey: sourceColumn) - unmatchedSource.insert(sourceColumn) continue } mapping[sourceColumn] = target - unmatchedSource.remove(sourceColumn) } + return resolved(mapping, source: source, destination: destination) + } + + /// Both sides' leftovers in their own table's column order, whichever way the mapping was made. + private static func resolved(_ mapping: [String: String], source: [String], destination: [String]) -> Match { let claimed = Set(mapping.values) return Match( mapping: mapping, - unmatchedSource: unmatchedSource.sorted(), + unmatchedSource: source.filter { mapping[$0] == nil }, unmatchedDestination: destination.filter { !claimed.contains($0) } ) } diff --git a/TablePro/Core/Services/Export/TableTransferService.swift b/TablePro/Core/Services/Export/TableTransferService.swift index 9491539dd7..34de499e57 100644 --- a/TablePro/Core/Services/Export/TableTransferService.swift +++ b/TablePro/Core/Services/Export/TableTransferService.swift @@ -168,6 +168,7 @@ final class TableTransferService: ObservableObject { databaseType: request.destinationType, targetTable: object.name, columnMapping: mapping, + sourceFields: request.sourceColumns[object.name] ?? [], isCancelled: { [flag = cancellationFlag] in flag.isCancelled } ) try await transferOne(object: object, from: source, into: sink, request: request) diff --git a/TablePro/Views/Import/RowImportSheet.swift b/TablePro/Views/Import/RowImportSheet.swift index 37c8f746b8..14c950e042 100644 --- a/TablePro/Views/Import/RowImportSheet.swift +++ b/TablePro/Views/Import/RowImportSheet.swift @@ -808,7 +808,13 @@ struct RowImportSheet: View { switch destination { case .existingTable: guard let table = selectedTargetTable else { return } - runImport(targetTable: table, mapping: existingMapping(), newTable: nil, scope: scope) + runImport( + targetTable: table, + mapping: existingMapping(), + sourceFields: mappings.map(\.field.name), + newTable: nil, + scope: scope + ) case .newTable: let name = newTableName.trimmingCharacters(in: .whitespaces) guard !name.isEmpty, let definition = newTableDefinition(tableName: name) else { @@ -816,7 +822,13 @@ struct RowImportSheet: View { showErrorDialog = true return } - runImport(targetTable: name, mapping: newTableMapping(), newTable: definition, scope: scope) + runImport( + targetTable: name, + mapping: newTableMapping(), + sourceFields: newColumns.map(\.field.name), + newTable: definition, + scope: scope + ) } } @@ -877,6 +889,7 @@ struct RowImportSheet: View { private func runImport( targetTable: String, mapping: [String: String], + sourceFields: [String], newTable: PluginCreateTableDefinition?, scope: DatabaseScope ) { @@ -895,7 +908,8 @@ struct RowImportSheet: View { encoding: .utf8, scope: scope, targetTable: targetTable, - columnMapping: mapping + columnMapping: mapping, + sourceFields: sourceFields ) await MainActor.run { showProgressDialog = false diff --git a/TableProTests/Core/Export/TableColumnMatcherTests.swift b/TableProTests/Core/Export/TableColumnMatcherTests.swift index 333a28ecae..d89a0e092e 100644 --- a/TableProTests/Core/Export/TableColumnMatcherTests.swift +++ b/TableProTests/Core/Export/TableColumnMatcherTests.swift @@ -98,15 +98,30 @@ struct TableColumnMatcherTests { #expect(match.contestedDestinations == ["name"]) } + /// The leftovers are out of name order on both sides, so a path that sorted them, as the + /// override path once did, would not compare equal. @Test("No overrides gives the automatic match") func noOverridesIsTheAutomaticMatch() { - let source = ["id", "Name", "name"] - let destination = ["id", "name"] + let source = ["zeta", "alpha", "id"] + let destination = ["id", "omega", "beta"] - #expect( - TableColumnMatcher.match(source: source, destination: destination, overrides: [:]) - == TableColumnMatcher.match(source: source, destination: destination) + let automatic = TableColumnMatcher.match(source: source, destination: destination) + + #expect(automatic.unmatchedSource == ["zeta", "alpha"]) + #expect(automatic.unmatchedDestination == ["omega", "beta"]) + #expect(TableColumnMatcher.match(source: source, destination: destination, overrides: [:]) == automatic) + } + + @Test("An override leaves the unmatched source columns in the source table's order") + func overrideKeepsSourceOrder() { + let match = TableColumnMatcher.match( + source: ["zeta", "alpha", "id", "name"], + destination: ["id", "name"], + overrides: ["name": nil] ) + + #expect(match.mapping == ["id": "id"]) + #expect(match.unmatchedSource == ["zeta", "alpha", "name"]) } @Test("Only destination columns named more than once are contested, in name order") diff --git a/TableProTests/Core/Plugins/ImportDataSinkAdapterMappingTests.swift b/TableProTests/Core/Plugins/ImportDataSinkAdapterMappingTests.swift index c0482c8c8d..487f420f13 100644 --- a/TableProTests/Core/Plugins/ImportDataSinkAdapterMappingTests.swift +++ b/TableProTests/Core/Plugins/ImportDataSinkAdapterMappingTests.swift @@ -14,12 +14,13 @@ import Testing /// line, and the stop modes halt on a mapping that matches nothing. @MainActor struct ImportDataSinkAdapterMappingTests { - private func adapter(mapping: [String: String]) -> ImportDataSinkAdapter { + private func adapter(mapping: [String: String], sourceFields: [String] = []) -> ImportDataSinkAdapter { ImportDataSinkAdapter( driver: MockDatabaseDriver(), databaseType: .mysql, targetTable: "people", - columnMapping: mapping + columnMapping: mapping, + sourceFields: sourceFields ) } @@ -48,14 +49,47 @@ struct ImportDataSinkAdapterMappingTests { try await sink.insertRow(["name": .text("Ada")]) } - /// The mapping is matched case-insensitively, so a header cased differently to the column still - /// reaches it rather than being refused. + /// A field the mapping was not made from, like a JSON key first seen past the sampled + /// documents, is matched ignoring case, so it reaches its column rather than being refused. @Test("Field matching ignores case") func fieldMatchingIgnoresCase() async throws { let sink = adapter(mapping: ["Name": "name"]) try await sink.insertRow(["NAME": .text("Ada")]) } + @Test("A spelling the mapping was not made from still reaches the field it matches ignoring case") + func unknownSpellingFolds() { + let sink = adapter(mapping: ["Name": "name"], sourceFields: ["Name", "id"]) + + let (columns, values) = sink.mappedColumnsAndValues(["NAME": .text("Ada")]) + + #expect(columns == ["name"]) + #expect(values == [.text("Ada")]) + } + + /// The mapping alone cannot tell a skipped field from a header cased differently, so a document + /// carrying only the skipped spelling had its value written into the column its twin maps to. + @Test("A field the user skipped is never written into the column its case twin maps to") + func skippedCaseTwinIsNotFolded() async { + let sink = adapter(mapping: ["Name": "name"], sourceFields: ["Name", "NAME"]) + + let (columns, _) = sink.mappedColumnsAndValues(["NAME": .text("x")]) + + #expect(columns.isEmpty) + await #expect(throws: PluginImportError.self) { + try await sink.insertRow(["NAME": .text("x")]) + } + } + + @Test("An unknown spelling beside a skipped field of the same name reaches no column") + func unknownSpellingBesideASkippedTwinDoesNotFold() { + let sink = adapter(mapping: ["Name": "name"], sourceFields: ["Name", "NAME"]) + + let (columns, _) = sink.mappedColumnsAndValues(["NAME": .text("skipped"), "name": .text("unknown")]) + + #expect(columns.isEmpty) + } + /// Every name used to be folded, so the twin landed on the mapped field's column and the INSERT /// named it twice. @Test("A field whose twin differing only by case is mapped exactly is left out") diff --git a/docs/features/import-export.mdx b/docs/features/import-export.mdx index cd029b7628..ce186d55df 100644 --- a/docs/features/import-export.mdx +++ b/docs/features/import-export.mdx @@ -249,16 +249,16 @@ Right-click tables in the sidebar and choose **Transfer To…** to copy their ro Rows only, into a destination table that already exists. A per-table row filter set in the export tree is not carried over; narrow the transfer by transferring fewer tables. + +**Delete existing rows first** empties each destination table before writing. There is no undo. + + ### Column mapping Each ticked table shows how many of its columns map. Columns match by name, the same spelling first and then the same name in another case. Click the count to point a column at a different destination column, or pick **Skip** to leave it out. **Match by Name** puts every column back. A destination column takes one source column. Map a second column onto it and **Transfer** stays off until one of the two moves or is skipped. - -**Delete existing rows first** empties each destination table before writing. There is no undo. - - ## Clipboard paste (CSV/TSV) Select a row in the data grid and press `Cmd+V` to paste tabular data straight in. Tabs parse as TSV, commas as CSV. From fdccf92b467d5956449fd3bf6bb3a62b2fafdfa3 Mon Sep 17 00:00:00 2001 From: Ngo Quoc Dat Date: Tue, 29 Sep 2026 21:01:16 +0700 Subject: [PATCH 3/3] refactor(plugins): pass the import source fields as a set --- TablePro/Core/Plugins/ImportDataSinkAdapter.swift | 4 ++-- TablePro/Core/Services/Export/ImportService.swift | 4 ++-- TablePro/Core/Services/Export/TableTransferService.swift | 2 +- TablePro/Views/Import/RowImportSheet.swift | 6 +++--- .../Core/Plugins/ImportDataSinkAdapterMappingTests.swift | 2 +- 5 files changed, 9 insertions(+), 9 deletions(-) diff --git a/TablePro/Core/Plugins/ImportDataSinkAdapter.swift b/TablePro/Core/Plugins/ImportDataSinkAdapter.swift index 2ff9c7bd15..596619012b 100644 --- a/TablePro/Core/Plugins/ImportDataSinkAdapter.swift +++ b/TablePro/Core/Plugins/ImportDataSinkAdapter.swift @@ -48,7 +48,7 @@ final class ImportDataSinkAdapter: PluginImportDataSink, @unchecked Sendable { databaseType: DatabaseType, targetTable: String? = nil, columnMapping: [String: String] = [:], - sourceFields: [String] = [], + sourceFields: Set = [], isCancelled: @escaping @Sendable () -> Bool = { false } ) { self.isCancelled = isCancelled @@ -60,7 +60,7 @@ final class ImportDataSinkAdapter: PluginImportDataSink, @unchecked Sendable { self.columnMapping = columnMapping self.mappingKeyByFoldedName = Dictionary(grouping: columnMapping.keys, by: { $0.lowercased() }) .compactMapValues { $0.count == 1 ? $0.first : nil } - self.sourceFields = Set(sourceFields) + self.sourceFields = sourceFields if let targetTable { self.rowGenerator = try? SQLStatementGenerator( tableName: targetTable, diff --git a/TablePro/Core/Services/Export/ImportService.swift b/TablePro/Core/Services/Export/ImportService.swift index 7891255296..9283e2fa05 100644 --- a/TablePro/Core/Services/Export/ImportService.swift +++ b/TablePro/Core/Services/Export/ImportService.swift @@ -58,7 +58,7 @@ final class ImportService: ObservableObject { knownStatementCount: Int? = nil, targetTable: String? = nil, columnMapping: [String: String] = [:], - sourceFields: [String] = [] + sourceFields: Set = [] ) async throws -> PluginImportResult { guard let plugin = PluginManager.shared.importPlugin(forFormat: formatId) else { throw PluginImportError.importFailed("Import format '\(formatId)' not found") @@ -211,7 +211,7 @@ final class ImportService: ObservableObject { progress: PluginImportProgress, targetTable: String?, columnMapping: [String: String], - sourceFields: [String] + sourceFields: Set ) async throws -> PluginImportResult { let sink = ImportDataSinkAdapter( driver: driver, diff --git a/TablePro/Core/Services/Export/TableTransferService.swift b/TablePro/Core/Services/Export/TableTransferService.swift index 34de499e57..0177aeb37a 100644 --- a/TablePro/Core/Services/Export/TableTransferService.swift +++ b/TablePro/Core/Services/Export/TableTransferService.swift @@ -168,7 +168,7 @@ final class TableTransferService: ObservableObject { databaseType: request.destinationType, targetTable: object.name, columnMapping: mapping, - sourceFields: request.sourceColumns[object.name] ?? [], + sourceFields: Set(request.sourceColumns[object.name] ?? []), isCancelled: { [flag = cancellationFlag] in flag.isCancelled } ) try await transferOne(object: object, from: source, into: sink, request: request) diff --git a/TablePro/Views/Import/RowImportSheet.swift b/TablePro/Views/Import/RowImportSheet.swift index 14c950e042..60b6855a09 100644 --- a/TablePro/Views/Import/RowImportSheet.swift +++ b/TablePro/Views/Import/RowImportSheet.swift @@ -811,7 +811,7 @@ struct RowImportSheet: View { runImport( targetTable: table, mapping: existingMapping(), - sourceFields: mappings.map(\.field.name), + sourceFields: Set(mappings.map(\.field.name)), newTable: nil, scope: scope ) @@ -825,7 +825,7 @@ struct RowImportSheet: View { runImport( targetTable: name, mapping: newTableMapping(), - sourceFields: newColumns.map(\.field.name), + sourceFields: Set(newColumns.map(\.field.name)), newTable: definition, scope: scope ) @@ -889,7 +889,7 @@ struct RowImportSheet: View { private func runImport( targetTable: String, mapping: [String: String], - sourceFields: [String], + sourceFields: Set, newTable: PluginCreateTableDefinition?, scope: DatabaseScope ) { diff --git a/TableProTests/Core/Plugins/ImportDataSinkAdapterMappingTests.swift b/TableProTests/Core/Plugins/ImportDataSinkAdapterMappingTests.swift index 487f420f13..f15e5b480f 100644 --- a/TableProTests/Core/Plugins/ImportDataSinkAdapterMappingTests.swift +++ b/TableProTests/Core/Plugins/ImportDataSinkAdapterMappingTests.swift @@ -14,7 +14,7 @@ import Testing /// line, and the stop modes halt on a mapping that matches nothing. @MainActor struct ImportDataSinkAdapterMappingTests { - private func adapter(mapping: [String: String], sourceFields: [String] = []) -> ImportDataSinkAdapter { + private func adapter(mapping: [String: String], sourceFields: Set = []) -> ImportDataSinkAdapter { ImportDataSinkAdapter( driver: MockDatabaseDriver(), databaseType: .mysql,