Re: [PATCH v1] Fix for Bug#19724 - ALTER TYPE ... ALTER ATTRIBUTE triggers internal error for base type of domain with check

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.