Skip to content

feat(postgres): add sqlx-advisory-locking connection string parameter honored in #[sqlx::test]#4180

Open
mpecan wants to merge 3 commits intolaunchbadge:mainfrom
mpecan:feat/sqlx-test-locking
Open

feat(postgres): add sqlx-advisory-locking connection string parameter honored in #[sqlx::test]#4180
mpecan wants to merge 3 commits intolaunchbadge:mainfrom
mpecan:feat/sqlx-test-locking

Conversation

@mpecan
Copy link

@mpecan mpecan commented Feb 25, 2026

Adds a sqlx-advisory-locking connection string parameter to PgConnectOptions that controls whether #[sqlx::test] acquires advisory locks during test database setup and migrations.

Since #3753 switched test setup to pg_advisory_xact_lock, databases that speak the PostgreSQL wire protocol but don't implement advisory locks (e.g. CockroachDB, see cockroachdb/cockroach#13546) cannot use #[sqlx::test] at all.

Design change

The original version of this PR added a locking = false attribute to the #[sqlx::test] macro. Based on maintainer feedback, the approach was redesigned to use a connection string parameter instead. This is a better fit because:

  • Configuration lives where the database identity is declared (DATABASE_URL)
  • Users set it once rather than annotating every test function
  • The setting is inherently database-specific, not test-specific
  • Follows the existing statement-cache-capacity precedent for sqlx-specific URL parameters

Named generically (sqlx-advisory-locking) rather than test-specific, so Migrator can respect it in the future if desired — but current scope is test setup only.

Usage

Set in DATABASE_URL — no per-test annotation needed:

DATABASE_URL="postgres://localhost/mydb?sqlx-advisory-locking=false"

All #[sqlx::test] tests automatically respect the setting: both test database schema setup and migrations skip advisory locks.

Does your PR solve an issue?

Partially addresses #198 (CockroachDB support).

Is this a breaking change?

No. Default behavior is unchanged (advisory_locking = true). Advisory locks are only skipped when explicitly opted out via the connection string.

@mpecan mpecan force-pushed the feat/sqlx-test-locking branch from 014bb58 to 3cf761a Compare February 25, 2026 18:28
@mpecan mpecan marked this pull request as draft February 25, 2026 19:40
@mpecan mpecan force-pushed the feat/sqlx-test-locking branch from 3cf761a to 501cd1a Compare February 25, 2026 19:41
Add a `locking` parameter to `#[sqlx::test]` that controls whether an
advisory lock is acquired before creating the test database schema.
Defaults to `true` to preserve existing PostgreSQL behavior.

Setting `locking = false` allows `#[sqlx::test]` to work with databases
that speak the PostgreSQL wire protocol but do not implement advisory
locks, such as CockroachDB. When disabled, migrator locking is also
skipped so the entire test setup is advisory-lock-free.

Follows the same pattern as `Migrator::set_locking()` (PR launchbadge#2063).
@mpecan mpecan force-pushed the feat/sqlx-test-locking branch from 501cd1a to 82f2933 Compare February 25, 2026 19:45
@mpecan mpecan marked this pull request as ready for review February 25, 2026 19:47
@abonander
Copy link
Collaborator

In all likelihood, you would be setting this on all invocations, right? So this probably makes more sense to live in the sqlx.toml file as something like:

[macros.test]
locking = false

But arguably, this is more a property of the database connection itself, so maybe something to put in the DATABASE_URL instead?

@abonander
Copy link
Collaborator

Similar to #4152

@mpecan
Copy link
Author

mpecan commented Feb 28, 2026

@abonander that is a much better idea, it also enables the code to be DB agnostic, instead having the connection string define behaviour (so we can run the "same" code, from the developers perspective on both DBs).

If I understand correctly, would we want to implement the same for the migrator (i.e. Migrator::set_locking populating automatically based on the connection string?), this would probably be a different PR, but I would be happy to build that as well.

Would need a decision on which takes precedence (I would expect calling Migrator::set_locking would override anything from the DB connection string, or should the environment definition override the code, hard decision, needs to be well documented).

Replace the `#[sqlx::test(locking = false)]` macro attribute with a
connection string parameter `?sqlx-advisory-locking=false`. This is a
better design because:

- Configuration lives where the database identity is declared
- Users set it once in DATABASE_URL rather than annotating every test
- Follows the existing `statement-cache-capacity` precedent
- Named generically to leave the door open for Migrator support

When `sqlx-advisory-locking=false` is set, both the test schema setup
and migrations skip advisory locks, allowing `#[sqlx::test]` to work
with CockroachDB and similar databases.
@mpecan mpecan changed the title feat(postgres): add locking attribute to #[sqlx::test] feat(postgres): add sqlx-advisory-locking connection string parameter Feb 28, 2026
@mpecan
Copy link
Author

mpecan commented Feb 28, 2026

@abonander implemented the approach you suggested.

I don't expect we want to start a CockroachDB integration test, any other suggestions as to how I can improve the test coverage of this case? I tried to do it in a way where I don't break any tests, but I cannot test against PostgreSQL since that is where we do need the advisory locks.

@mpecan mpecan changed the title feat(postgres): add sqlx-advisory-locking connection string parameter feat(postgres): add sqlx-advisory-locking connection string parameter honored in #[sqlx::test] Feb 28, 2026
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.

2 participants