Hello

+       /*
+        * Check it's a domain and check user has permission for ALTER DOMAIN.
+        * When re-adding a constraint during ALTER TABLE, skip the permission
+        * check since the constraint already existed, and the user altering a
+        * column it depends on need not own the domain.
+        */
+       if (is_readd)
+               Assert(typTup->typtype == TYPTYPE_DOMAIN);
+       else
+               checkDomainOwner(tup);

That assertion can fire with two concurrent sessions, it should be a
proper error message, similar to what's inside checkDomainOwner.

See the following isolation test:

setup
{
  CREATE TYPE ct AS (i int);
  CREATE DOMAIN d AS ct CONSTRAINT d_check CHECK ((VALUE).i > 0);
  CREATE TABLE t2 (x int CONSTRAINT t2_check CHECK ((row(x)::ct).i > 0));
  INSERT INTO t2 VALUES (1);
}

teardown
{
  DROP TABLE IF EXISTS t2;
  DROP TYPE IF EXISTS d CASCADE;
  DROP DOMAIN IF EXISTS d_old CASCADE;
  DROP TYPE IF EXISTS ct CASCADE;
}

session s1
step a_alter    { ALTER TYPE ct ALTER ATTRIBUTE i TYPE bigint; }

session s2
step b_begin    { BEGIN; SELECT count(*) FROM t2; }
step b_commit   { COMMIT; }

session s3
step c_swap     { ALTER DOMAIN d RENAME TO d_old; CREATE TYPE d AS (z int); }

permutation b_begin a_alter c_swap b_commit


+                               if (!con->skip_validation)
+                                       tab->domain_constraints =
+                                               
lappend_oid(tab->domain_constraints,
+                                                                       
constrAddr.objectId);

This can be uninitialized, AlterDomainAddConstraint doesn't guarantee
a write. I think this could use both a Assert(con->contype ==
CONSTR_CHECK); and initalizating constrAddr to InvalidObjectAddress.


Reply via email to