Skip to content

Support DROP INDEX CONCURRENTLY - #3902

Open
eypcnckr wants to merge 1 commit into
npgsql:mainfrom
eypcnckr:feat/drop-index-concurrently
Open

Support DROP INDEX CONCURRENTLY#3902
eypcnckr wants to merge 1 commit into
npgsql:mainfrom
eypcnckr:feat/drop-index-concurrently

Conversation

@eypcnckr

@eypcnckr eypcnckr commented Sep 1, 2026

Copy link
Copy Markdown

Closes #3901

What

CREATE INDEX CONCURRENTLY has been supported since #968, but the matching drop
was not - Generate(DropIndexOperation, ...) always emitted a plain DROP INDEX
inside the migration transaction. Removing an index created with
IsCreatedConcurrently() therefore takes an ACCESS EXCLUSIVE lock on the table.

Two things were missing, and both are needed for the feature to work end to end:

  • NpgsqlMigrationsAnnotationProvider did not override ForRemove(ITableIndex), so
    Npgsql:CreatedConcurrently never reached the DropIndexOperation produced by the
    differ - the operation had no annotations at all.
  • Generate(DropIndexOperation, ...) did not look at the annotation.

This PR honours the annotation on both sides and suppresses the transaction for the
drop, mirroring Generate(CreateIndexOperation, ...).

Design note

I reused the existing Npgsql:CreatedConcurrently annotation instead of introducing
a new one, so IsCreatedConcurrently() now means "created and dropped
concurrently". No new public API, and I think it matches the intent - but it is a
behaviour change for existing users: dropping such an index now runs outside the
migration transaction. If you would rather have an explicit IsDroppedConcurrently(),
say the word and I will rework it; the generator half stays the same either way.

Testing

  • DropIndexOperation_concurrently / DropIndexOperation_not_concurrently in
    NpgsqlMigrationsSqlGeneratorTest cover the generated SQL.
  • Drop_index_concurrently in MigrationsNpgsqlTest covers the full model-diff
    path, mirroring the existing Create_index_concurrently.

What I could and could not run locally: main targets net11.0 and pins the
.NET 11 preview SDK in global.json, which I do not have installed, so I could not
build main or run its test suite. To verify the change I:

  1. Applied the generator change to a hotfix/10.0.4 worktree (identical code there,
    targets net10.0), added the two NpgsqlMigrationsSqlGeneratorTest tests and ran
    them - both pass.
  2. Verified the ForRemove(ITableIndex) half separately against the shipped 10.0.0
    package by registering an IMigrationsAnnotationProvider with exactly this
    override and running the migrations differ: the DropIndexOperation goes from
    having no annotations to carrying Npgsql:CreatedConcurrently = True.

So both halves are verified, but not together on main - the new
Drop_index_concurrently functional test in particular has not been executed. Please
let CI confirm, and I am happy to fix anything it turns up.

@eypcnckr
eypcnckr force-pushed the feat/drop-index-concurrently branch from 4b2bbcd to 2b190ed Compare September 1, 2026 11:06

@roji roji left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, makes sense for me to reuse IsCreateConcurrently for deletion as well rather than introduce yet another API point - the same reasons that lead to wanting an index to be created concurrently like lead to the same thing for drop.

CREATE INDEX CONCURRENTLY has been supported since npgsql#968, but the matching
drop was not: Generate(DropIndexOperation) always emitted a plain
DROP INDEX inside the migration transaction, so removing an index created
with IsCreatedConcurrently() takes an ACCESS EXCLUSIVE lock on the table.

Two things were missing:

- NpgsqlMigrationsAnnotationProvider did not override ForRemove(ITableIndex),
  so the Npgsql:CreatedConcurrently annotation never reached the
  DropIndexOperation produced by the migrations differ.
- Generate(DropIndexOperation) did not look at that annotation.

Honour the annotation on both sides, and suppress the transaction for the
drop, mirroring what Generate(CreateIndexOperation) already does.
@roji
roji force-pushed the feat/drop-index-concurrently branch from 2b190ed to 2c5cb9f Compare September 2, 2026 13:16
@roji
roji enabled auto-merge (squash) September 2, 2026 13:17
@eypcnckr

eypcnckr commented Sep 4, 2026

Copy link
Copy Markdown
Author

@roji hi, CI failed, but the only failure is build (windows-2022, 17, Release), and it's in the Start PostgreSQL 17 (Windows) step — the job exits after ~1.5 min, before the SDK setup and build steps run. All Linux jobs (PG 13–18, Release + Debug) passed, so the new tests do pass.

Looks unrelated to the PR — could you re-run that one job when you get a chance?

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

DROP INDEX CONCURRENTLY is not supported

2 participants