Subject: support fast default for domain with constraints

From: jian he <[email-masked]>
Date: Wed, 5 Mar 2025 10:56:42 +0800

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
From 37c788dd5399ffdb90373a79f787e062eeda5d80 Mon Sep 17 00:00:00 2001
From: jian he <[email-masked]>
Date: Wed, 5 Mar 2025 10:33:50 +0800
Subject: [PATCH v1 2/2] support fast default for domain with constraints

This is primarily done by evaluating CoerceToDomain with soft error support.

If we evaulte CoerceToDomain to false, it means in ATExecAddColumn,
the defval node evaultion value cannot be cast to the domain.
However, we cannot fail at the Phase 2 stage in cases where the table is empty.

Therefore, if an error occurred while evaultion, do not raise the error,
we signal Phase 3 to do table rewrite, error will be raise on Phase 3.

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 evaulate CoerceToDomain in a soft error way.

[1]: https://git.postgresql.org/cgit/postgresql.git/commit/?id=aaaf9449ec6be62cb0d30ed3588dc384f56274bf
---
 src/backend/commands/tablecmds.c           | 40 ++++++++----
 src/test/regress/expected/fast_default.out | 74 ++++++++++++++++++++++
 src/test/regress/sql/fast_default.sql      | 45 +++++++++++++
 3 files changed, 145 insertions(+), 14 deletions(-)

diff --git a/src/backend/commands/tablecmds.c b/src/backend/commands/tablecmds.c
index 13156391241..8f7b832249b 100644
--- a/src/backend/commands/tablecmds.c
+++ b/src/backend/commands/tablecmds.c
@@ -7331,15 +7331,6 @@ ATExecAddColumn(List **wqueue, AlteredTableInfo *tab, Relation rel,
 	 * NULL if so, so without any modification of the tuple data we will get
 	 * the effect of NULL values in the new column.
 	 *
-	 * An exception occurs when the new column is of a domain type: the domain
-	 * might have a not-null constraint, or a check constraint that indirectly
-	 * rejects nulls.  If there are any domain constraints then we construct
-	 * an explicit NULL default value that will be passed through
-	 * CoerceToDomain processing.  (This is a tad inefficient, since it causes
-	 * rewriting the table which we really wouldn't have to do; but we do it
-	 * to preserve the historical behavior that such a failure will be raised
-	 * only if the table currently contains some rows.)
-	 *
 	 * Note: we use build_column_default, and not just the cooked default
 	 * returned by AddRelationNewConstraints, so that the right thing happens
 	 * when a datatype's default applies.
@@ -7419,13 +7410,16 @@ ATExecAddColumn(List **wqueue, AlteredTableInfo *tab, Relation rel,
 			 * specified DEFAULT value outside of the heap.  This is only
 			 * allowed for plain relations and non-generated columns, and the
 			 * default expression can't be volatile (stable is OK).  Note that
-			 * contain_volatile_functions deems CoerceToDomain immutable, but
-			 * here we consider that coercion to a domain with constraints is
-			 * volatile; else it might fail even when the table is empty.
+			 * contain_volatile_functions deems CoerceToDomain immutable.
+			 * We *do* support soft error of CoerceToDomain if
+			 * ExprState->escontext is not NULL.  we evaulate CoerceToDomain
+			 * (coercion to a domain with optionally constraints), if the error
+			 * happened during evaulation then we need table rewrite. we need
+			 * table rewrite not hard errror because of cases when the table is
+			 * empty.
 			 */
 			if (rel->rd_rel->relkind == RELKIND_RELATION &&
 				!colDef->generated &&
-				!has_domain_constraints &&
 				!contain_volatile_functions((Node *) defval))
 			{
 				EState	   *estate;
@@ -7435,10 +7429,28 @@ ATExecAddColumn(List **wqueue, AlteredTableInfo *tab, Relation rel,
 
 				/* Evaluate the default expression */
 				estate = CreateExecutorState();
-				exprState = ExecPrepareExpr(defval, estate);
+				if (!has_domain_constraints)
+					exprState = ExecPrepareExpr(defval, estate);
+				else
+					exprState = ExecPrepareExprSafe(defval, estate);
+
 				missingval = ExecEvalExpr(exprState,
 										  GetPerTupleExprContext(estate),
 										  &missingIsNull);
+
+				/*
+				 * When has_domain_constraints is true, exprState->escontext
+				 * shoult not NULL.  If evaluating defval (CoerceToDomain Node
+				 * when attribute->atttypid is domain ) fails, it means that the
+				 * default expression cannot be coerced to that domain. This is
+				 * acceptable if the table is empty.  Otherwise, set table
+				 * rewrite and let's error out in Phase 3.
+				*/
+				if (SOFT_ERROR_OCCURRED(exprState->escontext))
+				{
+					missingIsNull = true;
+					tab->rewrite |= AT_REWRITE_DEFAULT_VAL;
+				}
 				/* If it turns out NULL, nothing to do; else store it */
 				if (!missingIsNull)
 				{
diff --git a/src/test/regress/expected/fast_default.out b/src/test/regress/expected/fast_default.out
index ccbcdf8403f..30829a91200 100644
--- a/src/test/regress/expected/fast_default.out
+++ b/src/test/regress/expected/fast_default.out
@@ -317,11 +317,85 @@ SELECT a, b, length(c) = 3 as c_ok, d, e >= 10 as e_ok FROM t2;
  2 | 3 | t    | {This,is,abcd,the,real,world} | t
 (2 rows)
 
+---test fast default over domains with check constraint
+CREATE DOMAIN domain5 AS int check(value > 10);  -- stable
+create domain domain6 as int not null;
+create domain domain7 as int check(value is not null);
+create domain domain8 as int check(value > 10) DEFAULT random(min=>10, max=>100);
+CREATE TABLE t3(a int);
+ALTER TABLE t3 ADD COLUMN b domain5 default 1; --should not fail.
+NOTICE:  rewriting table t3 for reason 2
+INSERT INTO t3 DEFAULT VALUES; --should fail.
+ERROR:  value for domain domain5 violates check constraint "domain5_check"
+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 domain5 default 2; --table rewrite. then fail.
+NOTICE:  rewriting table t3 for reason 2
+ERROR:  value for domain domain5 violates check constraint "domain5_check"
+ALTER TABLE t3 ADD COLUMN b domain6; --table rewrite. then fail.
+NOTICE:  rewriting table t3 for reason 2
+ERROR:  domain domain6 does not allow null values
+ALTER TABLE t3 ADD COLUMN b domain6 default 11 + NULL; --table rewrite. then fail.
+NOTICE:  rewriting table t3 for reason 2
+ERROR:  domain domain6 does not allow null values
+ALTER TABLE t3 ADD COLUMN b domain7; --table rewrite. then fail.
+NOTICE:  rewriting table t3 for reason 2
+ERROR:  value for domain domain7 violates check constraint "domain7_check"
+ALTER TABLE t3 ADD COLUMN b domain7 default 11 + NULL; --table rewrite. then fail.
+NOTICE:  rewriting table t3 for reason 2
+ERROR:  value for domain domain7 violates check constraint "domain7_check"
+ALTER TABLE t3 ADD COLUMN b domain5 default 12; --no table rewrite.
+ALTER TABLE t3 ADD COLUMN c domain6 default 13; --no table rewrite.
+ALTER TABLE t3 ADD COLUMN d domain7 default 14; --no table rewrite.
+--no table rewrite. we consider explicit column default expression, not domain default
+ALTER TABLE t3 ADD COLUMN e domain8 default 15;
+SELECT attnum, attname, atthasmissing, atthasdef, attmissingval
+FROM pg_attribute
+WHERE attnum > 0 AND attrelid = 't3'::regclass AND attisdropped is false
+ORDER BY attnum;
+ attnum | attname | atthasmissing | atthasdef | attmissingval 
+--------+---------+---------------+-----------+---------------
+      1 | a       | f             | f         | 
+      3 | b       | t             | t         | {12}
+      4 | c       | t             | t         | {13}
+      5 | d       | t             | t         | {14}
+      6 | e       | t             | t         | {15}
+(5 rows)
+
+--table rewrite. we are applying domain volatile default expresion
+ALTER TABLE t3 ADD COLUMN f domain8;
+NOTICE:  rewriting table t3 for reason 2
+SELECT attnum, attname, atthasmissing, atthasdef, attmissingval
+FROM pg_attribute
+WHERE attnum > 0 AND attrelid = 't3'::regclass AND attisdropped is false
+ORDER BY attnum;
+ attnum | attname | atthasmissing | atthasdef | attmissingval 
+--------+---------+---------------+-----------+---------------
+      1 | a       | f             | f         | 
+      3 | b       | f             | t         | 
+      4 | c       | f             | t         | 
+      5 | d       | f             | t         | 
+      6 | e       | f             | t         | 
+      7 | f       | f             | f         | 
+(6 rows)
+
+SELECT a,b,c,d,e, f > 10 as f_ok FROM t3 ORDER BY a;
+ a | b  | c  | d  | e  | f_ok 
+---+----+----+----+----+------
+ 1 | 12 | 13 | 14 | 15 | t
+ 2 | 12 | 13 | 14 | 15 | t
+(2 rows)
+
 DROP TABLE t2;
+DROP TABLE t3;
 DROP DOMAIN domain1;
 DROP DOMAIN domain2;
 DROP DOMAIN domain3;
 DROP DOMAIN domain4;
+DROP DOMAIN domain5;
+DROP DOMAIN domain6;
+DROP DOMAIN domain7;
+DROP DOMAIN domain8;
 DROP FUNCTION foo(INT);
 -- Fall back to full rewrite for volatile expressions
 CREATE TABLE T(pk INT NOT NULL PRIMARY KEY);
diff --git a/src/test/regress/sql/fast_default.sql b/src/test/regress/sql/fast_default.sql
index 068dd0bc8aa..5aaa7e7818c 100644
--- a/src/test/regress/sql/fast_default.sql
+++ b/src/test/regress/sql/fast_default.sql
@@ -287,11 +287,56 @@ ORDER BY attnum;
 
 SELECT a, b, length(c) = 3 as c_ok, d, e >= 10 as e_ok FROM t2;
 
+---test fast default over domains with check constraint
+CREATE DOMAIN domain5 AS int check(value > 10);  -- stable
+create domain domain6 as int not null;
+create domain domain7 as int check(value is not null);
+create domain domain8 as int check(value > 10) DEFAULT random(min=>10, max=>100);
+
+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 domain5 default 2; --table rewrite. then fail.
+ALTER TABLE t3 ADD COLUMN b domain6; --table rewrite. then fail.
+ALTER TABLE t3 ADD COLUMN b domain6 default 11 + NULL; --table rewrite. then fail.
+ALTER TABLE t3 ADD COLUMN b domain7; --table rewrite. then fail.
+ALTER TABLE t3 ADD COLUMN b domain7 default 11 + NULL; --table rewrite. then fail.
+
+ALTER TABLE t3 ADD COLUMN b domain5 default 12; --no table rewrite.
+ALTER TABLE t3 ADD COLUMN c domain6 default 13; --no table rewrite.
+ALTER TABLE t3 ADD COLUMN d domain7 default 14; --no table rewrite.
+--no table rewrite. we consider explicit column default expression, not domain default
+ALTER TABLE t3 ADD COLUMN e domain8 default 15;
+
+SELECT attnum, attname, atthasmissing, atthasdef, attmissingval
+FROM pg_attribute
+WHERE attnum > 0 AND attrelid = 't3'::regclass AND attisdropped is false
+ORDER BY attnum;
+
+--table rewrite. we are applying domain volatile default expresion
+ALTER TABLE t3 ADD COLUMN f domain8;
+
+SELECT attnum, attname, atthasmissing, atthasdef, attmissingval
+FROM pg_attribute
+WHERE attnum > 0 AND attrelid = 't3'::regclass AND attisdropped is false
+ORDER BY attnum;
+
+SELECT a,b,c,d,e, f > 10 as f_ok FROM t3 ORDER BY a;
+
 DROP TABLE t2;
+DROP TABLE t3;
 DROP DOMAIN domain1;
 DROP DOMAIN domain2;
 DROP DOMAIN domain3;
 DROP DOMAIN domain4;
+DROP DOMAIN domain5;
+DROP DOMAIN domain6;
+DROP DOMAIN domain7;
+DROP DOMAIN domain8;
 DROP FUNCTION foo(INT);
 
 -- Fall back to full rewrite for volatile expressions
-- 
2.34.1

From 44e49d3c070c3744f75540a7d48af723a49926f0 Mon Sep 17 00:00:00 2001
From: jian he <[email-masked]>
Date: Wed, 5 Mar 2025 10:26:10 +0800
Subject: [PATCH v1 1/2] add soft error variant of ExecPrepareExpr,
 ExecInitExpr

ExecPrepareExprSafe and ExecInitExprSafe.
ExecPrepareExprSafe initialize for expression execution with soft error support.
not all expression node support it. some like CoerceToDomain do support it.

XXX more comments.
---
 src/backend/executor/execExpr.c | 63 +++++++++++++++++++++++++++++++++
 src/include/executor/executor.h |  2 ++
 2 files changed, 65 insertions(+)

diff --git a/src/backend/executor/execExpr.c b/src/backend/executor/execExpr.c
index 03566c4d181..04f8b839d30 100644
--- a/src/backend/executor/execExpr.c
+++ b/src/backend/executor/execExpr.c
@@ -170,6 +170,46 @@ ExecInitExpr(Expr *node, PlanState *parent)
 	return state;
 }
 
+/*
+ * ExecInitExpr: soft error variant of ExecInitExpr.
+ * use it only for expression nodes support soft errors, not all expression
+ * nodes support it.
+*/
+ExprState *
+ExecInitExprSafe(Expr *node, PlanState *parent)
+{
+	ExprState  *state;
+	ExprEvalStep scratch = {0};
+
+	/* Special case: NULL expression produces a NULL ExprState pointer */
+	if (node == NULL)
+		return NULL;
+
+	/* Initialize ExprState with empty step list */
+	state = makeNode(ExprState);
+	state->expr = node;
+	state->parent = parent;
+	state->ext_params = NULL;
+	state->escontext = makeNode(ErrorSaveContext);
+	state->escontext->type = T_ErrorSaveContext;
+	state->escontext->error_occurred = false;
+	state->escontext->details_wanted = true;
+
+	/* Insert setup steps as needed */
+	ExecCreateExprSetupSteps(state, (Node *) node);
+
+	/* Compile the expression proper */
+	ExecInitExprRec(node, state, &state->resvalue, &state->resnull);
+
+	/* Finally, append a DONE step */
+	scratch.opcode = EEOP_DONE;
+	ExprEvalPushStep(state, &scratch);
+
+	ExecReadyExpr(state);
+
+	return state;
+}
+
 /*
  * ExecInitExprWithParams: prepare a standalone expression tree for execution
  *
@@ -778,6 +818,29 @@ ExecPrepareExpr(Expr *node, EState *estate)
 	return result;
 }
 
+/*
+ * ExecPrepareExprSafe: soft error variant of ExecPrepareExpr.
+ *
+ * use it when expression node *support* soft error expression execution.
+ * ExecPrepareExpr comments apply to here too.
+ */
+ExprState *
+ExecPrepareExprSafe(Expr *node, EState *estate)
+{
+	ExprState  *result;
+	MemoryContext oldcontext;
+
+	oldcontext = MemoryContextSwitchTo(estate->es_query_cxt);
+
+	node = expression_planner(node);
+
+	result = ExecInitExprSafe(node, NULL);
+
+	MemoryContextSwitchTo(oldcontext);
+
+	return result;
+}
+
 /*
  * ExecPrepareQual --- initialize for qual execution outside a normal
  * Plan tree context.
diff --git a/src/include/executor/executor.h b/src/include/executor/executor.h
index d12e3f451d2..b7ab95437fe 100644
--- a/src/include/executor/executor.h
+++ b/src/include/executor/executor.h
@@ -305,6 +305,7 @@ ExecProcNode(PlanState *node)
  * prototypes from functions in execExpr.c
  */
 extern ExprState *ExecInitExpr(Expr *node, PlanState *parent);
+extern ExprState *ExecInitExprSafe(Expr *node, PlanState *parent);
 extern ExprState *ExecInitExprWithParams(Expr *node, ParamListInfo ext_params);
 extern ExprState *ExecInitQual(List *qual, PlanState *parent);
 extern ExprState *ExecInitCheck(List *qual, PlanState *parent);
@@ -353,6 +354,7 @@ extern ProjectionInfo *ExecBuildUpdateProjection(List *targetList,
 												 TupleTableSlot *slot,
 												 PlanState *parent);
 extern ExprState *ExecPrepareExpr(Expr *node, EState *estate);
+extern ExprState *ExecPrepareExprSafe(Expr *node, EState *estate);
 extern ExprState *ExecPrepareQual(List *qual, EState *estate);
 extern ExprState *ExecPrepareCheck(List *qual, EState *estate);
 extern List *ExecPrepareExprList(List *nodes, EState *estate);
-- 
2.34.1

---

From: Tom Lane <[email-masked]>
Date: Tue, 04 Mar 2025 22:13:40 -0500

> 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