diff --git a/src/backend/commands/typecmds.c b/src/backend/commands/typecmds.c index c4c3cdb5461..e9c3215ccec 100644 --- a/src/backend/commands/typecmds.c +++ b/src/backend/commands/typecmds.c @@ -128,7 +128,7 @@ static Oid findTypeSubscriptingFunction(List *procname, Oid typeOid); static Oid findRangeSubOpclass(List *opcname, Oid subtype); static Oid findRangeCanonicalFunction(List *procname, Oid typeOid); static Oid findRangeSubtypeDiffFunction(List *procname, Oid subtype); -static void validateDomainCheckConstraint(Oid domainoid, const char *ccbin, LOCKMODE lockmode); +static void validateDomainCheckConstraint(Oid domainoid, const char *ccbin); static void validateDomainNotNullConstraint(Oid domainoid); static List *get_rels_with_domain(Oid domainOid, LOCKMODE lockmode); static void checkEnumOwner(HeapTuple tup); @@ -3018,7 +3018,7 @@ AlterDomainAddConstraint(List *names, Node *newConstraint, * to. */ if (!constr->skip_validation) - validateDomainCheckConstraint(domainoid, ccbin, ShareLock); + validateDomainCheckConstraint(domainoid, ccbin); /* * We must send out an sinval message for the domain, to ensure that @@ -3135,12 +3135,7 @@ AlterDomainValidateConstraint(List *names, const char *constrName) val = SysCacheGetAttrNotNull(CONSTROID, tuple, Anum_pg_constraint_conbin); conbin = TextDatumGetCString(val); - /* - * Locking related relations with ShareUpdateExclusiveLock is ok - * because not-yet-valid constraints are still enforced against - * concurrent inserts or updates. - */ - validateDomainCheckConstraint(domainoid, conbin, ShareUpdateExclusiveLock); + validateDomainCheckConstraint(domainoid, conbin); /* * Now update the catalog, while we have the door open. @@ -3235,16 +3230,9 @@ validateDomainNotNullConstraint(Oid domainoid) /* * Verify that all columns currently using the domain satisfy the given check * constraint expression. - * - * It is used to validate existing constraints and to add newly created check - * constraints to a domain. - * - * The lockmode is used for relations using the domain. It should be - * ShareLock when adding a new constraint to domain. It can be - * ShareUpdateExclusiveLock when validating an existing constraint. */ static void -validateDomainCheckConstraint(Oid domainoid, const char *ccbin, LOCKMODE lockmode) +validateDomainCheckConstraint(Oid domainoid, const char *ccbin) { Expr *expr = (Expr *) stringToNode(ccbin); List *rels; @@ -3261,7 +3249,9 @@ validateDomainCheckConstraint(Oid domainoid, const char *ccbin, LOCKMODE lockmod exprstate = ExecPrepareExpr(expr, estate); /* Fetch relation list with attributes based on this domain */ - rels = get_rels_with_domain(domainoid, lockmode); + /* ShareLock is sufficient to prevent concurrent data changes */ + + rels = get_rels_with_domain(domainoid, ShareLock); foreach(rt, rels) { diff --git a/src/test/isolation/expected/alter-domain-validate.out b/src/test/isolation/expected/alter-domain-validate.out new file mode 100644 index 00000000000..483e65c4a81 --- /dev/null +++ b/src/test/isolation/expected/alter-domain-validate.out @@ -0,0 +1,23 @@ +Parsed test spec with 3 sessions + +starting permutation: s1_lock s2_insert s3_add s3_validate s1_unlock s3_validated s3_check +step s1_lock: DO $$ BEGIN PERFORM pg_advisory_lock(8888); END $$; +step s2_insert: WITH wait AS MATERIALIZED (SELECT pg_advisory_lock(8888)) INSERT INTO alter_domain_validate_t SELECT (-1)::alter_domain_validate_d FROM wait; +step s3_add: ALTER DOMAIN alter_domain_validate_d ADD CONSTRAINT alter_domain_validate_d_pos CHECK (VALUE > 0) NOT VALID; +step s3_validate: ALTER DOMAIN alter_domain_validate_d VALIDATE CONSTRAINT alter_domain_validate_d_pos; +step s1_unlock: DO $$ BEGIN PERFORM pg_advisory_unlock(8888); END $$; +step s2_insert: <... completed> +step s3_validate: <... completed> +ERROR: column "a" of table "alter_domain_validate_t" contains values that violate the new constraint +step s3_validated: SELECT convalidated FROM pg_constraint WHERE conname = 'alter_domain_validate_d_pos'; +convalidated +------------ +f +(1 row) + +step s3_check: SELECT count(*) FROM alter_domain_validate_t; +count +----- + 1 +(1 row) + diff --git a/src/test/isolation/isolation_schedule b/src/test/isolation/isolation_schedule index 15c33fad4c5..b8ebe92553c 100644 --- a/src/test/isolation/isolation_schedule +++ b/src/test/isolation/isolation_schedule @@ -90,6 +90,7 @@ test: skip-locked-3 test: skip-locked-4 test: drop-index-concurrently-1 test: multiple-cic +test: alter-domain-validate test: alter-table-1 test: alter-table-2 test: alter-table-3 diff --git a/src/test/isolation/specs/alter-domain-validate.spec b/src/test/isolation/specs/alter-domain-validate.spec new file mode 100644 index 00000000000..daf9d83bfb6 --- /dev/null +++ b/src/test/isolation/specs/alter-domain-validate.spec @@ -0,0 +1,30 @@ +# Test ALTER DOMAIN VALIDATE CONSTRAINT waits for already-running DML. + +setup +{ + CREATE DOMAIN alter_domain_validate_d AS int; + CREATE TABLE alter_domain_validate_t (a alter_domain_validate_d); +} + +teardown +{ + DROP TABLE alter_domain_validate_t; + DROP DOMAIN alter_domain_validate_d; +} + +session s1 +step s1_lock { DO $$ BEGIN PERFORM pg_advisory_lock(8888); END $$; } +step s1_unlock { DO $$ BEGIN PERFORM pg_advisory_unlock(8888); END $$; } + +session s2 +# CoerceToDomain initializes the domain constraint list during executor +# startup, before this CTE waits on the advisory lock. +step s2_insert { WITH wait AS MATERIALIZED (SELECT pg_advisory_lock(8888)) INSERT INTO alter_domain_validate_t SELECT (-1)::alter_domain_validate_d FROM wait; } + +session s3 +step s3_add { ALTER DOMAIN alter_domain_validate_d ADD CONSTRAINT alter_domain_validate_d_pos CHECK (VALUE > 0) NOT VALID; } +step s3_validate { ALTER DOMAIN alter_domain_validate_d VALIDATE CONSTRAINT alter_domain_validate_d_pos; } +step s3_validated { SELECT convalidated FROM pg_constraint WHERE conname = 'alter_domain_validate_d_pos'; } +step s3_check { SELECT count(*) FROM alter_domain_validate_t; } + +permutation s1_lock s2_insert s3_add s3_validate s1_unlock s3_validated s3_check