Thread: bug when apply fast default mechanism for adding new column over domain with default value

hi.

While looking at ATExecAddColumn, I saw there are two DomainHasConstraints,
which are cache unfriendly,
then I think I found a bug for applying fast default over domain with
default value.


bug demo:
create domain int_domain3 as int check(value > 1) default 11;
create domain int_domain4 as int default 11;
drop table if exists t0;
create table t0(a int);
insert into t0 values(1),(2);
alter table t0 add column b int_domain3;

select * from t0;
 a | b
---+----
 1 | 11
 2 | 11
(2 rows)

alter table t0 add column c int_domain4;
select * from t0;
a | b  | c
---+----+---
 1 | 11 |
 2 | 11 |
(2 rows)

I expect column "c" value also be value 11.


column b type is domain int_domain3, which has a constraint.
As a result, adding column b triggers a table rewrite, ensuring the
domain default value is successfully applied.

column c is domain int_domain4, which has no constraint.
This allows for a fast default mechanism applied to column c, but we cannot.
StoreAttrDefault, pg_attrdef can not cope with domain with default expression.
also domain default expression is stored at pg_tye.typdefaultbin.


Attach a patch to fix this issue by cause it to table rewrite.
also minor refactor to avoid double DomainHasConstraints function call.

jian he <jian.universality@gmail.com> writes:
> then I think I found a bug for applying fast default over domain with
> default value.

Yeah, that's definitely a bug: it worked correctly in versions
before we invented the attmissingval mechanism.

> column b type is domain int_domain3, which has a constraint.
> As a result, adding column b triggers a table rewrite, ensuring the
> domain default value is successfully applied.

I do not believe that case should require a table rewrite.
Both the default and the check constraint are immutable,
so we ought to be able to apply the check once and then
use the default as the attmissingval.

> Attach a patch to fix this issue by cause it to table rewrite.

I see no attached patch, but in any case forcing a table rewrite
seems like the wrong direction...

            regards, tom lane