public inbox for [email protected]
help / color / mirror / Atom feedFrom: Chao Li <[email protected]>
To: Postgres hackers <[email protected]>
Cc: Andrew Dunstan <[email protected]>
Cc: jian he <[email protected]>
Subject: Fix domain fast defaults on empty tables
Date: Fri, 5 Jun 2026 15:48:00 +0800
Message-ID: <[email protected]> (raw)
Hi,
I tested "[a0b6ef29a] Enable fast default for domains with non-volatile constraints". After tracing some cases from the regression tests, I came up with this test case and found a bug:
```
evantest=# create domain d_div as int check (1 / (value - 1) > 0);
CREATE DOMAIN
evantest=# create table t (a int);
CREATE TABLE
evantest=# alter table t add column b d_div default 1;
ERROR: division by zero
```
It looks like errors inside the CHECK expression itself, such as the int4div division-by-zero in this test, are still hard errors that can fail the ALTER TABLE command.
For the fix, I worked out two solutions:
Solution 1
========
We can add PG_TRY/PG_CATCH to capture those hard errors. But as a0b6ef29a's commit message mentions, the fallback should only apply while evaluating the CoerceToDomain expression. To avoid broadly suppressing hard errors, I only catch hard errors from domain_check_safe(), which verifies the default value against the domain. The default expression itself is still evaluated with hard errors.
My concern with this solution is that there might be some error from domain_check_safe() that I am not aware of and that would be hidden by the try-catch. But that may be acceptable, because it matches the behavior before a0b6ef29a, so at least it is not a regression.
Solution 2
========
This solution is simpler and is based on the purpose of the original feature. a0b6ef29a's commit message says the purpose of the feature is to avoid table rewriting. Since an empty table has no data to rewrite, we can bypass the fast path for empty tables.
The problem with this solution is that I currently use RelationGetNumberOfBlocks(rel) to decide whether the table is empty, so it cannot handle cases like:
```
Begin;
Delete from t;
Alter table t add column …
```
See the attached patches for details. v1-s1 is solution 1, and v1-s2 is solution 2. Please let me know which solution is preferred.
Best regards,
--
Chao Li (Evan)
HighGo Software Co., Ltd.
https://www.highgo.com/
Attachments:
[application/octet-stream] v1-s1-0001-Handle-throwing-domain-checks-when-probing-fas.patch (9.8K, 2-v1-s1-0001-Handle-throwing-domain-checks-when-probing-fas.patch)
download | inline diff:
From 576164f2334a958dcaf39747ba8e068c852e4fc3 Mon Sep 17 00:00:00 2001
From: "Chao Li (Evan)" <[email protected]>
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 <[email protected]>
---
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)
[application/octet-stream] v1-s2-0001-Preserve-empty-table-behavior-for-domain-fast-.patch (8.6K, 3-v1-s2-0001-Preserve-empty-table-behavior-for-domain-fast-.patch)
download | inline diff:
From 697a9568279634b0cf4e000be7850e8845e37ce0 Mon Sep 17 00:00:00 2001
From: "Chao Li (Evan)" <[email protected]>
Date: Thu, 4 Jun 2026 18:06:01 +0800
Subject: [PATCH v1-s2] Preserve empty-table behavior for domain fast defaults.
Commit a0b6ef29a allowed ALTER TABLE ADD COLUMN to use the
missing-value fast path for domains with non-volatile constraints.
However, while proving that the default can be stored as a missing value,
a domain CHECK expression can throw an error, for example division by
zero. That changed behavior for freshly empty tables, where the old
rewrite path scanned no rows and therefore did not evaluate the invalid
domain value during ALTER TABLE.
Skip the fast-default probe for constrained domains when the relation has
no heap blocks. In that case there is no data rewrite to avoid, and using
the rewrite path preserves the previous behavior.
Add regression coverage for an empty table whose domain CHECK expression
throws during evaluation, while keeping the non-empty case as an error.
Author: Chao Li <[email protected]>
---
src/backend/commands/tablecmds.c | 13 +++++++++++++
src/test/regress/expected/fast_default.out | 18 ++++++++++++++++--
src/test/regress/sql/fast_default.sql | 17 +++++++++++++++--
3 files changed, 44 insertions(+), 4 deletions(-)
diff --git a/src/backend/commands/tablecmds.c b/src/backend/commands/tablecmds.c
index a1845240a98..d74635d0a23 100644
--- a/src/backend/commands/tablecmds.c
+++ b/src/backend/commands/tablecmds.c
@@ -7548,6 +7548,7 @@ ATExecAddColumn(List **wqueue, AlteredTableInfo *tab, Relation rel,
bool has_domain_constraints;
bool has_missing = false;
bool has_volatile = false;
+ bool skip_fast_default = false;
/*
* For an identity column, we can't use build_column_default(),
@@ -7599,6 +7600,17 @@ ATExecAddColumn(List **wqueue, AlteredTableInfo *tab, Relation rel,
elog(ERROR, "failed to coerce base type to domain");
}
+ /*
+ * For constrained domains, a physically empty table does not need the
+ * fast default optimization. Use the rewrite path instead,
+ * preserving the old behavior of not evaluating the domain constraint
+ * when there are no heap tuples to rewrite.
+ */
+ if (has_domain_constraints &&
+ rel->rd_rel->relkind == RELKIND_RELATION &&
+ RelationGetNumberOfBlocks(rel) == 0)
+ skip_fast_default = true;
+
if (defval)
{
NewColumnValue *newval;
@@ -7632,6 +7644,7 @@ ATExecAddColumn(List **wqueue, AlteredTableInfo *tab, Relation rel,
* such a failure is only raised when the table has rows.
*/
if (rel->rd_rel->relkind == RELKIND_RELATION &&
+ !skip_fast_default &&
!colDef->generated &&
!has_volatile &&
!contain_volatile_functions((Node *) defval))
diff --git a/src/test/regress/expected/fast_default.out b/src/test/regress/expected/fast_default.out
index 5813f1c61a5..1bcc7be43e3 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,8 @@ 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; -- fast proof fails
+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 +378,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 +396,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..8b58c6d44d0 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; -- fast proof fails
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)
view thread (11+ messages) latest in thread
reply
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Reply to all the recipients using the --to and --cc options:
reply via email
To: [email protected]
Cc: [email protected], [email protected], [email protected], [email protected]
Subject: Re: Fix domain fast defaults on empty tables
In-Reply-To: <[email protected]>
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
This inbox is served by agora; see mirroring instructions
for how to clone and mirror all data and code used for this inbox