Copy-DbaDbTableData - Map query columns onto the writable destination columns - #10666
Copy-DbaDbTableData - Map query columns onto the writable destination columns#10666andreasjordan wants to merge 4 commits into
Conversation
… columns With -Query and without -ForceExplicitMapping the command added no column mappings and left SqlBulkCopy to map by position. SqlBulkCopy's own list contains every destination column: a computed column makes the server reject the insert (error 271), and a rowversion column is silently dropped together with the source column that lands on it - every column behind it shifts by one, the last one stays empty, and the command reports success. That is the silent data corruption of #10661 (reproduced in the lab with a rowversion column; a computed column fails loudly on this stack). The command now builds the positional mapping itself, source ordinal by source ordinal onto the destination columns that can actually be written - computed and rowversion columns excluded. An identity placeholder keeps working as documented, because a mapping onto an identity column without -KeepIdentity is still ignored by the server. A query that returns more columns than the destination can take is refused with a message naming both counts instead of dropping the surplus. Table mode and -ForceExplicitMapping keep their name-based mapping; the only change there is that rowversion columns are no longer mapped, which SqlBulkCopy tolerated anyway. Help text updated accordingly. Fixes #10661 (do Copy-DbaDbTableData) Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…message Style only, no behaviour change: the verbose message the previous commit reworded still wrapped the column name in single quotes, the repository standard is double quotes only. (do Copy-DbaDbTableData) Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…as created with The two Query-mode tests select "Id" from dbo.dbatoolsci_example4, whose column is "id". On a case insensitive instance that is the same column; on a case sensitive one the server answers "Invalid column name 'Id'" and the test fails with "Expected 1, but got $null". Found by the first setCS run whose COPY lane sits on the case sensitive SQL05 instances. (do Copy-DbaDbTableData) Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
potatoqualitee
left a comment
There was a problem hiding this comment.
The positional destination mapping still treats temporal and ledger GENERATED ALWAYS columns as writable. For example, with destination columns A, ValidFrom, B, ValidTo and a query returning A, B, source B maps to ValidFrom and SqlBulkCopy fails because GENERATED ALWAYS period columns cannot accept explicit values. This materially breaks copying query results into supported temporal destinations. Exclude columns whose version-supported GeneratedAlwaysType is not None, guard the metadata lookup for older SQL Server versions, and add an interleaved temporal-column regression test.
|
Ich can try to have a look at this tomorrow. |
…ble destination columns The period columns of a temporal table and the ledger metadata columns are GENERATED ALWAYS: not computed, not rowversion, but just as unwritable, and SMO reports them with Computed = false. The positional mapping counted them, mapped query columns onto them and the server rejected the insert with error 13536. The lookup is guarded by the version because SMO only supports GeneratedAlwaysType on SQL Server 2016 and later. (do Copy-DbaDbTableData) Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
|
Confirmed and fixed in 863aaa7. SMO reports the period columns of a temporal table with The filter now also excludes columns whose New regression test: a system-versioned temporal destination with the period columns interleaved between the writable ones ( created by Claude and reviewed by Andreas Jordan |
Fixes #10661
Problem
With
-Queryand without-ForceExplicitMapping, a destination table with a computed or a rowversion column between its writable columns makes the copy go wrong: either the server rejects the insert, or - worse - every column behind the unwritable one shifts by one position, the last column stays empty, no warning is written and the command reports success.Mechanism
In that mode the command added no column mappings at all and left
SqlBulkCopyto map by position (Copy-DbaDbTableData.ps1:710).SqlBulkCopy's own list contains every destination column:INSERT BULKwith error 271 ("cannot be modified because it is either a computed column or is the result of a UNION operator") - a loud failure;Both reproduced in the lab (Microsoft.Data.SqlClient 6.1.5, SQL Server 2022 to 2025). The report names a computed column; on our stack that shape fails loudly and the silent shift needs a rowversion column. The reporter has since verified that their table has no rowversion column at all, so with their driver/server combination the computed column itself is dropped silently - the loud-vs-silent behaviour is version dependent. The fix covers both, since the command no longer relies on the implicit mapping in either shape.
What changed
DataType.SqlDataType -eq "Timestamp"), i.e. it is the list of columns that can actually be written.-Querymode without-ForceExplicitMappingthe command now builds the positional mapping itself: query column n is mapped to the n-th writable destination column viaColumnMappings.Add([int], [string]). This is whatSqlBulkCopydid on its own, minus the unwritable columns.try/catchso it honours-EnableExceptionand-Continues to the next destination like every other failure there. Before, the surplus was either rejected by SqlBulkCopy with a generic error or silently lost.-Queryand-ForceExplicitMappingdescribes the new rule; the old text promised that ordinal mapping "will cause it to fail" on computed columns.Identity placeholders keep working as documented: a mapping onto an identity column without
-KeepIdentityis still ignored by the server, and with-KeepIdentitythe value is written - verified with rawSqlBulkCopybefore the change and by a test after it.What deliberately did not change
Table mode and
-ForceExplicitMappingkeep their name-based mapping. The only effect the new column list has there is that a source column named like a destination rowversion column is no longer mapped -SqlBulkCopytolerated that mapping anyway, so the result is identical.Tests
New
ContextinCopy-DbaDbTableData.Tests.ps1(COPY lane), asserting the data, not the row count:-KeepIdentity: identity generated, other columns right;Lab through the testing-dbatools harness (SQL 2022 -> SQL 2025): 18/18 green with the fix. Red on old with the command change stashed: 1 and 4 fail with the server's computed-column error, 2 fails with
Expected @(22, 222) But was @(33, 333)- the reported symptom; 3 passes on both, it pins the behaviour that must not change.Added while the PR was open
The two existing Query-mode tests ("relies on the default source database", "3 part query") selected
Idfrom a table whose column isid. Fine on CI, which is case insensitive, but on the case sensitive SQL05 instances of the lab the server answersInvalid column name 'Id'and both tests fail withExpected 1, but got $null. Fixed in the third commit by using the column name as created; verified with the query on the CS instance and by re-running the whole file against the case sensitive COPY pair (SQL05SQL2022->SQL05SQL2025): 18/18 green, where the same file failed both Query-mode tests in the morning run.The review pointed out that the writable-column filter still counted GENERATED ALWAYS columns - the period columns of a temporal table and the ledger metadata columns, which SMO reports with
Computed = $false. The fourth commit excludes columns whoseGeneratedAlwaysTypeis notNone, guarded with$destServer.VersionMajor -ge 13because SMO only supports the property on SQL Server 2016 and later, and adds a regression test with a system-versioned temporal destination whose period columns are interleaved between the writable ones. Green on both editions in the lab, red without the command change with exactly the server's 13536 error. Test file after the addition: 19/19 on both editions.Thanks to @JankeUwe for the report and the row-level inspection that made the shift undeniable.
created by Claude and reviewed by Andreas Jordan
🤖 Generated with Claude Code