sp_IndexCleanup: carry index options through rebuilds and skip no-op merges

[erikdarlingdata]

Fixes #905
Fixes #906

Rebuild scripts keep IGNORE_DUP_KEY and STATISTICS_NORECOMPUTE (#905)

The procedure now reads two more settings for every index: sys.indexes.ignore_dup_key, and the no_recompute flag of the matching sys.stats row (stats_id = index_id). It collects them the same way PR #913 added the foreign key column.

The MERGE SCRIPT and the filtered-index include script add IGNORE_DUP_KEY = ON and STATISTICS_NORECOMPUTE = ON when the index being rebuilt has them. A MAKE UNIQUE script also takes IGNORE_DUP_KEY = ON from the unique constraint that it replaces. So inserts that SQL Server skipped before the constraint is dropped are still skipped after. Both columns exist on every supported version, so the change needs no version check.

The dynamic SQL that fills #index_details was close to 4,000 characters. At that length, a string with no max-type operand is cut off with no error. The string now starts from an empty nvarchar(max), so the new columns fit. A comment above it gives the reason.

No MERGE SCRIPT when the merge changes nothing (#906)

The old check asked whether Rule 6 had recorded merged includes. But Rule 6 records a row for every Key Superset winner, whether or not the include list changes. The check also looked only at MERGE INCLUDES rows. So it missed both cases in the issue.

The new check compares the winner's final include list with its own includes, rebuilt from #index_details in the same order. For MAKE UNIQUE rows, it also checks uniqueness. A winner that changes nothing becomes KEEP. It then gets the normal compression row or KEPT - NEEDS COMPRESSION. For example, ix9_ab keeps its compression advice, ix9_a keeps its DISABLE SCRIPT, and uq_t10_k keeps its DISABLE CONSTRAINT SCRIPT.

The winner is unwound when the foreign key guard keeps a loser (#913 follow-up)

The guard from #913 cleared superseded_by on the loser row, which was already NULL. The claim and the merged include list are on the winner. So the winner still got a MERGE SCRIPT that added the includes of an index that stays.

After the guard, the procedure now rebuilds the include list of each affected winner. The new list has the winner's own includes plus the includes of the losers that are still disabled. It rebuilds superseded_by from the same losers. A MERGE INCLUDES winner with no loser left goes back to KEEP. The guard's comment no longer says that this already happened.

If the winner backs the foreign key, the duplicate is still disabled. If both indexes back keys, neither is disabled.

rule_coverage_test.py no longer depends on uptime

Groups B and G now pass @dedupe_only = 1. Group B has the two @min_writes and @min_reads runs on the write-only pair, and Group G has both multi-database runs. On a server that is up more than 7 days, Rule 1 disabled the never-read fixture indexes. The three assertions that failed on a server up 8 days now pass.

Group E's fixture needed one change. Its subset now also includes filler, because its merge added nothing, and such a merge is no longer emitted. The control still checks that the key column is not included. Group G's fixture still produces a real include merge and a real MAKE UNIQUE.

New groups in rule_coverage_test.py:

• I: the options are carried. The test runs the generated script in a transaction that it rolls back. Then it reads ignore_dup_key, no_recompute and the result of a duplicate insert.
• J: merges that change nothing are not emitted, and the compression row stays. A real merge is still emitted.
• K: three foreign key cases. The key is backed by the loser, by the winner, or by both indexes.

With dev's procedure installed before this change, the new rule_coverage_test.py had 66 passes and 15 failures. These are the 15 failures:

• all 8 Group I assertions that check the fix
• 4 in Group J: no MERGE SCRIPT and a compression row for each of ix_sg_sup_ab and ux_sg_uqnoop
• 3 in Group K: ux_a3_code gets a MERGE SCRIPT, ux_a3_code is said to supersede ux_z3_code, and the winner where both indexes back keys is merged

The positive controls, the K2 regression case and the three uptime assertions pass on both.

Changes by the coordinator

After #915 merged, dev was merged into this branch. It merged with no conflicts.

run_tests.py assertion 1-UC: 1d: Unique index merge has CREATE UNIQUE failed with this change, because it expected the behavior that #906 removes. Its subset, ix_uc_ac, had no column that uix_uc_acd lacks, so the merge changed nothing. ix_uc_ac now includes col_e, so the merge adds a column. Assertion 1d now also checks that the script includes [col_e].

fixture_cases_test.py no longer runs 9-Executable: ix_test_7 (MERGE SCRIPT) executes, so it has 30 assertions, not 31. ix_test_7 (Age, CreationDate) already covers its subset, so it has no MERGE SCRIPT to run. The harness adds this assertion only when a MERGE SCRIPT exists. Three MERGE SCRIPTs still run in that harness: ix_c1, ix_test_3 and uq_test_1.

The coordinator also ran the generated scripts on SQL Server 2022, each inside a transaction that was rolled back:

• A MERGE SCRIPT for a unique index with both options kept ignore_dup_key = 1 and no_recompute = 1. A duplicate insert was skipped with no error.
• A MAKE UNIQUE script plus the DROP CONSTRAINT for a constraint with IGNORE_DUP_KEY = ON kept ignore_dup_key = 1. A duplicate insert was skipped.
• A filtered-index include script kept no_recompute = 1.

Tests

On SQL Server 2019 Developer, up 8 days, with dev and #915 merged in:

Harness	Result
run_tests.py	47 pass, 0 fail
fixture_cases_test.py	30 pass, 0 fail
rule_coverage_test.py	81 pass, 0 fail
no_access_test.py	4 pass, 0 fail

CI runs all four harnesses on SQL Server 2017, 2019, 2022 and 2025.

Not done

• SQL Server 2016 and Azure SQL were not tested. The change uses catalog columns that exist since SQL Server 2005, and no new syntax.
• SQL Server refuses IGNORE_DUP_KEY = ON on a filtered index (Msg 10618). So that option never reaches the filtered-index script. The script carries it anyway, to match the MERGE SCRIPT. Its test covers STATISTICS_NORECOMPUTE only.
• The MAKE UNIQUE branch of the winner unwind has no test. To reach it, a foreign key backed index must lose to a MAKE UNIQUE winner through Rule 7.6, and no fixture got there.

Unique structures that differ in IGNORE_DUP_KEY

An earlier version of this description named one case that changes behavior. In that case, an index that is already unique and has IGNORE_DUP_KEY = ON replaces a strict unique constraint. The behavior does not change. On SQL Server 2022, a strict structure and an IGNORE_DUP_KEY structure can share their keys. Then SQL Server skips the duplicate row, and the strict structure raises no error. This held in 16 combinations, including inserts with a forced wide plan.

The reverse cases do change behavior. In each one, the IGNORE_DUP_KEY structure is removed, and a strict one stays. Duplicate inserts then fail with error 2601 or 2627. These cases happen on dev before this change too, so they are in #916.

🤖 Generated with Claude Code

https://claude.ai/code/session_015SjCdz1y4HnJ5yNfFpN3Dv

[claude]

Claude finished @erikdarlingdata's task in 18s —— View job

---

Review of PR #914

I read the sp_IndexCleanup.sql diff. I did not run any of the test harnesses, so the results below are the PR's own numbers.

Verdict: no blocking issues.

• sp_IndexCleanup: DROP_EXISTING scripts turn off IGNORE_DUP_KEY and STATISTICS_NORECOMPUTE #905 option carry-through
  • ignore_dup_key and no_recompute are read once and carried into the MERGE SCRIPT and the filtered-index script.
  • sys.stats is read through QUOTENAME(@current_database_name), and the lookup is an EXISTS, so there are no duplicate rows.
  • Both columns exist on every supported version, so no version gating is needed.
  • Starting @sql from CONVERT(nvarchar(max), N'') is the right fix for the 4,000-character truncation, and the comment explains why.
  • Taking IGNORE_DUP_KEY from the replaced unique constraint on MAKE UNIQUE rows is sensible.
• sp_IndexCleanup: MERGE SCRIPT rebuilds an index even when the merge adds no columns #906 no-op merge check
  • Comparing the final include list with the index's own includes, rebuilt from #index_details in the same order, is more accurate than the old #merged_includes check.
  • The ia.is_unique = 1 condition for MAKE UNIQUE rows is correct.
• Winner unwind after the foreign key guard
  • It rebuilds both strings from table rows instead of editing the old text, which avoids name-substring and comma bugs.
  • The unwind is restricted to winners that have a loser in #foreign_key_kept.
  • Reverting to KEEP only for MERGE INCLUDES, and not for MAKE UNIQUE, matches the stated intent.
• Repo rules
  • Install-All/DarlingData.sql is not touched.
  • No @version or @version_date bumps.
  • No new database objects.
  • The new code follows the CLAUDE.md style: block comments, aliases, 1/0 in EXISTS, and the XML string-building pattern.

Minor notes, not blocking

1. The duplicated include-list STUFF/XML expression now appears in the unwind, the KEEP check and the original Rule 6 build. If the ordering in one ever drifts from the others, the KEEP check will silently stop matching. That would only cause extra MERGE SCRIPTs, not wrong ones. A short "keep in sync with Rule 6" comment would help.
2. The MAKE UNIQUE branch of the winner unwind has no test, as the PR already says. The logic reads correctly, but it is the least-verified path.
3. Tests: assertion 1d now checks for [col_e], and the ix_test_7 assertion is no longer run. Both look consistent with sp_IndexCleanup: MERGE SCRIPT rebuilds an index even when the merge adds no columns #906.

The change looks good to me.
· branch fix/indexcleanup-script-generation

[claude]

Claude finished @erikdarlingdata's task in 10s —— View job

---

Re-review after latest push (89801f0)

The new commit only adds comments to sp_IndexCleanup.sql. I read that diff and did not run any harnesses.

Verdict: no blocking issues.

• Note 1 from my earlier review is addressed. The Rule 6 build and the no-op check before the MERGE SCRIPT now both say the include-list format has to stay in step: QUOTENAME(column_name), joined with ', ', in column_name order.
• The comments are block comments and match the repo style.
• Install-All/DarlingData.sql is not touched.
• There are no @version or @version_date changes.
• There are no new database objects.

Still open, and not blocking: the MAKE UNIQUE branch of the winner unwind has no test. The PR description already says so.
· branch fix/indexcleanup-script-generation