sql-server: Record only the key the capture instance uses by peterdukelarsen · Pull Request #40131 · MaterializeInc/materialize

peterdukelarsen · 2026-10-23T21:30:45Z

Motivation

Primary motivation is SS-371, where we can serve incorrect data if a unique constraint has been dropped upstream and we use the recorded unique constraint in planning.

This is also related to SS-466, which is for EXCLUDE CONSTRAINTS syntax. The main goal with supporting exclude constraints is making it possible to safely drop a constraint. I dug the original issue (SS-371) up because I noticed we weren't evaluating constraints during replication.

The idea here is to limit the constraints we pull in to only the constraint fixed in SQL Server's upstream capture instance.

Details

What this does is, during purification of new sources we only retain the constraints that are referenced by the capture instance. SQL Server prevents dropping these unique constraints (except when there's a second capture instance, the old capture instance loses the enforcement).

This will significantly reduce the risk of a customer seeing this in production and effectively get us the benefits from EXCLUDE CONSTRAINTS in terms of flexibility.

The main downside of this is the loss of the optimization. This is not mutually exclusive with actually detecting or partially detecting key drops -- I think we'll ideally add reliable constraint drop detection and reintroduce the current behavior.

A couple more details on behavioral specifics:

A UNIQUE constraint is recorded only when the capture instance was created with @index_name pointing at it. The primary key is then not recorded, since SQL Server lets it be dropped.
A table whose capture instance has no index (no primary key at enable time, no @index_name) records no key.
A UNIQUE constraint over nullable columns is never recorded, because SQL Server refuses it as a CDC index (error 22838).

Verification

Had Claude play around with SQL Server to verify behavior. Local unit tests.

Purification recorded every PRIMARY KEY and UNIQUE constraint of an upstream table as a key of the Materialize relation. SQL Server only protects the index its capture instance uses to identify rows; any other constraint can be dropped at any time, and the drop is not recorded in cdc.ddl_history, so duplicate rows reached a relation that still declared the key and the optimizer planned DISTINCT away. Read cdc.change_tables.index_name alongside the table metadata and keep only the constraint of that name. Co-Authored-By: Claude Fable 5.1 <[email protected]>

martykulma

Nice, thanks @peterdukelarsen! A couple minor comments, nothing blocking.

martykulma · 2026-10-26T17:48:47Z

+    /// table's newest one, so it is the only key the upstream table keeps for
+    /// as long as the source replicates from it. Any other PRIMARY KEY or


partially true? Not as long as the source replication from it, as long as the capture instance MZ runs against is the only one.

martykulma · 2026-10-26T17:54:32Z

+$ sql-server-execute name=sql-server
+EXEC sys.sp_cdc_enable_table @source_schema = 'dbo', @source_name = 'superseded', @role_name = 'SA', @supports_net_changes = 0, @capture_instance = 'superseded_two', @index_name = 'superseded_code';
+ALTER TABLE superseded DROP CONSTRAINT superseded_pk;
+INSERT INTO superseded VALUES (1, 30, 300);


Should this add a verification step before enabling the second capture instance to ensure that the PK drop is in fact blocked?

# While superseded_one is the only capture instance, SQL Server rejects dropping its index ... $ sql-server-set-from-sql name=sql-server var=drop-sole-pk-outcome BEGIN TRY ALTER TABLE superseded DROP CONSTRAINT superseded_pk; SELECT 'dropped'; END TRY BEGIN CATCH SELECT CAST(ERROR_NUMBER() AS VARCHAR(10)); END CATCH > SELECT '${drop-sole-pk-outcome}'; 22979 # ... but a newer capture instance with a different index lifts that protection. ...

Co-Authored-By: Claude Fable 5.1 <[email protected]>

def- · 2026-10-26T19:49:32Z

QA LLM Review

1. MEDIUM -- A nonclustered CDC index can still be disabled with `ALTER INDEX ALL`, so the recorded key goes stale

src/sql-server-util/src/desc.rs:94

The recorded constraint is trusted because SQL Server blocks dropping or disabling the capture instance's index, but ALTER INDEX ALL ON t DISABLE is not blocked and turns off a nonclustered CDC index. SQL Server then accepts duplicates and CDC captures them, while Materialize keeps the key and elides DISTINCT/GROUP BY, which returns the wrong results this PR is meant to prevent. This is not a regression from main, but it affects the @index_name UNIQUE path that the PR now documents as protected, and the default path for PRIMARY KEY NONCLUSTERED tables.

Details

Repro with the unique_not_null shape from the new test (a heap whose unique index is the @index_name constraint):

CREATE TABLE e8 (f1 INT NOT NULL, f2 INT NOT NULL, f3 INT, CONSTRAINT e8_key UNIQUE (f1, f2));
INSERT INTO e8 VALUES (1, 1, NULL);
EXEC sys.sp_cdc_enable_table @source_schema='dbo', @source_name='e8', @role_name=NULL, @supports_net_changes=0, @index_name='e8_key';
ALTER TABLE e8 DROP CONSTRAINT e8_key;  -- Msg 22979
ALTER INDEX e8_key ON e8 DISABLE;       -- Msg 22983
ALTER INDEX ALL ON e8 DISABLE;          -- succeeds
INSERT INTO e8 VALUES (1, 1, 2);        -- succeeds, row lands in cdc.dbo_e8_CT
ALTER INDEX ALL ON e8 REBUILD;          -- Msg 1505 duplicate key, index stays disabled

The same happens with a default capture instance on a heap with PRIMARY KEY NONCLUSTERED. Here index_name is the PK, and a duplicate PK insert is accepted after ALTER INDEX ALL ... DISABLE. If the table has a clustered index, disabling all indexes and then rebuilding only the clustered one leaves a nonclustered CDC index disabled in the same way. cdc.ddl_history records none of these statements, so the replication-side DDL check has nothing to react to.

A clustered CDC index is safe. Disabling it makes the table unwritable (Msg 8655), and rebuilding it re-checks uniqueness. One option is to join sys.indexes on (ch.source_object_id, ch.index_name) in GET_COLUMNS_FOR_TABLES_WITH_CDC_QUERY and keep the constraint only when the index is clustered (type = 1). The join also yields the canonical index name. cdc.change_tables.index_name stores @index_name exactly as typed, so @index_name = 'E1_CODE' for a constraint named e1_Code currently fails the exact-match filter and records no key. Another option is to check sys.indexes.is_disabled for the CDC index during the CDC poll.

…ey (#40201) Closes: [SS-468](https://linear.app/materializeinc/issue/SS-468) Documents the behavior from #40131: a SQL Server table records at most one key, the `PRIMARY KEY` or `UNIQUE` constraint behind the capture instance's index. Replaces the "Dropping a `UNIQUE` constraint puts the table in an error state" row, which did not match the replication path even before that change. Also adds the v26.49.0 release notes for the change: an Improvements bullet, and an Advisory that re-creating a SQL Server table (blue/green deployment, source versioning) can change the plans of dependent objects, for example a `DISTINCT` that used to be removed is now kept. Results do not change. Merge after v26.49 has rolled out. Pages for review: - https://preview.materialize.com/materialize/40201/ingest-data/sql-server/#unique-keys - https://preview.materialize.com/materialize/40201/ingest-data/sql-server/#changing-constraints - https://preview.materialize.com/materialize/40201/ingest-data/sql-server/source-versioning/ - https://preview.materialize.com/materialize/40201/releases/#v26.49-advisory 🤖 Generated with [Claude Code](https://claude.com/claude-code) --------- Co-authored-by: Claude Fable 5.1 <[email protected]>

Dates in RC snapshots are provisional until the release ships. One commit is added per snapshot; the final snapshot sets the real dates. ## ⚠️ Needs the release owner before this can merge **1. This PR conflicts with `main`, and the conflict is substantive.** While this PR sat open, [#40201](#40201) ("docs: describe which SQL Server constraint Materialize records as a key") merged to `main` on 2026-11-05 and independently added its own `## v26.49.0` section — an `### Advisory {#v26.49-advisory}` plus an `### Improvements {#v26.49-improvements}` bullet. Both sides edit the same region of `_index.md`, so `mergeable` is `CONFLICTING`. Two problems ride along with the conflict: - **The same PR is described twice.** [#40201](#40201) documents the behavior of [#40131](#40131), which `rc.1` already published here as a Bug Fix ("Reduced the risk of new SQL Server source tables returning incorrect results after a unique constraint or primary key is dropped upstream…"). *Recommendation: keep `#40201`'s Advisory and Improvement and drop the `rc.1` bug-fix bullet* — the hand-written pair is more complete, carries the upgrade-impact guidance, and links `/ingest-data/sql-server/#unique-keys`. - **Anchor collision.** `main`'s `## v26.49.0` carries `{#v26.49-advisory}` and `{#v26.49-improvements}`; this branch's combined section carries `{#v26.49-improvements}`, `{#v26.49-agent-skills}`, and `{#v26.49-bug-fixes}`. Merging both defines `#v26.49-improvements` twice on one page. *Recommendation: rename this branch's three anchors to `{#v26.49.1-improvements}`, `{#v26.49.1-agent-skills}`, `{#v26.49.1-bug-fixes}`* — which is what the `v26.49.1` snapshot's review notes intended — *and leave `{#v26.49-advisory}` and `{#v26.49-improvements}` to `#40201`'s section.* `#40201`'s review links already point at `/releases/#v26.49-advisory`, and the compatibility YAML's `/releases/#v26491` target is unaffected. **2. Wording drift against the merged `v26.49.1` snapshot.** Commit `39e6b854a` landed here on 2026-11-05, before [MaterializeInc/mz-skills#355](MaterializeInc/mz-skills#355) merged on 2026-11-06. Two entries read differently on the branch than in the merged snapshot — the hydration improvement is "Investigate hydration on the cluster details page" here versus "See how a cluster hydrated, and why it did not" there, and the Freshness and Disk-column bullets differ likewise. Resolving the conflict above is the natural moment to decide which wording ships. ## Note on the section heading The `v26.49.1` patch train reused this branch (commit `39e6b854a`) rather than opening its own, so this PR now publishes a single combined `## v26.49.1` section — this train's `rc.1`–`rc.3` entries plus the three `v26.49.1` entries, with the Cloud version in parentheses. That is the house convention for a patch that ships in its parent's rollout week; compare the published `## v26.48.2` (`*Released to Materialize Cloud: 2026-10-29 (v26.48.0)*`). The compatibility YAML accordingly carries one `v26.49.1` row dated 2026-11-06, which is what actually shipped to Self-Managed that day. The `final` commit for this train is therefore **empty**: its increment carries no entries ([MaterializeInc/release#635](MaterializeInc/release#635) states "No PRs in `v26.49.0` require review", `v26.49.0-rc.3...v26.49.0` is the lone version-bump commit, and the agent-skills window is empty), and the real dates — Cloud **2026-11-05**, Self-Managed **2026-11-06** — were already set by `39e6b854a`. It exists so the branch's commit-subject ledger records that the snapshot was applied. ## Decisions required (carried over from `rc.1`) - **`DROP INDEX`** ([#39098](#39098)): it now rejects drops that used to succeed. Call it out as a breaking change rather than an Improvement? - **Console on large clusters** ([#40306](#40306)): the Cloud Console deploys separately, so this may already be live. Keep it in v26.49? - **Maintained Objects page** ([#40105](#40105)): omitted because it is behind a LaunchDarkly flag. Is it generally available? - **`mz profile init` OAuth** ([#40319](#40319)): omitted because the PR lists open items. Which `mz` release carries it? - **Upsert v2** ([#40189](#40189)): still omitted because `enable_upsert_v2` is `false`. Publish it? ## Snapshot drafts (with PR links) in mz-skills - rc.1: [release_notes.md](https://github.com/MaterializeInc/mz-skills/blob/main/data/mz-release-notes/v26.49.0/rc.1/release_notes.md), [omitted.md](https://github.com/MaterializeInc/mz-skills/blob/main/data/mz-release-notes/v26.49.0/rc.1/omitted.md), [MaterializeInc/mz-skills#351](MaterializeInc/mz-skills#351) - rc.2: [release_notes.md](https://github.com/MaterializeInc/mz-skills/blob/main/data/mz-release-notes/v26.49.0/rc.2/release_notes.md), [omitted.md](https://github.com/MaterializeInc/mz-skills/blob/main/data/mz-release-notes/v26.49.0/rc.2/omitted.md), [MaterializeInc/mz-skills#352](MaterializeInc/mz-skills#352) - rc.3: [release_notes.md](https://github.com/MaterializeInc/mz-skills/blob/main/data/mz-release-notes/v26.49.0/rc.3/release_notes.md), [omitted.md](https://github.com/MaterializeInc/mz-skills/blob/main/data/mz-release-notes/v26.49.0/rc.3/omitted.md), [MaterializeInc/mz-skills#353](MaterializeInc/mz-skills#353) - final: [release_notes.md](https://github.com/MaterializeInc/mz-skills/blob/main/data/mz-release-notes/v26.49.0/final/release_notes.md), [omitted.md](https://github.com/MaterializeInc/mz-skills/blob/main/data/mz-release-notes/v26.49.0/final/omitted.md) **Borderline PRs omitted:** rc.1: 10 — [rc.1/omitted.md#borderline](https://github.com/MaterializeInc/mz-skills/blob/main/data/mz-release-notes/v26.49.0/rc.1/omitted.md#borderline), rc.2: 0, rc.3: 0, final: 0 🤖 Generated with [Claude Code](https://claude.com/claude-code) --------- Co-authored-by: Claude <[email protected]> Co-authored-by: Valerie Chiang <[email protected]> Co-authored-by: Pranshu Maheshwari <[email protected]>

peterdukelarsen marked this pull request as ready for review October 26, 2026 16:49

peterdukelarsen requested a review from a team October 26, 2026 17:04

martykulma approved these changes Oct 26, 2026

View reviewed changes

pr feedback

498dd77

Co-Authored-By: Claude Fable 5.1 <[email protected]>

peterdukelarsen merged commit f20fb37 into MaterializeInc:main Oct 26, 2026
82 checks passed

peterdukelarsen mentioned this pull request Oct 26, 2026

docs: describe which SQL Server constraint Materialize records as a key #40201

Merged

This was referenced Nov 2, 2026

doc: SQL Server tables key only on the capture instance's index #40432

Closed

docs: describe which SQL Server constraint Materialize records as a key #40433

Closed

bosconi mentioned this pull request Nov 6, 2026

v26.49.0 release notes #40469

Merged

Navigation Menu

Repository navigation

sql-server: Record only the key the capture instance uses - #40131

sql-server: Record only the key the capture instance uses#40131
peterdukelarsen merged 2 commits into
MaterializeInc:mainfrom
peterdukelarsen:plarsen/sqlserver-capture-instance-constraints

peterdukelarsen commented Oct 23, 2026 •

edited by github-actions Bot

Loading

Uh oh!

martykulma left a comment

Uh oh!

martykulma Oct 26, 2026

Uh oh!

martykulma Oct 26, 2026

Uh oh!

Uh oh!

def- commented Oct 26, 2026

Uh oh!

Reviewers

Uh oh!

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

		/// table's newest one, so it is the only key the upstream table keeps for
		/// as long as the source replicates from it. Any other PRIMARY KEY or

Conversation

peterdukelarsen commented Oct 23, 2026 • edited by github-actions Bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Motivation

Details

Verification

Uh oh!

martykulma left a comment

Choose a reason for hiding this comment

Uh oh!

martykulma Oct 26, 2026

Choose a reason for hiding this comment

Uh oh!

martykulma Oct 26, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

def- commented Oct 26, 2026

QA LLM Review

1. MEDIUM -- A nonclustered CDC index can still be disabled with ALTER INDEX ALL, so the recorded key goes stale

Uh oh!

Reviewers

Uh oh!

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

peterdukelarsen commented Oct 23, 2026 •

edited by github-actions Bot

Loading

1. MEDIUM -- A nonclustered CDC index can still be disabled with `ALTER INDEX ALL`, so the recorded key goes stale