From 576164f2334a958dcaf39747ba8e068c852e4fc3 Mon Sep 17 00:00:00 2001 From: "Chao Li (Evan)" Date: Thu, 4 Jun 2026 18:06:01 +0800 Subject: [PATCH v1-s1] Handle throwing domain checks when probing fast defaults. ALTER TABLE ADD COLUMN tries to prove that a default value can be stored as a missing value, using soft errors so that a failed domain constraint falls back to a table rewrite. However, a domain CHECK expression can throw an error before returning false, for example due to division by zero. That made the fast-default probe fail immediately, even for an empty table where the rewrite path would have succeeded. For direct domain coercions, evaluate the default expression separately and then validate the resulting datum with domain_check_safe(). Catch errors only around the domain validation step, so errors from the default expression itself are still reported normally. Author: Chao Li --- src/backend/commands/tablecmds.c | 66 ++++++++++++++++++---- src/test/regress/expected/fast_default.out | 19 ++++++- src/test/regress/sql/fast_default.sql | 17 +++++- 3 files changed, 87 insertions(+), 15 deletions(-) diff --git a/src/backend/commands/tablecmds.c b/src/backend/commands/tablecmds.c index a1845240a98..585df658258 100644 --- a/src/backend/commands/tablecmds.c +++ b/src/backend/commands/tablecmds.c @@ -7640,23 +7640,67 @@ ATExecAddColumn(List **wqueue, AlteredTableInfo *tab, Relation rel, ExprState *exprState; Datum missingval; bool missingIsNull; + volatile bool caught_error = false; ErrorSaveContext escontext = {T_ErrorSaveContext}; + CoerceToDomain *ctest = NULL; - /* Evaluate the default expression with soft errors */ + if (has_domain_constraints && IsA(defval, CoerceToDomain)) + ctest = castNode(CoerceToDomain, defval); + + /* + * Evaluate the default expression, using soft errors where + * possible + */ estate = CreateExecutorState(); - exprState = ExecPrepareExprWithContext(defval, estate, - (Node *) &escontext); - missingval = ExecEvalExpr(exprState, - GetPerTupleExprContext(estate), - &missingIsNull); + if (ctest) + { + MemoryContext oldcxt = CurrentMemoryContext; + + /* + * Evaluate the default expression itself with hard + * errors. + */ + exprState = ExecPrepareExpr(ctest->arg, estate); + missingval = ExecEvalExpr(exprState, + GetPerTupleExprContext(estate), + &missingIsNull); + + /* + * A domain CHECK expression can fail by throwing an + * error, rather than by returning false. Treat that like + * any other failed proof that the value is safe to cache. + */ + PG_TRY(); + { + (void) domain_check_safe(missingval, missingIsNull, + ctest->resulttype, + NULL, NULL, + (Node *) &escontext); + } + PG_CATCH(); + { + MemoryContextSwitchTo(oldcxt); + FlushErrorState(); + caught_error = true; + } + PG_END_TRY(); + } + else + { + 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 the domain constraint check failed, 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 (escontext.error_occurred) + if (caught_error || escontext.error_occurred) { missingIsNull = true; tab->rewrite |= AT_REWRITE_DEFAULT_VAL; diff --git a/src/test/regress/expected/fast_default.out b/src/test/regress/expected/fast_default.out index 5813f1c61a5..bdaa5aedb42 100644 --- a/src/test/regress/expected/fast_default.out +++ b/src/test/regress/expected/fast_default.out @@ -322,12 +322,23 @@ 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 DOMAIN domain9 AS int CHECK(1 / (value - 1) > 0); 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); +-- likewise, an empty table succeeds if the domain check expression errors +CREATE TABLE test_add_domain_col_empty(a int); +ALTER TABLE test_add_domain_col_empty ADD COLUMN b domain9 DEFAULT 1; +NOTICE: rewriting table test_add_domain_col_empty for reason 2 +DROP TABLE test_add_domain_col_empty; +-- but errors in the default expression itself should not be hidden +CREATE TABLE test_add_domain_col_bad_default(a int); +ALTER TABLE test_add_domain_col_bad_default ADD COLUMN b domain5 DEFAULT 1 / 0; +ERROR: division by zero +DROP TABLE test_add_domain_col_bad_default; -- 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 @@ -338,6 +349,9 @@ 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 domain9 DEFAULT 1; -- table rewrite, then fail +NOTICE: rewriting table test_add_domain_col for reason 2 +ERROR: division by zero 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 @@ -365,8 +379,8 @@ 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; +CREATE DOMAIN domain10 AS int CHECK(value > 10) CHECK((value + random(min=>1::int, max=>1)) > 0); +ALTER TABLE test_add_domain_col ADD COLUMN g domain10 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 @@ -383,6 +397,7 @@ DROP DOMAIN domain6; DROP DOMAIN domain7; DROP DOMAIN domain8; DROP DOMAIN domain9; +DROP DOMAIN domain10; 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 e5d9a3d2fd1..290321fe60a 100644 --- a/src/test/regress/sql/fast_default.sql +++ b/src/test/regress/sql/fast_default.sql @@ -292,6 +292,7 @@ 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 DOMAIN domain9 AS int CHECK(1 / (value - 1) > 0); CREATE TABLE test_add_domain_col(a int); -- succeeds despite constraint-violating default because table is empty @@ -299,10 +300,21 @@ 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); +-- likewise, an empty table succeeds if the domain check expression errors +CREATE TABLE test_add_domain_col_empty(a int); +ALTER TABLE test_add_domain_col_empty ADD COLUMN b domain9 DEFAULT 1; +DROP TABLE test_add_domain_col_empty; + +-- but errors in the default expression itself should not be hidden +CREATE TABLE test_add_domain_col_bad_default(a int); +ALTER TABLE test_add_domain_col_bad_default ADD COLUMN b domain5 DEFAULT 1 / 0; +DROP TABLE test_add_domain_col_bad_default; + -- 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 domain9 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 @@ -325,8 +337,8 @@ 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; +CREATE DOMAIN domain10 AS int CHECK(value > 10) CHECK((value + random(min=>1::int, max=>1)) > 0); +ALTER TABLE test_add_domain_col ADD COLUMN g domain10 DEFAULT 14; -- virtual generated columns cannot have domain types ALTER TABLE test_add_domain_col ADD COLUMN h domain5 @@ -343,6 +355,7 @@ DROP DOMAIN domain6; DROP DOMAIN domain7; DROP DOMAIN domain8; DROP DOMAIN domain9; +DROP DOMAIN domain10; DROP FUNCTION foo(INT); -- Fall back to full rewrite for volatile expressions -- 2.50.1 (Apple Git-155)