Обсуждение: support fast default for domain with constraints

hi.

Thanks to commit aaaf9449ec6be62cb0d30ed3588dc384f56274bf[1],
ExprState.escontext (ErrorSaveContext) was added, and ExecEvalConstraintNotNull,
ExecEvalConstraintCheck were changed to use errsave instead of hard error.
Now we can use it to evaluate CoerceToDomain in a soft error way, that
is what this patch intended to do.
previously ExprState.escontext was mainly used in SQL/JSON related patches.


To achieve that, we have to populate ExprState.escontext before
passing it to ExecInitExprRec.
So I created two functions: ExecInitExprSafe, ExecPrepareExprSafe.
ExecPrepareExprSafe is an error safe variant of ExecPrepareExpr.
within ExecPrepareExprSafe, we use ExecInitExprSafe.
ExecInitExprSafe differs from ExecInitExpr is that the output
ExprState has its escontext set to a valid ErrorSaveContext.


demo:
CREATE DOMAIN domain5 AS int check(value > 10);  -- stable
create domain domain6 as int not null;

CREATE TABLE t3(a int);
ALTER TABLE t3 ADD COLUMN b domain5 default 1; --should not fail.
INSERT INTO t3 DEFAULT VALUES; --should fail.
ALTER TABLE t3 DROP COLUMN b; --need drop it for the following tests
INSERT INTO t3 VALUES(1),(2);

ALTER TABLE t3 ADD COLUMN b domain6; --table rewrite. then fail.
ALTER TABLE t3 ADD COLUMN c domain6 default 13; --no table rewrite.
fast default applied. attmissingval is stored.


[1] https://git.postgresql.org/cgit/postgresql.git/commit/?id=aaaf9449ec6be62cb0d30ed3588dc384f56274bf

jian he <jian.universality@gmail.com> writes:
> Thanks to commit aaaf9449ec6be62cb0d30ed3588dc384f56274bf[1],
> ExprState.escontext (ErrorSaveContext) was added, and ExecEvalConstraintNotNull,
> ExecEvalConstraintCheck were changed to use errsave instead of hard error.
> Now we can use it to evaluate CoerceToDomain in a soft error way, that
> is what this patch intended to do.

This patch appears to summarily throw away a couple of
backwards-compatibility concerns that the previous round
took care to preserve:

* not throwing an error if the default would fail the domain
constraints, but the table is empty so there is no need to
instantiate the default.

* not assuming that the domain constraints are immutable.

Now it's fair to question how important the second point is
considering that we mostly treat domain constraints as immutable
elsewhere.  But I think the first point has actual practical uses
--- for example, if you want to set things up so that inserts must
specify that column explicitly.  So I don't think it's okay to
discard that behavior.

Maybe we need a regression test case demonstrating that that
behavior exists, to discourage people from breaking it ...

            regards, tom lane

On Wed, Mar 5, 2025 at 11:13 AM Tom Lane <tgl@sss.pgh.pa.us> wrote:
>
> This patch appears to summarily throw away a couple of
> backwards-compatibility concerns that the previous round
> took care to preserve:
>
> * not throwing an error if the default would fail the domain
> constraints, but the table is empty so there is no need to
> instantiate the default.
>
hi. Thanks for pointing this out.
I noticed an empty table scarenio, but didn't check it thoroughly.
The attached patch preserves this backwards-compatibility.
now it's aligned with master behavior, i think.

main gotcha is:
ALTER TABLE ADD COLUMN...
If no explicitly DEFAULT, the defval either comes from pg_type.typdefaultbin,
or constructed via makeNullConst branch.
In that case, we need to use soft error evaluation, because we allow
these cases for an empty table;
In other cases, we can directly evaluate explicitly the DEFAULT clause.


> * not assuming that the domain constraints are immutable.
>
> Now it's fair to question how important the second point is
> considering that we mostly treat domain constraints as immutable
> elsewhere.  But I think the first point has actual practical uses
> --- for example, if you want to set things up so that inserts must
> specify that column explicitly.  So I don't think it's okay to
> discard that behavior.
>

in v2-0003. I created a new function:
bool DomainHaveVolatileConstraints(Oid type_id, bool *have_volatile)
within DomainHaveVolatileConstraints
i use contain_volatile_functions to test whether check_expr is volatile or not.
contain_volatile_functions won't be expensive, i think.

if true then have_volatile is set to true.
if have_volatile is true then we need table rewrite.

hi.

rearrange the patch.
v3-0001 and v3-0002 is preparare patches.
v3-0001 add function: ExecPrepareExprSafe and ExecInitExprSafe.
v3-0002 add function: DomainHaveVolatileConstraints


v3-0003 tests and apply fast default for domain with constraints.
v3-0003 table with empty rows aligned with master behavior.
also no table rewrite if the domain has volatile check constraints,
so less surprising behavior.

On Thu, Mar 6, 2025 at 11:04 AM jian he <jian.universality@gmail.com> wrote:
>
> hi.
>
> rearrange the patch.
> v3-0001 and v3-0002 is preparare patches.
> v3-0001 add function: ExecPrepareExprSafe and ExecInitExprSafe.
> v3-0002 add function: DomainHaveVolatileConstraints
>

i actually do need DomainHaveVolatileConstraints
for virtual generated columns over domain with constraints in [1],
which I am working on.

for example:
create domain d1 as int check(value > random(min=>11::int, max=>12));
create domain d2 as int check(value > 12);
create table t(a int);
insert into t select g from generate_series(1, 10) g;

----we do need table rewrite in phase 3.
alter table t add column b d1 generated always as (a+11) virtual;

--we can only do table scan in phase 3.
alter table t add column c d2 generated always as (a + 12) virtual;

Generally, table rewrite is more expensive than table scan.
In the above case, if domain constraints are not volatile, table scan
should be fine.

[1]: https://postgr.es/m/CACJufxHArQysbDkWFmvK+D1TPHQWWTxWN15cMuUaTYX3xhQXgg@mail.gmail.com

hi.

rebase because of commit: 8dd7c7cd0a2605d5301266a6b67a569d6a305106
also did minor enhancement.

v4-0001 add function: ExecPrepareExprSafe and ExecInitExprSafe.
v4-0002 add function: DomainHaveVolatileConstraints
v4-0003 tests and apply fast default for domain with constraints.

v4-0003 table with empty rows aligned with master behavior.
also will do table rewrite if the new column is domain with volatile
check constraints,
so less surprising behavior.

On Mon, Mar 24, 2025 at 7:14 PM jian he <jian.universality@gmail.com> wrote:
>
> v4-0003 table with empty rows aligned with master behavior.
> also will do table rewrite if the new column is domain with volatile
> check constraints,
> so less surprising behavior.

I found out that my v4-0003 is wrong.

For example, the following ALTER TABLE ADD COLUMN should not fail.
CREATE DOMAIN domain5 AS int check(value > 10) default 8;
CREATE TABLE t3(a int);
ALTER TABLE t3 ADD COLUMN b domain5 default 1; --ok, table rewrite

I also reduced the bloated tests.
summary of the behavior that is different from master:
if domain constraint is not volatile *and* domain's default expression satisfy
constraint's condition then no need table rewrite.

hi.

in previous patches v6-0001 to v6-0003, we added support for ALTER TABLE ADD
COLUMN with fast defaults for domains having non-volatile constraints.

inspired by another patch of mine: https://commitfest.postgresql.org/patch/5907
I believe it's doable to perform only a table scan when using ALTER TABLE ADD
COLUMN with a domain that has volatile constraints.

some example:
CREATE DOMAIN domain8 as int check((value + random(min=>11::int,
max=>11)) > 12);
CREATE TABLE t3(a int);
INSERT INTO t3 VALUES(1),(2);
ALTER TABLE t3 ADD COLUMN f domain8 default 1; --error while coercing to domain
ALTER TABLE t3 ADD COLUMN f domain8 default 20; --ok

The idea is the same as mentioned in [1],
for struct NewColumnValue, add another field (scan_only) to indicate
that we use table scan to evaluate the CoerceToDomain node.

summary of the attached v7.
v7-0001, v7-00002: preparatory patch.
v7-0003 adds fast default support for ALTER TABLE ADD COLUMN when the domain has
non-volatile constraints.
A table rewrite is still required for domains with volatile constraints.

v7-0004 skip table rewrite (table scan only) for ALTER TABLE ADD
COLUMN with domains has volatile constraints.

[1] https://postgr.es/m/CACJufxFhWyWzf2sJS9txSKeyA8hstxGDb8q2QWWwbo5Q1smPMA@mail.gmail.com

On Mon, Sep 1, 2025 at 2:27 PM jian he <jian.universality@gmail.com> wrote:
>
> summary of the attached v7.
> v7-0001, v7-00002: preparatory patch.
> v7-0003 adds fast default support for ALTER TABLE ADD COLUMN when the domain has
> non-volatile constraints.
> A table rewrite is still required for domains with volatile constraints.
>
> v7-0004 skip table rewrite (table scan only) for ALTER TABLE ADD
> COLUMN with domains has volatile constraints.
>

Hi.

rebase, and further simplified.

maybe we could perform a table scan for ALTER TABLE ADD COLUMN when the domain
has volatile constraints like v7-0004, avoiding a table rewrite.
However, this approach
feels inelegant, so I do not plan to pursue it.

So, the fast default now applies to domains with non-volatile constraint
expressions only.

Regarding the prior discussion about empty table behavior. This patch is
consistent with the master: not throwing an error if the default would fail the
domain constraints.



--
jian
https://www.enterprisedb.com/

On 2026-01-26 Mo 2:52 AM, jian he wrote:
> On Mon, Sep 1, 2025 at 2:27 PM jian he <jian.universality@gmail.com> wrote:
>> summary of the attached v7.
>> v7-0001, v7-00002: preparatory patch.
>> v7-0003 adds fast default support for ALTER TABLE ADD COLUMN when the domain has
>> non-volatile constraints.
>> A table rewrite is still required for domains with volatile constraints.
>>
>> v7-0004 skip table rewrite (table scan only) for ALTER TABLE ADD
>> COLUMN with domains has volatile constraints.
>>
> Hi.
>
> rebase, and further simplified.
>
> maybe we could perform a table scan for ALTER TABLE ADD COLUMN when the domain
> has volatile constraints like v7-0004, avoiding a table rewrite.
> However, this approach
> feels inelegant, so I do not plan to pursue it.
>
> So, the fast default now applies to domains with non-volatile constraint
> expressions only.
>
> Regarding the prior discussion about empty table behavior. This patch is
> consistent with the master: not throwing an error if the default would fail the
> domain constraints.
>
>
>


here's an updated patch set.


main changes:


. renamed DomainHaveVolatileConstraints renamed to 
DomainHasVolatileConstraints, improved the comments and code structure

. squashed two commits into one, as there's only one user for the 
soft-error functions

. rename  ExecPrepareExprExtended to ExecPrepareExprWithContext and 
ExecInitExprExtended to ExecInitExprWithContext, with improved comments.


cheers


andrew

--
Andrew Dunstan
EDB: https://www.enterprisedb.com

On Wed, Mar 11, 2026 at 1:18 AM Andrew Dunstan <andrew@dunslane.net> wrote:
>
> here's an updated patch set.
>

+/*
+ * ExecPrepareExprWithContext: same as ExecPrepareExpr, but with an optional
+ * ErrorSaveContext for soft error handling during domain constraint
evaluation.
+ *
+ * See ExecInitExprWithContext for details on the escontext parameter.
+ */
+ExprState *
+ExecPrepareExprWithContext(Expr *node, EState *estate, Node *escontext)

Since ExecPrepareExprWithContext can be used more broadly, we should delete the
mention of domain constraint from the above comments.

I checked alter_table.sgml again, no need to change it, I think.

Slightly changed the regression test comments.



--
jian
https://www.enterprisedb.com/

I think it's cleaner just to modify the existing function with an extra parameter, which the existing callers will pass as NULL.

Would be nice to test domains with both volatile and non-volatile checks.
Also, perhaps virtual generated columns could use a test?

Also added some tests.

cheers

andrew

--
Andrew Dunstan
EDB: https://www.enterprisedb.com

On Thu, Mar 12, 2026 at 3:50 AM Andrew Dunstan <andrew@dunslane.net> wrote:
> Also added some tests.

V11 looks good to me.


On Wed, Mar 11, 2026 at 6:34 PM Viktor Holmberg <v@viktorh.net> wrote:
>
> I’ve been burned my this issue in the past so would be great if this could get in.
>
> + /*
> + * If the domain has volatile constraints, we must do a table rewrite
> + * since the constraint result could differ per row and cannot be
> + * evaluated once and cached as a missing value.
> + */
> + if (has_volatile)
> + {
> + Assert(has_domain_constraints);
> + tab->rewrite |= AT_REWRITE_DEFAULT_VAL;
> + }
>
> I'm not sure. But seems to me this makes the pre-existing guard for virtual columns
> redundant?
> I mean this code on line 7633:
> if (colDef->generated != ATTRIBUTE_GENERATED_VIRTUAL)
>  tab->rewrite |= AT_REWRITE_DEFAULT_VAL;
>

If the first `if (has_volatile)` is false, then
> if (colDef->generated != ATTRIBUTE_GENERATED_VIRTUAL)
>  tab->rewrite |= AT_REWRITE_DEFAULT_VAL;
is still reachable.

> Also, perhaps virtual generated columns could use a test?

Virtual generated columns based on domain are not currently supported.
I have a patch for it: https://commitfest.postgresql.org/patch/5725
but it's not doable now because of
https://git.postgresql.org/cgit/postgresql.git/commit/?id=0cd69b3d7ef357f2b43258de5831c4de0bd51dec
You may also be interested in https://commitfest.postgresql.org/patch/5907



--
jian
https://www.enterprisedb.com/