preserve key order, type arguments and identity on read-back, and honour IgnoredIndexes - #511
Open
jakobt wants to merge 4 commits into
Open
preserve key order, type arguments and identity on read-back, and honour IgnoredIndexes#511jakobt wants to merge 4 commits into
jakobt wants to merge 4 commits into
Conversation
Column order is part of a composite key's identity, and all three catalog reads got it wrong in a different way. The index column query ordered by ic.index_column_id, which is the column's position in the table, not its position in the index. An index on (ProductId, Id) where Id is the first column of the table came back as (Id, ProductId) -- a different index, with a different leading column. It now orders by ic.key_ordinal, and separates included columns out first since they carry key_ordinal 0. The primary key query read INFORMATION_SCHEMA.CONSTRAINT_COLUMN_USAGE, which cannot express key order, with no ORDER BY at all; Table.PrimaryKeyColumns then derived the order from the column list. It now reads the backing index's key_ordinal, and the fetched table pins that order via SetPrimaryKeyOrder rather than re-deriving it. The foreign key query had no ORDER BY, and ForeignKey.ColumnNames/LinkedNames each sorted their input independently. For a composite key whose two sides do not sort into the same relative order that pairs the wrong columns together -- ColumnNames[i] must stay matched to LinkedNames[i]. The query now orders by constraint_column_id and the setters preserve what they are given. Found reverse-engineering a production database; the index case was reported by schema comparison as a table rebuild.
…catalog
Columns were read from information_schema.columns, which reports length only in
characters and exposes no is_identity. A decimal(18,2) came back as "decimal", a
datetime2(3) as "datetime2", and an IDENTITY column as an ordinary one.
Delta detection did not notice, because TableColumn compares RawType() and that
strips everything after the "(". What did notice is every path that regenerates
DDL from a fetched table. TableDelta.WriteRollback emits the actual column's
type, so rolling back an unrelated nullability change on Orders.Amount wrote
"alter column [Amount] decimal NOT NULL" -- decimal defaults to (18,0), which
truncates the scale of every row. The same lossy type feeds AlterColumnTypeSql
and the re-add of a dropped column, and a re-added IDENTITY column came back
plain.
Reading from sys.columns instead gives precision, scale and is_identity
directly. Lengths are converted back from bytes, halved for the Unicode types,
with -1 kept as MAX.
TableBase.IgnoreIndex is Weasel.Core API, and the PostgreSQL and SQLite table deltas both filter on it. SQL Server never consulted it, so an index the caller had explicitly asked Weasel to leave alone still landed in Indexes.Extras -- which WriteUpdate turns into "drop index". The documented escape hatch dropped the index it was supposed to protect, and AssertDatabaseMatchesConfiguration could never go green. Filtered on the expected table only, which is what the PostgreSQL twin does.
Reading the declared key order faithfully changes what the comparison sees, and comparing it positionally would report drift on schemas that are not drifted. A primary key model is built by flagging columns, so it can only ever express the table's column order. A key declared PRIMARY KEY (c, a) on a table whose columns are a, b, c therefore compared unequal to itself, and the patch dropped the constraint and re-added it as (a, c) -- reordering the clustered key, and rebuilding the table and every nonclustered index with it. Order is now compared only when the caller pinned it with SetPrimaryKeyOrder. A foreign key pairs its columns positionally, but the pairing rather than the order the pairs are written in is what defines the constraint. The two sides are now compared as a set of (column, linked column) pairs, so the same key written in another order is no longer a drop and recreate, while a key that genuinely pairs different columns still is. SetPrimaryKeyOrder now rejects a pin that repeats a column, names one outside the key, or covers only part of it -- each produced a migration that dropped the key and then failed to add the replacement. The pin orders the flagged set rather than replacing it, so it cannot resurrect a dropped column.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
IgnoreIndexdid nothing on SQL Server, though PostgreSQL and SQLite honour it — thegenerated patch dropped the index it was meant to protect.
could pair the wrong columns together.
decimal(18,2)read back asdecimal; rollback then emitted(18,0)and truncated the scale.