If this is not set, these tests are guaranteed to fail, and we add checkpoints between the 4 test cases.

I ran this test 100 times in a loop with and without the session-level advisory lock and it was:

# with locking
Success: 100 Fail: 0

# without locking
Success: 0 Fail: 100

Jotting down the errors we'd expect below. Now, we're not guaranteed to get all of the errors.

unexpected error: failed to run SQL migration: 00001_a.sql: ERROR: duplicate key value violates unique constraint "pg_type_typname_nsp_index" (SQLSTATE 23505)

// or 

unexpected error: failed to run SQL migration: 00001_a.sql: ERROR: type "owner_type" already exists (SQLSTATE 42710)

// or 

unexpected error: failed to run SQL migration: 00010_j.sql: ERROR: deadlock detected (SQLSTATE 40P01)

That last one is interesting because migration 00010_j.sql has -- +goose NO TRANSACTION set and attempts to run:

CREATE UNIQUE INDEX CONCURRENTLY ON owners(owner_name);

This will always raise a deadlock because of the way concurrent indexes are rebuilt in postrges.

This was the script:

#!/bin/bash

SUCCESS=0
FAIL=0
for i in {1..100}
do
    echo "Running $i"
    go test -timeout 30s -run \
        ^TestLockModeAdvisorySession$ github.com/pressly/goose/v4/tests/e2e/postgres \
        -race -count=1 > /dev/null 2>&1
    case $? in
        0) 
            (( SUCCESS++ ))
            ;;
        1) 
            (( FAIL++ ))
            ;;
        *) 
            echo "Unknown"
            exit 1
            ;;
    esac
done

echo "Success: $SUCCESS Fail: $FAIL"

This should be added to the documentation as a gotcha, regardless of how edge case this is, when using session-level advisory locks.

I believe this satisfies the most common use cases raised in #335 #191 #258 by @tomwhipple @jessedearing @dahu33 and others..

(I still, need to clean up the internal abstractions, but tests are passing and things are shaping up).

I'd also like to add support for transaction-level advisory lock, but this will require a bit more thought on how to handle migrations that MUST be run outside a transaction, see comment by @arvenil.

This kind of falls into "Grouped Migrations" territory #485 #222, where we'll (optionally) attempt to run all-or-nothing migrations if possible. This will require a look ahead on the migrations to be applied, so we can group them based on whether it's safe to be run in a tx or not.