public inbox for [email protected]
help / color / mirror / Atom feedFix domain fast defaults on empty tables
11+ messages / 5 participants
[nested] [flat]
* Fix domain fast defaults on empty tables
@ 2026-06-05 07:48 Chao Li <[email protected]>
2026-06-05 09:04 ` Re: Fix domain fast defaults on empty tables Heikki Linnakangas <[email protected]>
0 siblings, 1 reply; 11+ messages in thread
From: Chao Li @ 2026-06-05 07:48 UTC (permalink / raw)
To: Postgres hackers <[email protected]>; +Cc: Andrew Dunstan <[email protected]>; jian he <[email protected]>
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)
^ permalink raw reply [nested|flat] 11+ messages in thread
* Re: Fix domain fast defaults on empty tables
2026-06-05 07:48 Fix domain fast defaults on empty tables Chao Li <[email protected]>
@ 2026-06-05 09:04 ` Heikki Linnakangas <[email protected]>
2026-06-05 09:52 ` Re: Fix domain fast defaults on empty tables Chao Li <[email protected]>
2026-06-05 14:08 ` Re: Fix domain fast defaults on empty tables Tom Lane <[email protected]>
0 siblings, 2 replies; 11+ messages in thread
From: Heikki Linnakangas @ 2026-06-05 09:04 UTC (permalink / raw)
To: [email protected]; Chao Li <[email protected]>; Postgres hackers <[email protected]>; +Cc: Andrew Dunstan <[email protected]>; jian he <[email protected]>
On 5 June 2026 10:48:00 EEST, Chao Li <[email protected]> wrote:
>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.
It seems totally reasonable to get an error in that case. '1' is not a valid value for the datatype, whether or not there are any rows in the table.
- Heikki
^ permalink raw reply [nested|flat] 11+ messages in thread
* Re: Fix domain fast defaults on empty tables
2026-06-05 07:48 Fix domain fast defaults on empty tables Chao Li <[email protected]>
2026-06-05 09:04 ` Re: Fix domain fast defaults on empty tables Heikki Linnakangas <[email protected]>
@ 2026-06-05 09:52 ` Chao Li <[email protected]>
1 sibling, 0 replies; 11+ messages in thread
From: Chao Li @ 2026-06-05 09:52 UTC (permalink / raw)
To: Heikki Linnakangas <[email protected]>; +Cc: [email protected]; Andrew Dunstan <[email protected]>; jian he <[email protected]>
> On Jun 5, 2026, at 17:04, Heikki Linnakangas <[email protected]> wrote:
>
>
>
> On 5 June 2026 10:48:00 EEST, Chao Li <[email protected]> wrote:
>> 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.
>
> It seems totally reasonable to get an error in that case. '1' is not a valid value for the datatype, whether or not there are any rows in the table.
>
> - Heikki
I agree that rejecting the default is semantically reasonable. But the concern is that this is a user-visible behavior change, and the feature didn’t seem intended to make that change. Before a0b6ef29a, this ALTER TABLE command could succeed. If we now want to reject such defaults, I think that should be documented explicitly.
Also, there might be some practical use for the current behavior. For example, an invalid default can be used to prevent INSERT from omitting the column.
Best regards,
--
Chao Li (Evan)
HighGo Software Co., Ltd.
https://www.highgo.com/
^ permalink raw reply [nested|flat] 11+ messages in thread
* Re: Fix domain fast defaults on empty tables
2026-06-05 07:48 Fix domain fast defaults on empty tables Chao Li <[email protected]>
2026-06-05 09:04 ` Re: Fix domain fast defaults on empty tables Heikki Linnakangas <[email protected]>
@ 2026-06-05 14:08 ` Tom Lane <[email protected]>
2026-06-06 12:41 ` Re: Fix domain fast defaults on empty tables Andrew Dunstan <[email protected]>
2026-06-08 00:33 ` Re: Fix domain fast defaults on empty tables jian he <[email protected]>
2026-06-08 08:20 ` Re: Fix domain fast defaults on empty tables jian he <[email protected]>
1 sibling, 3 replies; 11+ messages in thread
From: Tom Lane @ 2026-06-05 14:08 UTC (permalink / raw)
To: Heikki Linnakangas <[email protected]>; +Cc: [email protected]; Chao Li <[email protected]>; Andrew Dunstan <[email protected]>; jian he <[email protected]>
Heikki Linnakangas <[email protected]> writes:
> On 5 June 2026 10:48:00 EEST, Chao Li <[email protected]> wrote:
>> 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 seems totally reasonable to get an error in that case. '1' is not a valid value for the datatype, whether or not there are any rows in the table.
I think there's reason for concern here, which is that we do not throw
an error for the apparently equivalent case
regression=# create table t2 (a int, b d_div default 1);
CREATE TABLE
This will give you an error at INSERT, but not CREATE. So this
is inconsistent, as well as different from the pre-v19 behavior.
Concretely, I'm pretty sure it is a hazard for pg_dump, which thinks
it can freely transform bits of CREATE operations into ALTERs.
I didn't try to make an example case, but I suspect it is now possible
to create a database that will fail dump/restore because of this
inconsistency.
regards, tom lane
^ permalink raw reply [nested|flat] 11+ messages in thread
* Re: Fix domain fast defaults on empty tables
2026-06-05 07:48 Fix domain fast defaults on empty tables Chao Li <[email protected]>
2026-06-05 09:04 ` Re: Fix domain fast defaults on empty tables Heikki Linnakangas <[email protected]>
2026-06-05 14:08 ` Re: Fix domain fast defaults on empty tables Tom Lane <[email protected]>
@ 2026-06-06 12:41 ` Andrew Dunstan <[email protected]>
2026-06-06 20:19 ` Re: Fix domain fast defaults on empty tables Tom Lane <[email protected]>
2 siblings, 1 reply; 11+ messages in thread
From: Andrew Dunstan @ 2026-06-06 12:41 UTC (permalink / raw)
To: Tom Lane <[email protected]>; Heikki Linnakangas <[email protected]>; +Cc: [email protected]; Chao Li <[email protected]>; jian he <[email protected]>
On 2026-06-05 Fr 10:08 AM, Tom Lane wrote:
> Heikki Linnakangas<[email protected]> writes:
>> On 5 June 2026 10:48:00 EEST, Chao Li<[email protected]> wrote:
>>> 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 seems totally reasonable to get an error in that case. '1' is not a valid value for the datatype, whether or not there are any rows in the table.
> I think there's reason for concern here, which is that we do not throw
> an error for the apparently equivalent case
>
> regression=# create table t2 (a int, b d_div default 1);
> CREATE TABLE
>
> This will give you an error at INSERT, but not CREATE. So this
> is inconsistent, as well as different from the pre-v19 behavior.
>
> Concretely, I'm pretty sure it is a hazard for pg_dump, which thinks
> it can freely transform bits of CREATE operations into ALTERs.
> I didn't try to make an example case, but I suspect it is now possible
> to create a database that will fail dump/restore because of this
> inconsistency.
>
>
Seems reasonable. So which of Chao's solutions do you prefer? I think
both will meet the pg_dump issue, not sure how much we care about the
case where we have deleted all the rows but not truncated the table.
cheers
andrew
--
Andrew Dunstan
EDB: https://www.enterprisedb.com
^ permalink raw reply [nested|flat] 11+ messages in thread
* Re: Fix domain fast defaults on empty tables
2026-06-05 07:48 Fix domain fast defaults on empty tables Chao Li <[email protected]>
2026-06-05 09:04 ` Re: Fix domain fast defaults on empty tables Heikki Linnakangas <[email protected]>
2026-06-05 14:08 ` Re: Fix domain fast defaults on empty tables Tom Lane <[email protected]>
2026-06-06 12:41 ` Re: Fix domain fast defaults on empty tables Andrew Dunstan <[email protected]>
@ 2026-06-06 20:19 ` Tom Lane <[email protected]>
2026-06-07 02:09 ` Re: Fix domain fast defaults on empty tables Chao Li <[email protected]>
0 siblings, 1 reply; 11+ messages in thread
From: Tom Lane @ 2026-06-06 20:19 UTC (permalink / raw)
To: Andrew Dunstan <[email protected]>; +Cc: Heikki Linnakangas <[email protected]>; [email protected]; Chao Li <[email protected]>; jian he <[email protected]>
Andrew Dunstan <[email protected]> writes:
> On 2026-06-05 Fr 10:08 AM, Tom Lane wrote:
>> Concretely, I'm pretty sure it is a hazard for pg_dump, which thinks
>> it can freely transform bits of CREATE operations into ALTERs.
>> I didn't try to make an example case, but I suspect it is now possible
>> to create a database that will fail dump/restore because of this
>> inconsistency.
> Seems reasonable. So which of Chao's solutions do you prefer? I think
> both will meet the pg_dump issue, not sure how much we care about the
> case where we have deleted all the rows but not truncated the table.
[ studies it a bit ...] TBH, I don't like either of these,
nor do I like a0b6ef29a to begin with. The fundamental problem
with this whole mess is that it treats certain kinds of
default-expression evaluation error (i.e., domain CHECK failures)
differently from others. There is no way that that leads to a
consistent user experience. The current complaint is one
manifestation of that, but I'm sure there are others, eg having
to do with domain coercions lower down in a default expression.
It could work if ExecPrepareExprWithContext were capable of
soft-trapping essentially all execution errors, but that's
not true today and I strongly doubt it ever will be true.
I think Chao's v1-s2-0001 points the way towards what could be a
workable solution: if we see that the table is known empty (after
we already have exclusive lock on it!), we could skip both the table
rewrite and the insertion of an attmissingval, and thereby not need
to evaluate the default at all during ALTER TABLE. As Chao says,
simplistic versions of "known empty" would expose some user-visible
behavioral inconsistencies, but I'm not sure how much that matters.
For pg_dump and similar applications we would get the behavior we
needed even with just an is-physically-empty check, since all their
CREATE/ALTER DDL happens before we ever insert any data. You could
imagine going further and scanning the table for live tuples, but
I don't know that that's going to be worth the cycles.
However, I doubt that a bare "RelationGetNumberOfBlocks() == 0" check
is acceptable: we probably need to let the table access method have
control of this. heapam could do "RelationGetNumberOfBlocks() == 0",
but other TAMs might need to do something else. So that means that
this path requires a new TableAmRoutine method, and that probably
puts it in the too-late-for-v19 category.
My recommendation: we ought to revert a0b6ef29a for now and
redesign the optimization for v20.
If you don't like that answer, I'd be firmly against v1-s1-0001
in any case. It repeats the classic mistake of supposing that
we can PG_CATCH random errors and not invoke transaction cleanup.
BTW, I do not like the fact that a0b6ef29a removed the comment
stanza explaining why we need this fake default expression at all.
That's still largely applicable AFAICS.
regards, tom lane
^ permalink raw reply [nested|flat] 11+ messages in thread
* Re: Fix domain fast defaults on empty tables
2026-06-05 07:48 Fix domain fast defaults on empty tables Chao Li <[email protected]>
2026-06-05 09:04 ` Re: Fix domain fast defaults on empty tables Heikki Linnakangas <[email protected]>
2026-06-05 14:08 ` Re: Fix domain fast defaults on empty tables Tom Lane <[email protected]>
2026-06-06 12:41 ` Re: Fix domain fast defaults on empty tables Andrew Dunstan <[email protected]>
2026-06-06 20:19 ` Re: Fix domain fast defaults on empty tables Tom Lane <[email protected]>
@ 2026-06-07 02:09 ` Chao Li <[email protected]>
2026-06-08 15:40 ` Re: Fix domain fast defaults on empty tables Andrew Dunstan <[email protected]>
0 siblings, 1 reply; 11+ messages in thread
From: Chao Li @ 2026-06-07 02:09 UTC (permalink / raw)
To: Tom Lane <[email protected]>; +Cc: Andrew Dunstan <[email protected]>; Heikki Linnakangas <[email protected]>; [email protected]; jian he <[email protected]>
> On Jun 7, 2026, at 04:19, Tom Lane <[email protected]> wrote:
>
> Andrew Dunstan <[email protected]> writes:
>> On 2026-06-05 Fr 10:08 AM, Tom Lane wrote:
>>> Concretely, I'm pretty sure it is a hazard for pg_dump, which thinks
>>> it can freely transform bits of CREATE operations into ALTERs.
>>> I didn't try to make an example case, but I suspect it is now possible
>>> to create a database that will fail dump/restore because of this
>>> inconsistency.
>
>> Seems reasonable. So which of Chao's solutions do you prefer? I think
>> both will meet the pg_dump issue, not sure how much we care about the
>> case where we have deleted all the rows but not truncated the table.
>
> [ studies it a bit ...] TBH, I don't like either of these,
> nor do I like a0b6ef29a to begin with. The fundamental problem
> with this whole mess is that it treats certain kinds of
> default-expression evaluation error (i.e., domain CHECK failures)
> differently from others. There is no way that that leads to a
> consistent user experience. The current complaint is one
> manifestation of that, but I'm sure there are others, eg having
> to do with domain coercions lower down in a default expression.
>
> It could work if ExecPrepareExprWithContext were capable of
> soft-trapping essentially all execution errors, but that's
> not true today and I strongly doubt it ever will be true.
>
> I think Chao's v1-s2-0001 points the way towards what could be a
> workable solution: if we see that the table is known empty (after
> we already have exclusive lock on it!), we could skip both the table
> rewrite and the insertion of an attmissingval, and thereby not need
> to evaluate the default at all during ALTER TABLE. As Chao says,
> simplistic versions of "known empty" would expose some user-visible
> behavioral inconsistencies, but I'm not sure how much that matters.
> For pg_dump and similar applications we would get the behavior we
> needed even with just an is-physically-empty check, since all their
> CREATE/ALTER DDL happens before we ever insert any data. You could
> imagine going further and scanning the table for live tuples, but
> I don't know that that's going to be worth the cycles.
>
> However, I doubt that a bare "RelationGetNumberOfBlocks() == 0" check
> is acceptable: we probably need to let the table access method have
> control of this. heapam could do "RelationGetNumberOfBlocks() == 0",
> but other TAMs might need to do something else. So that means that
> this path requires a new TableAmRoutine method, and that probably
> puts it in the too-late-for-v19 category.
>
> My recommendation: we ought to revert a0b6ef29a for now and
> redesign the optimization for v20.
>
> If you don't like that answer, I'd be firmly against v1-s1-0001
> in any case. It repeats the classic mistake of supposing that
> we can PG_CATCH random errors and not invoke transaction cleanup.
>
> BTW, I do not like the fact that a0b6ef29a removed the comment
> stanza explaining why we need this fake default expression at all.
> That's still largely applicable AFAICS.
>
> regards, tom lane
Thanks, Tom, for the detailed explanation.
I'll hold off here and wait for Andrew's decision on how to proceed. Sounds like reverting a0b6ef29a may be the preferred option for now.
For v1-s2, I’m not ware of a better way to decide whether a table is known empty. I ever considered a count(*)-style scan or a LIMIT 1 scan, but those seem worse, they would add a new table scan inside ALTER TABLE just to decide whether to evaluate the default.
Best regards,
--
Chao Li (Evan)
HighGo Software Co., Ltd.
https://www.highgo.com/
^ permalink raw reply [nested|flat] 11+ messages in thread
* Re: Fix domain fast defaults on empty tables
2026-06-05 07:48 Fix domain fast defaults on empty tables Chao Li <[email protected]>
2026-06-05 09:04 ` Re: Fix domain fast defaults on empty tables Heikki Linnakangas <[email protected]>
2026-06-05 14:08 ` Re: Fix domain fast defaults on empty tables Tom Lane <[email protected]>
2026-06-06 12:41 ` Re: Fix domain fast defaults on empty tables Andrew Dunstan <[email protected]>
2026-06-06 20:19 ` Re: Fix domain fast defaults on empty tables Tom Lane <[email protected]>
2026-06-07 02:09 ` Re: Fix domain fast defaults on empty tables Chao Li <[email protected]>
@ 2026-06-08 15:40 ` Andrew Dunstan <[email protected]>
0 siblings, 0 replies; 11+ messages in thread
From: Andrew Dunstan @ 2026-06-08 15:40 UTC (permalink / raw)
To: Chao Li <[email protected]>; Tom Lane <[email protected]>; +Cc: Heikki Linnakangas <[email protected]>; [email protected]; jian he <[email protected]>
On 2026-06-06 Sa 10:09 PM, Chao Li wrote:
>
>> On Jun 7, 2026, at 04:19, Tom Lane <[email protected]> wrote:
>>
>> Andrew Dunstan <[email protected]> writes:
>>> On 2026-06-05 Fr 10:08 AM, Tom Lane wrote:
>>>> Concretely, I'm pretty sure it is a hazard for pg_dump, which thinks
>>>> it can freely transform bits of CREATE operations into ALTERs.
>>>> I didn't try to make an example case, but I suspect it is now possible
>>>> to create a database that will fail dump/restore because of this
>>>> inconsistency.
>>> Seems reasonable. So which of Chao's solutions do you prefer? I think
>>> both will meet the pg_dump issue, not sure how much we care about the
>>> case where we have deleted all the rows but not truncated the table.
>> [ studies it a bit ...] TBH, I don't like either of these,
>> nor do I like a0b6ef29a to begin with. The fundamental problem
>> with this whole mess is that it treats certain kinds of
>> default-expression evaluation error (i.e., domain CHECK failures)
>> differently from others. There is no way that that leads to a
>> consistent user experience. The current complaint is one
>> manifestation of that, but I'm sure there are others, eg having
>> to do with domain coercions lower down in a default expression.
>>
>> It could work if ExecPrepareExprWithContext were capable of
>> soft-trapping essentially all execution errors, but that's
>> not true today and I strongly doubt it ever will be true.
>>
>> I think Chao's v1-s2-0001 points the way towards what could be a
>> workable solution: if we see that the table is known empty (after
>> we already have exclusive lock on it!), we could skip both the table
>> rewrite and the insertion of an attmissingval, and thereby not need
>> to evaluate the default at all during ALTER TABLE. As Chao says,
>> simplistic versions of "known empty" would expose some user-visible
>> behavioral inconsistencies, but I'm not sure how much that matters.
>> For pg_dump and similar applications we would get the behavior we
>> needed even with just an is-physically-empty check, since all their
>> CREATE/ALTER DDL happens before we ever insert any data. You could
>> imagine going further and scanning the table for live tuples, but
>> I don't know that that's going to be worth the cycles.
>>
>> However, I doubt that a bare "RelationGetNumberOfBlocks() == 0" check
>> is acceptable: we probably need to let the table access method have
>> control of this. heapam could do "RelationGetNumberOfBlocks() == 0",
>> but other TAMs might need to do something else. So that means that
>> this path requires a new TableAmRoutine method, and that probably
>> puts it in the too-late-for-v19 category.
>>
>> My recommendation: we ought to revert a0b6ef29a for now and
>> redesign the optimization for v20.
>>
>> If you don't like that answer, I'd be firmly against v1-s1-0001
>> in any case. It repeats the classic mistake of supposing that
>> we can PG_CATCH random errors and not invoke transaction cleanup.
>>
>> BTW, I do not like the fact that a0b6ef29a removed the comment
>> stanza explaining why we need this fake default expression at all.
>> That's still largely applicable AFAICS.
>>
>> regards, tom lane
> Thanks, Tom, for the detailed explanation.
>
> I'll hold off here and wait for Andrew's decision on how to proceed. Sounds like reverting a0b6ef29a may be the preferred option for now.
>
> For v1-s2, I’m not ware of a better way to decide whether a table is known empty. I ever considered a count(*)-style scan or a LIMIT 1 scan, but those seem worse, they would add a new table scan inside ALTER TABLE just to decide whether to evaluate the default.
>
Given Tom's reasonable suggestion that we really need a new Table AM
callback to do this sensibly, I agree we should revert it. Will do.
cheers
andrew
--
Andrew Dunstan
EDB: https://www.enterprisedb.com
^ permalink raw reply [nested|flat] 11+ messages in thread
* Re: Fix domain fast defaults on empty tables
2026-06-05 07:48 Fix domain fast defaults on empty tables Chao Li <[email protected]>
2026-06-05 09:04 ` Re: Fix domain fast defaults on empty tables Heikki Linnakangas <[email protected]>
2026-06-05 14:08 ` Re: Fix domain fast defaults on empty tables Tom Lane <[email protected]>
@ 2026-06-08 00:33 ` jian he <[email protected]>
2026-06-08 18:12 ` Re: Fix domain fast defaults on empty tables Tom Lane <[email protected]>
2 siblings, 1 reply; 11+ messages in thread
From: jian he @ 2026-06-08 00:33 UTC (permalink / raw)
To: Tom Lane <[email protected]>; +Cc: Heikki Linnakangas <[email protected]>; [email protected]; Chao Li <[email protected]>; Andrew Dunstan <[email protected]>
On Fri, Jun 5, 2026 at 10:08 PM Tom Lane <[email protected]> wrote:
>
> Heikki Linnakangas <[email protected]> writes:
> > On 5 June 2026 10:48:00 EEST, Chao Li <[email protected]> wrote:
> >> 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 seems totally reasonable to get an error in that case. '1' is not a valid value for the datatype, whether or not there are any rows in the table.
>
> I think there's reason for concern here, which is that we do not throw
> an error for the apparently equivalent case
>
> regression=# create table t2 (a int, b d_div default 1);
> CREATE TABLE
>
> This will give you an error at INSERT, but not CREATE. So this
> is inconsistent, as well as different from the pre-v19 behavior.
>
However, this is normal behavior for non-domain types.
create table t2 (a numeric default (1::numeric/0.0::float4)); -- ok
alter table t2 add column b numeric default ((1::numeric/0.0::float4)); -- error
^ permalink raw reply [nested|flat] 11+ messages in thread
* Re: Fix domain fast defaults on empty tables
2026-06-05 07:48 Fix domain fast defaults on empty tables Chao Li <[email protected]>
2026-06-05 09:04 ` Re: Fix domain fast defaults on empty tables Heikki Linnakangas <[email protected]>
2026-06-05 14:08 ` Re: Fix domain fast defaults on empty tables Tom Lane <[email protected]>
2026-06-08 00:33 ` Re: Fix domain fast defaults on empty tables jian he <[email protected]>
@ 2026-06-08 18:12 ` Tom Lane <[email protected]>
0 siblings, 0 replies; 11+ messages in thread
From: Tom Lane @ 2026-06-08 18:12 UTC (permalink / raw)
To: jian he <[email protected]>; +Cc: Heikki Linnakangas <[email protected]>; [email protected]; Chao Li <[email protected]>; Andrew Dunstan <[email protected]>
jian he <[email protected]> writes:
> On Fri, Jun 5, 2026 at 10:08 PM Tom Lane <[email protected]> wrote:
>> I think there's reason for concern here, which is that we do not throw
>> an error for the apparently equivalent case
>> regression=# create table t2 (a int, b d_div default 1);
>> CREATE TABLE
>> This will give you an error at INSERT, but not CREATE. So this
>> is inconsistent, as well as different from the pre-v19 behavior.
> However, this is normal behavior for non-domain types.
> create table t2 (a numeric default (1::numeric/0.0::float4)); -- ok
> alter table t2 add column b numeric default ((1::numeric/0.0::float4)); -- error
Well, that's not great either. The idea of avoiding evaluating
the default expression altogether when the table is empty could
ameliorate that problem too.
But my point stands: a0b6ef29a introduces different treatment for
domain-CHECK-constraint errors than other types of runtime errors
in the default expression. I think that is fundamentally the wrong
direction to go in. We want more consistency here, not less.
regards, tom lane
^ permalink raw reply [nested|flat] 11+ messages in thread
* Re: Fix domain fast defaults on empty tables
2026-06-05 07:48 Fix domain fast defaults on empty tables Chao Li <[email protected]>
2026-06-05 09:04 ` Re: Fix domain fast defaults on empty tables Heikki Linnakangas <[email protected]>
2026-06-05 14:08 ` Re: Fix domain fast defaults on empty tables Tom Lane <[email protected]>
@ 2026-06-08 08:20 ` jian he <[email protected]>
2 siblings, 0 replies; 11+ messages in thread
From: jian he @ 2026-06-08 08:20 UTC (permalink / raw)
To: Tom Lane <[email protected]>; +Cc: Heikki Linnakangas <[email protected]>; [email protected]; Chao Li <[email protected]>; Andrew Dunstan <[email protected]>
On Fri, Jun 5, 2026 at 10:08 PM Tom Lane <[email protected]> wrote:
>
> Heikki Linnakangas <[email protected]> writes:
> > On 5 June 2026 10:48:00 EEST, Chao Li <[email protected]> wrote:
> >> 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 seems totally reasonable to get an error in that case. '1' is not a valid value for the datatype, whether or not there are any rows in the table.
>
> I think there's reason for concern here, which is that we do not throw
> an error for the apparently equivalent case
>
> regression=# create table t2 (a int, b d_div default 1);
> CREATE TABLE
>
> This will give you an error at INSERT, but not CREATE. So this
> is inconsistent, as well as different from the pre-v19 behavior.
>
> Concretely, I'm pretty sure it is a hazard for pg_dump, which thinks
> it can freely transform bits of CREATE operations into ALTERs.
> I didn't try to make an example case, but I suspect it is now possible
> to create a database that will fail dump/restore because of this
> inconsistency.
Per the docs [1], we have two relevant forms:
ALTER [ COLUMN ] column_name SET DEFAULT expression
ADD [ COLUMN ] [ IF NOT EXISTS ] column_name data_type ... [ DEFAULT
default_expr ]
ATExecColumnDefault -> AddRelationNewConstraints does NOT evaluate the
default expression.
So ALTER COLUMN SET DEFAULT will never error out on a bad expression
because it is never evaluated at DDL time.
ALTER TABLE ADD COLUMN with a DEFAULT: pg_dump.c will never emit such a command.
You can verify this by searching for the keyword "add" in
src/bin/pg_dump/pg_dump.c, and looking at each occurrence one by one.
Looking at dumpTableSchema() confirms the same: we do not produce
command: ALTER TABLE ADD COLUMN.
See dumpTableSchema below comments related FOR LOOP part also
/*
* Dump additional per-column properties that we can't handle in the
* main CREATE TABLE command.
*/
[1] https://www.postgresql.org/docs/current/sql-altertable.html
--------------------------------------------
create or replace function dummy() returns numeric AS $$
BEGIN
RETURN 1/0;
END$$ immutable LANGUAGE plpgsql;
create table t1(a numeric default dummy());
alter table t1 add column b numeric default dummy();
ERROR: division by zero
CONTEXT: PL/pgSQL expression "1/0"
PL/pgSQL function dummy() line 3 at RETURN
As you can see, if pg_dump somehow converted the CREATE default expression into
an ALTER TABLE ADD COLUMN DEFAULT, we would already be facing this issue.
Am I missing something?
--
jian
https://www.enterprisedb.com/
^ permalink raw reply [nested|flat] 11+ messages in thread
end of thread, other threads:[~2026-06-08 18:12 UTC | newest]
Thread overview: 11+ messages (download: mbox mbox.gz follow: Atom feed)
-- links below jump to the message on this page --
2026-06-05 07:48 Fix domain fast defaults on empty tables Chao Li <[email protected]>
2026-06-05 09:04 ` Heikki Linnakangas <[email protected]>
2026-06-05 09:52 ` Chao Li <[email protected]>
2026-06-05 14:08 ` Tom Lane <[email protected]>
2026-06-06 12:41 ` Andrew Dunstan <[email protected]>
2026-06-06 20:19 ` Tom Lane <[email protected]>
2026-06-07 02:09 ` Chao Li <[email protected]>
2026-06-08 15:40 ` Andrew Dunstan <[email protected]>
2026-06-08 00:33 ` jian he <[email protected]>
2026-06-08 18:12 ` Tom Lane <[email protected]>
2026-06-08 08:20 ` jian he <[email protected]>
This inbox is served by agora; see mirroring instructions
for how to clone and mirror all data and code used for this inbox