Skip to content

sp_IndexCleanup: make a foreign key's backing index the deliberate winner of its dedupe pair, not the alphabetical one #917

Closed

Description

@stroexd

Your second suggestion from the #913 review, written up so it is not lost in a PR thread. Happy to take it.

Where it stands after #913

#913 keeps both indexes of the pair when the loser backs a foreign key. The useful half of what you suggested already happens, though, and only by luck. Measured on 2022 CU27 with two mirror fixtures:

backing index is ux_z_code (sorts last)   -> KEPT - FOREIGN KEY on ux_z_code, no DISABLE, duplicate stays
backing index is ux_a_code2 (sorts first) -> DISABLE on ux_z_code2, duplicate cleaned up, no KEPT row

Same shape of table both times. The only difference is which of the two happens to sort first, because Rule 2 breaks a priority tie on index_name. So when the backing index wins, the duplicate goes today and nothing needs changing; when it loses, #913 stops the DISABLE and the duplicate stays.

What would finish it

  1. Make the backing index win on purpose. A term in index_priority for "a foreign key is backed by this index" would settle the pair the right way round before any rule picks a loser, instead of leaving it to the alphabet. That also makes Never disable an index a foreign key is backed by (#902) #913's guard the belt rather than the braces: it would still catch anything a rule decides on a basis other than priority, which is most of them.
  2. Merge includes into it where the loser covers more. If the duplicate carries includes the backing index does not, promoting the backing index has to absorb them or the coverage goes with the DISABLE, which is the failure shape your tests README calls the characteristic one.
  3. Decide the case that has no answer. When both indexes of the pair back foreign keys there is no loser to pick at all. Keeping both, as Never disable an index a foreign key is backed by (#902) #913 does, is the only safe answer I can see, but it is worth saying so on purpose rather than by omission.

What I would want to measure first

Whether (1) alone is enough in practice, or whether a rule that does not read priority still picks the backing index as the loser. Rule 3 disables the narrower index of a subset pair and Rule 7.5 works on constraints, and neither consults index_priority the way Rule 2 does. If one of them can still do it, the priority term is a nicety and the guard is what actually protects the key, which is worth knowing before adding a magic number to the scoring.

Also whether a FK-backing index should outrank a clustered index's 1000, which it should not, so the term has to sit below that.

Happy to be told this is not worth the complexity. #913 already closes the data-integrity half, and this part is about getting the duplicate cleaned up as well.

Activity

  1. erikdarlingdata commented on Oct 5, 2026

    @erikdarlingdata
    Owner

    I ran your mirror fixtures, and the rules that you asked about, against dev at f0994f0 (#913, #914 and #915 merged). I used SQL Server 2022 and @dedupe_only = 1.

    Your measurement holds on dev. In Rule 2, when the backing index sorts last, both indexes stay, and the report shows KEPT - FOREIGN KEY. When the backing index sorts first, the other duplicate gets its DISABLE SCRIPT. Rule 5 (same keys, different includes) and Rule 7.5b (two unique constraints) also kept both when the backing structure sorts last.

    The rules that do not read index_priority never picked the backing index:

    • Rule 3: a unique index that backs a key was not disabled as the narrower index of a subset pair. It got only a compression row.
    • Rule 7 and Rule 7.5: when the foreign key references a unique constraint, the procedure left the constraint alone. It also left the nonclustered index with the same keys alone, unique or not. It wrote no DROP CONSTRAINT, no MAKE UNIQUE script and no KEPT row.

    So a term in index_priority covers every case that I found, because Rules 2, 5 and 7.5b pick by priority and then by name. The guard from #913 still catches anything else.

    About the clustered index's 1000: clustered indexes and primary keys never take part in these rules, because is_eligible_for_dedupe is 0 for them. The term has to outrank the terms that can differ between the two indexes of a pair, such as the seek, scan and include terms.

    #916 needs the same lever for IGNORE_DUP_KEY, and the two terms interact. The section "The same decision as #917" in #916 has the case and a measurement.

    If you want to work this, go ahead. I will not touch it. #916 is yours too if you want it, because the two terms have to be designed together.

  2. added 2 commits that reference this issue on Oct 14, 2026
  3. erikdarlingdata commented on Oct 14, 2026

    @erikdarlingdata
    Owner

    Fixed by #920, merged to dev in b81754f. An index a foreign key is backed by now wins its dedupe pair on an index_priority term instead of on name, absorbs the loser's includes through DROP_EXISTING, and the #913 guard stays as the belt for the rules that do not read priority.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions