diff --git a/src/backend/commands/tablecmds.c b/src/backend/commands/tablecmds.c index 54d93d0a0de..cd6d720386f 100644 --- a/src/backend/commands/tablecmds.c +++ b/src/backend/commands/tablecmds.c @@ -7486,15 +7486,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. @@ -7513,6 +7504,7 @@ ATExecAddColumn(List **wqueue, AlteredTableInfo *tab, Relation rel, { bool has_domain_constraints; bool has_missing = false; + bool has_volatile = false; /* * For an identity column, we can't use build_column_default(), @@ -7530,8 +7522,18 @@ ATExecAddColumn(List **wqueue, AlteredTableInfo *tab, Relation rel, else defval = (Expr *) build_column_default(rel, attribute->attnum); + has_domain_constraints = + DomainHasConstraints(attribute->atttypid, &has_volatile); + + /* + * 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) + tab->rewrite |= AT_REWRITE_DEFAULT_VAL; + /* Build CoerceToDomain(NULL) expression if needed */ - has_domain_constraints = DomainHasConstraints(attribute->atttypid, NULL); if (!defval && has_domain_constraints) { Oid baseTypeId; @@ -7573,27 +7575,50 @@ ATExecAddColumn(List **wqueue, AlteredTableInfo *tab, Relation rel, * Attempt to skip a complete table rewrite by storing the * 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. + * default expression can't be volatile (stable is OK), and the + * domain constraint expressions can't be volatile (stable is OK). + * + * Note that contain_volatile_functions considers CoerceToDomain + * immutable, so we rely on DomainHasConstraints (called above) + * rather than checking defval alone. + * + * For domains with non-volatile constraints, we evaluate the + * default using soft error handling: if the constraint check + * fails (e.g., CHECK(value > 10) with DEFAULT 8), we fall back to + * a table rewrite. This preserves the historical behavior that + * such a failure is only raised when the table has rows. */ if (rel->rd_rel->relkind == RELKIND_RELATION && !colDef->generated && - !has_domain_constraints && + !has_volatile && !contain_volatile_functions((Node *) defval)) { EState *estate; ExprState *exprState; Datum missingval; bool missingIsNull; + ErrorSaveContext escontext = {T_ErrorSaveContext}; - /* Evaluate the default expression */ + /* Evaluate the default expression with soft errors */ estate = CreateExecutorState(); - exprState = ExecPrepareExpr(defval, estate); + exprState = ExecPrepareExprWithContext(defval, estate, + (Node *) &escontext); missingval = ExecEvalExpr(exprState, GetPerTupleExprContext(estate), &missingIsNull); + + /* + * If the domain constraint check failed (via errsave), + * missingval is unreliable. Fall back to a table rewrite; + * Phase 3 will re-evaluate with hard errors, so the user gets + * an error only if the table has rows. + */ + if (SOFT_ERROR_OCCURRED(&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/backend/executor/execExpr.c b/src/backend/executor/execExpr.c index b96eb0c55cc..bd46b75e498 100644 --- a/src/backend/executor/execExpr.c +++ b/src/backend/executor/execExpr.c @@ -141,6 +141,26 @@ static void ExecInitJsonCoercion(ExprState *state, JsonReturning *returning, */ ExprState * ExecInitExpr(Expr *node, PlanState *parent) +{ + return ExecInitExprWithContext(node, parent, NULL); +} + +/* + * ExecInitExprWithContext: same as ExecInitExpr, but with an optional + * ErrorSaveContext for soft error handling. + * + * When 'escontext' is non-NULL, expression nodes that support soft errors + * (currently CoerceToDomain's NOT NULL and CHECK constraint steps) will use + * errsave() instead of ereport(), allowing the caller to detect and handle + * failures without a transaction abort. + * + * The escontext must be provided at initialization time (not after), because + * it is copied into per-step data during expression compilation. + * + * Not all expression node types support soft errors. If in doubt, pass NULL. + */ +ExprState * +ExecInitExprWithContext(Expr *node, PlanState *parent, Node *escontext) { ExprState *state; ExprEvalStep scratch = {0}; @@ -154,6 +174,7 @@ ExecInitExpr(Expr *node, PlanState *parent) state->expr = node; state->parent = parent; state->ext_params = NULL; + state->escontext = (ErrorSaveContext *) escontext; /* Insert setup steps as needed */ ExecCreateExprSetupSteps(state, (Node *) node); @@ -763,6 +784,18 @@ ExecBuildUpdateProjection(List *targetList, */ ExprState * ExecPrepareExpr(Expr *node, EState *estate) +{ + return ExecPrepareExprWithContext(node, estate, NULL); +} + +/* + * ExecPrepareExprWithContext: same as ExecPrepareExpr, but with an optional + * ErrorSaveContext for soft error handling. + * + * See ExecInitExprWithContext for details on the escontext parameter. + */ +ExprState * +ExecPrepareExprWithContext(Expr *node, EState *estate, Node *escontext) { ExprState *result; MemoryContext oldcontext; @@ -771,7 +804,7 @@ ExecPrepareExpr(Expr *node, EState *estate) node = expression_planner(node); - result = ExecInitExpr(node, NULL); + result = ExecInitExprWithContext(node, NULL, escontext); MemoryContextSwitchTo(oldcontext); diff --git a/src/include/executor/executor.h b/src/include/executor/executor.h index d46ba59895d..82c442d23f8 100644 --- a/src/include/executor/executor.h +++ b/src/include/executor/executor.h @@ -324,6 +324,7 @@ ExecProcNode(PlanState *node) * prototypes from functions in execExpr.c */ extern ExprState *ExecInitExpr(Expr *node, PlanState *parent); +extern ExprState *ExecInitExprWithContext(Expr *node, PlanState *parent, Node *escontext); extern ExprState *ExecInitExprWithParams(Expr *node, ParamListInfo ext_params); extern ExprState *ExecInitQual(List *qual, PlanState *parent); extern ExprState *ExecInitCheck(List *qual, PlanState *parent); @@ -372,6 +373,7 @@ extern ProjectionInfo *ExecBuildUpdateProjection(List *targetList, TupleTableSlot *slot, PlanState *parent); extern ExprState *ExecPrepareExpr(Expr *node, EState *estate); +extern ExprState *ExecPrepareExprWithContext(Expr *node, EState *estate, Node *escontext); extern ExprState *ExecPrepareQual(List *qual, EState *estate); extern ExprState *ExecPrepareCheck(List *qual, EState *estate); extern List *ExecPrepareExprList(List *nodes, EState *estate); diff --git a/src/test/regress/expected/fast_default.out b/src/test/regress/expected/fast_default.out index ccbcdf8403f..ffbc47089b1 100644 --- a/src/test/regress/expected/fast_default.out +++ b/src/test/regress/expected/fast_default.out @@ -317,11 +317,72 @@ 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 constraints +CREATE DOMAIN domain5 AS int CHECK(value > 10) DEFAULT 8; +CREATE DOMAIN domain6 as int CHECK(value > 10) DEFAULT random(min=>11, max=>100); +CREATE DOMAIN domain7 as int CHECK((value + random(min=>11::int, max=>11)) > 12); +CREATE DOMAIN domain8 as int NOT NULL; +CREATE TABLE test_add_domain_col(a int); +-- succeeds despite constraint-violating default because table is empty +ALTER TABLE test_add_domain_col ADD COLUMN a1 domain5; +NOTICE: rewriting table test_add_domain_col for reason 2 +ALTER TABLE test_add_domain_col DROP COLUMN a1; +INSERT INTO test_add_domain_col VALUES(1),(2); +-- tests with non-empty table +ALTER TABLE test_add_domain_col ADD COLUMN b domain5; -- table rewrite, then fail +NOTICE: rewriting table test_add_domain_col for reason 2 +ERROR: value for domain domain5 violates check constraint "domain5_check" +ALTER TABLE test_add_domain_col ADD COLUMN b domain8; -- table rewrite, then fail +NOTICE: rewriting table test_add_domain_col for reason 2 +ERROR: domain domain8 does not allow null values +ALTER TABLE test_add_domain_col ADD COLUMN b domain5 DEFAULT 1; -- table rewrite, then fail +NOTICE: rewriting table test_add_domain_col for reason 2 +ERROR: value for domain domain5 violates check constraint "domain5_check" +ALTER TABLE test_add_domain_col ADD COLUMN b domain5 DEFAULT 12; -- ok, no table rewrite +-- explicit column default expression overrides domain's default +-- expression, so no table rewrite +ALTER TABLE test_add_domain_col ADD COLUMN c domain6 DEFAULT 14; +ALTER TABLE test_add_domain_col ADD COLUMN c1 domain8 DEFAULT 13; -- no table rewrite +SELECT attnum, attname, atthasmissing, atthasdef, attmissingval +FROM pg_attribute +WHERE attnum > 0 AND attrelid = 'test_add_domain_col'::regclass AND attisdropped is false +AND atthasmissing +ORDER BY attnum; + attnum | attname | atthasmissing | atthasdef | attmissingval +--------+---------+---------------+-----------+--------------- + 3 | b | t | t | {12} + 4 | c | t | t | {14} + 5 | c1 | t | t | {13} +(3 rows) + +-- We need to rewrite the table whenever domain default contains volatile expression +ALTER TABLE test_add_domain_col ADD COLUMN d domain6; +NOTICE: rewriting table test_add_domain_col for reason 2 +-- We need to rewrite the table whenever domain constraint expression contains volatile expression +ALTER TABLE test_add_domain_col ADD COLUMN e domain7 default 14; +NOTICE: rewriting table test_add_domain_col for reason 2 +ALTER TABLE test_add_domain_col ADD COLUMN f domain7; +NOTICE: rewriting table test_add_domain_col for reason 2 +-- domain with both volatile and non-volatile CHECK constraints: the +-- volatile one forces a table rewrite +CREATE DOMAIN domain9 AS int CHECK(value > 10) CHECK((value + random(min=>1::int, max=>1)) > 0); +ALTER TABLE test_add_domain_col ADD COLUMN g domain9 DEFAULT 14; +NOTICE: rewriting table test_add_domain_col for reason 2 +-- virtual generated columns cannot have domain types +ALTER TABLE test_add_domain_col ADD COLUMN h domain5 + GENERATED ALWAYS AS (a + 20) VIRTUAL; -- error +ERROR: virtual generated column "h" cannot have a domain type DROP TABLE t2; +DROP TABLE test_add_domain_col; 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 DOMAIN domain9; 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..8ff29cf2697 100644 --- a/src/test/regress/sql/fast_default.sql +++ b/src/test/regress/sql/fast_default.sql @@ -287,11 +287,62 @@ 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 constraints +CREATE DOMAIN domain5 AS int CHECK(value > 10) DEFAULT 8; +CREATE DOMAIN domain6 as int CHECK(value > 10) DEFAULT random(min=>11, max=>100); +CREATE DOMAIN domain7 as int CHECK((value + random(min=>11::int, max=>11)) > 12); +CREATE DOMAIN domain8 as int NOT NULL; + +CREATE TABLE test_add_domain_col(a int); +-- succeeds despite constraint-violating default because table is empty +ALTER TABLE test_add_domain_col ADD COLUMN a1 domain5; +ALTER TABLE test_add_domain_col DROP COLUMN a1; +INSERT INTO test_add_domain_col VALUES(1),(2); + +-- tests with non-empty table +ALTER TABLE test_add_domain_col ADD COLUMN b domain5; -- table rewrite, then fail +ALTER TABLE test_add_domain_col ADD COLUMN b domain8; -- table rewrite, then fail +ALTER TABLE test_add_domain_col ADD COLUMN b domain5 DEFAULT 1; -- table rewrite, then fail +ALTER TABLE test_add_domain_col ADD COLUMN b domain5 DEFAULT 12; -- ok, no table rewrite + +-- explicit column default expression overrides domain's default +-- expression, so no table rewrite +ALTER TABLE test_add_domain_col ADD COLUMN c domain6 DEFAULT 14; + +ALTER TABLE test_add_domain_col ADD COLUMN c1 domain8 DEFAULT 13; -- no table rewrite +SELECT attnum, attname, atthasmissing, atthasdef, attmissingval +FROM pg_attribute +WHERE attnum > 0 AND attrelid = 'test_add_domain_col'::regclass AND attisdropped is false +AND atthasmissing +ORDER BY attnum; + +-- We need to rewrite the table whenever domain default contains volatile expression +ALTER TABLE test_add_domain_col ADD COLUMN d domain6; + +-- We need to rewrite the table whenever domain constraint expression contains volatile expression +ALTER TABLE test_add_domain_col ADD COLUMN e domain7 default 14; +ALTER TABLE test_add_domain_col ADD COLUMN f domain7; + +-- domain with both volatile and non-volatile CHECK constraints: the +-- volatile one forces a table rewrite +CREATE DOMAIN domain9 AS int CHECK(value > 10) CHECK((value + random(min=>1::int, max=>1)) > 0); +ALTER TABLE test_add_domain_col ADD COLUMN g domain9 DEFAULT 14; + +-- virtual generated columns cannot have domain types +ALTER TABLE test_add_domain_col ADD COLUMN h domain5 + GENERATED ALWAYS AS (a + 20) VIRTUAL; -- error + DROP TABLE t2; +DROP TABLE test_add_domain_col; 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 DOMAIN domain9; DROP FUNCTION foo(INT); -- Fall back to full rewrite for volatile expressions