Score the foreign key and IGNORE_DUP_KEY properties (#917, #916)

[stroexd]

Closes #917. Closes #916.

One change for both, as you asked, because the order of the two terms decides the case where they meet.

The terms

Current score: clustered 1000, unique index 500 / unique constraint 50, seeks 200, scans 100, includes 50. Two added:

Term	Value
A foreign key is backed by this index	800
This structure skips a duplicate key	400

Each has to outrank every term that can differ between the two structures of a pair, which is seeks + scans + includes, 350 together. 400 clears that.

The foreign key term then has to outrank the IGNORE_DUP_KEY one plus those three, 750, so 800. That ordering is your interaction case from #916: a strict unique index backing a key against an IGNORE_DUP_KEY duplicate backing nothing. Keeping the key's index and giving it the option is the one outcome where nothing changes, and I measured both directions on 2022 CU27.

On your note about the clustered 1000: neither term reaches it alone. The sum can pass it, and that is unreachable because a clustered index and a primary key carry is_eligible_for_dedupe = 0 and never enter a pair. Keeping the sums under 1000 is not possible anyway - the ordering forces the foreign key term past 700, and a unique index already carries 500.

The carry-over

The term makes the skipper win its own pair, so the common case needs nothing carried. Two things still reach a keeper that does not skip:

• a pair the skipper loses, which today means a foreign key is backed by the other structure
• Rule 7, which reads no priority at all: it keeps the nonclustered index and drops the constraint whichever of them skips

A rebuilt keeper carries the option in its WITH list. That arm existed from #914 for a replaced constraint; it now reads #ignore_dup_key_carried, which is the whole population, so the two consumers share one definition. A keeper that is not rebuilt gets its own row, sort_order 6, between the merges and the disables. That ordering is the point rather than cosmetics: while either structure skips, SQL Server skips, so the keeper has to be skipping before the loser is disabled.

Your #916 case 3 is the one #914 writes no script for - a Rule 7 replacement that is already unique, where the rebuild would add neither a column nor uniqueness. That is now the ALTER.

Two things I measured that neither issue has

ALTER INDEX ... SET (IGNORE_DUP_KEY = ON) is refused against a unique constraint's backing index. Error 1979, "Cannot use index option ignore_dup_key to alter index '...' as it enforces a primary or unique constraint". So a constraint keeper cannot be given the option after the fact, and your option 1 is unavailable for it. Rebuilding one means dropping and re-adding the constraint, a window with no uniqueness in it for the sake of removing a duplicate.

That case is unreachable, and the term ordering is why. A constraint keeper only comes out of Rule 7.5b, which pairs two constraints, and there the skipping one outranks the strict one (450 against 50 plus usage, which caps at 400). The only term that could lift a strict constraint past it is the foreign key one, and the rules exclude a column a foreign key references from that pairing: I built that exact pair as a fixture and it reaches no dedupe rule at all, just a compression row each.

I started by building your option 2 for it - keep both, with a KEPT - IGNORE_DUP_KEY row - and then took it back out when the fixture proved nothing reaches it. Dead code with a passing test is worse than no code. What stayed is one predicate: keeper_is_unique_constraint filters the script, so a change to the rules' foreign key exclusion cannot turn into a generated script that errors. Tell me if you would rather have the guard anyway.

Your three points from #917

1. Done, above.
2. Already handled. A MERGE INCLUDES winner rebuilds through DROP_EXISTING with the merged list, so promoting the backing index absorbs the loser's includes rather than losing them with the DISABLE.
3. Said on purpose now. Both structures back keys means no loser to pick, so Never disable an index a foreign key is backed by (#902) #913's guard keeps both - that is still the only safe answer, and it is the guard rather than the score that says so.

What this changes in the existing tests

Group 13's fixture has the backing index sorting last, which is exactly the arm where #913's guard used to fire. It wins on the term now, so:

• 13a unchanged: the backing index is still not disabled, for a better reason.
• 13b flipped: its duplicate IS disabled now, naming the backing index. That is the half sp_IndexCleanup: make a foreign key's backing index the deliberate winner of its dedupe pair, not the alphabetical one #917 asked for.
• 13c flipped: no KEPT - FOREIGN KEY row, because the guard had nothing to do. Drawing one where the index won on merit would claim the guard did something it never did, which is the distinction your Never disable an index a foreign key is backed by (#902) #913 review asked for.

The guard stays as the belt for the rules that do not read priority.

Verified

run_tests.py: 56 passed, 0 failed on 2022 CU27, group 17 added with eight assertions.

End to end, applying the generated scripts in the order the results give them and rolling back:

17b: duplicate parent key skipped, orphan child error 547, key is_disabled 0 is_not_trusted 0
17b without the carry-over (dev today): error 2601
17c: duplicate key skipped
17c without the carry-over (dev today): error 2601
17a: duplicate key skipped, nothing to carry because the strict one is the loser

rule_coverage_test.py and fixture_cases_test.py I did not run: the first wants 14 days of uptime and the second a real StackOverflow database, and this container has neither.

[stroexd]

All four matrix jobs were red, and the two failing assertions were mine - in rule_coverage_test.py, asserting the behaviour this branch deliberately changes.

K-ForeignKeyWinner: K1 positive control: ux_z3_code is KEPT - FOREIGN KEY  (found 0)
K-ForeignKeyWinner: K1: ux_z3_code is not disabled  (found ['MERGE SCRIPT'])

K1's fixture backs its foreign key with the index that loses on name: ux_z3_code is created first, the constraint binds to it, then ux_a3_code appears. Those assertions were written for the world before this branch, where index_priority could not see a foreign key: the pair was settled on index_name, the FK index was picked as the loser, and the #902 guard had to rescue it and label the row KEPT - FOREIGN KEY.

The term decides the pair now. The FK index wins it, so nothing tries to disable it, so there is no guard row to find. The failure was the test holding on to the premise the fix removes. K1 is now the mirror of K2 - FK-backed index wins, absorbs the duplicate's includes, duplicate disabled - and K3 keeps its guard assertion, because both of its indexes back a key and neither may go. K3 passed throughout.

The part I did not want to assume: whether absorbing includes is safe on an index a foreign key is backed by. The winner's rebuild is CREATE INDEX ... WITH (DROP_EXISTING = ON), so it keeps the key columns and the uniqueness the constraint needs, while a plain DROP INDEX would be refused. Measured rather than reasoned, on SQL Server 2022 CU27 (16.0.4295.3) against this exact fixture:

fk_name      key_index_id  backing_index
fk_ic_sg_c3  2             ux_z3_code        <- the constraint really is on the name loser

MERGE lief durch
fk_still_bound
ux_z3_code                                   <- still bound after the DROP_EXISTING rebuild

So the generated script is executable and leaves the constraint intact, which is what makes "the FK index wins and merges" the right outcome rather than a convenient one. That measurement is in the test comment, with the build it was taken on.

How I ran it. I rebuilt the job locally rather than pushing at CI: mcr.microsoft.com/mssql/server:2022-latest, go-sqlcmd v1.10.0 as the workflow installs, sp_IndexCleanup into master, the StackOverflow2013 fixture, then all four harnesses with the same arguments as the step.

harness	before	after
run_tests.py	56 pass	56 pass
fixture_cases_test.py	30 pass	30 pass
rule_coverage_test.py	90 pass, 2 fail	94 pass
no_access_test.py	4 pass	4 pass

The count went to 94 because K1 gained two assertions beyond the corrected pair: that the FK-backed winner's script really carries INCLUDE ([e1], [e2]), and that nothing disables or drops it. The two that were already passing on K1 - no MERGE on ux_a3_code, and nothing claiming it supersedes ux_z3_code - are untouched.