hi.
create domain d1 as int not null;
create domain d2 as int check (value > 1);
create domain d3 as int check (value is not null);
create table t0(b int, a d3 GENERATED ALWAYS as (b + 11) stored);
insert into t0 values (1, default);
ERROR: value for domain d3 violates check constraint "d3_check"
in the above example, a domain with check constraint not working as intended,
similarly, a domain with not-null constraint will also not work.
now table t0 can not insert any data, because of check constraint violation.
so i think this is a bug, (hope i didn't miss anything).
in ExecBuildProjectionInfo, we compile "values (1, default)" as
targetlist expression (CONST, COERCETODOMAIN)
Then in ExecResult, ExecProject, ExecInterpExpr we evaluate the
compiled expression;
we failed at ExecEvalConstraintCheck. we are acting like: ``null::d3``.
explain(costs off, verbose) insert into t0 values (1, default);
QUERY PLAN
----------------------------------------
Insert on public.t0
-> Result
Output: 1, NULL::integer
the plan is ``NULL::integer``, which will not fail, but ``NULL::d3`` will fail.
that means, the output plan is right. it's the execution wrong?
the main fix should be in rewriteTargetListIU.
UPDATE don't have this issue, since there is no Result node,
ExecComputeStoredGenerated will do the domain constraint check.
related:
https://git.postgresql.org/cgit/postgresql.git/commit/?id=0da39aa7667b06e16189d318f7850d559d446d52
From 1820d039b472ea71d3b80b98096a81bb5fc3857b Mon Sep 17 00:00:00 2001
From: jian he <jian.universal...@gmail.com>
Date: Wed, 26 Feb 2025 01:05:46 +0800
Subject: [PATCH v1 1/1] fix default insertion for stored generated column with
domain over constraints.
create domain d3 as int check (value is not null);
create table t0(b int, a d3 GENERATED ALWAYS as (b + 11) stored);
insert into t0 values (1, default);
ERROR: value for domain d3 violates check constraint "d3_check"
explain(costs off, verbose) insert into t0 values (1, default);
QUERY PLAN
---------------------------------------
Insert on public.t0
-> Result
Output: 1, NULL::integer
We first evaluate the Result node. Domain coercion in the Result node may lead
to domain constraint violations. These domain constraints should not be checked in
ExecResult, as ExecComputeStoredGenerated will handle them.
context: https://git.postgresql.org/cgit/postgresql.git/commit/?id=0da39aa7667b06e16189d318f7850d559d446d52
discussion: https://postgr.es/m/
---
src/backend/executor/nodeModifyTable.c | 31 +++++++++++++++---
src/backend/optimizer/prep/preptlist.c | 3 +-
src/backend/parser/parse_coerce.c | 5 +--
src/backend/rewrite/rewriteHandler.c | 23 +++++++++++--
src/backend/rewrite/rewriteManip.c | 3 +-
src/include/parser/parse_coerce.h | 2 +-
.../regress/expected/generated_stored.out | 32 +++++++++++++++++++
src/test/regress/sql/generated_stored.sql | 15 +++++++++
8 files changed, 102 insertions(+), 12 deletions(-)
diff --git a/src/backend/executor/nodeModifyTable.c b/src/backend/executor/nodeModifyTable.c
index b0fe50075ad..1db7c1e45b6 100644
--- a/src/backend/executor/nodeModifyTable.c
+++ b/src/backend/executor/nodeModifyTable.c
@@ -67,6 +67,7 @@
#include "storage/lmgr.h"
#include "utils/builtins.h"
#include "utils/datum.h"
+#include "utils/lsyscache.h"
#include "utils/rel.h"
#include "utils/snapmgr.h"
@@ -216,13 +217,33 @@ ExecCheckPlanOutput(Relation resultRel, List *targetList)
{
/* Normal case: demand type match */
if (exprType((Node *) tle->expr) != attr->atttypid)
- ereport(ERROR,
- (errcode(ERRCODE_DATATYPE_MISMATCH),
- errmsg("table row type and query-specified row type do not match"),
- errdetail("Table has type %s at ordinal position %d, but query expects %s.",
+ {
+ if (attr->attgenerated == '\0')
+ {
+ ereport(ERROR,
+ errcode(ERRCODE_DATATYPE_MISMATCH),
+ errmsg("table row type and query-specified row type do not match"),
+ errdetail("Table has type %s at ordinal position %d, but query expects %s.",
+ format_type_be(attr->atttypid),
+ attno,
+ format_type_be(exprType((Node *) tle->expr))));
+ }
+ else
+ {
+ /*XXX explain why this is needed */
+ Oid baseTypeId;
+ int32 baseTypeMod = attr->atttypmod;
+ baseTypeId = getBaseTypeAndTypmod(attr->atttypid, &baseTypeMod);
+
+ if (exprType((Node *) tle->expr) != baseTypeId)
+ errcode(ERRCODE_DATATYPE_MISMATCH),
+ errmsg("table row type and query-specified row type do not match"),
+ errdetail("Table has domain type %s at ordinal position %d, but query expects %s.",
format_type_be(attr->atttypid),
attno,
- format_type_be(exprType((Node *) tle->expr)))));
+ format_type_be(exprType((Node *) tle->expr)));
+ }
+ }
}
else
{
diff --git a/src/backend/optimizer/prep/preptlist.c b/src/backend/optimizer/prep/preptlist.c
index c2a77503d4b..769ff600eaf 100644
--- a/src/backend/optimizer/prep/preptlist.c
+++ b/src/backend/optimizer/prep/preptlist.c
@@ -440,7 +440,8 @@ expand_insert_targetlist(PlannerInfo *root, List *tlist, Relation rel)
att_tup->atttypmod,
att_tup->attcollation,
att_tup->attlen,
- att_tup->attbyval);
+ att_tup->attbyval,
+ true);
/* Must run expression preprocessing on any non-const nodes */
if (!IsA(new_expr, Const))
new_expr = eval_const_expressions(root, new_expr);
diff --git a/src/backend/parser/parse_coerce.c b/src/backend/parser/parse_coerce.c
index 0b5b81c7f27..7c10dbdb424 100644
--- a/src/backend/parser/parse_coerce.c
+++ b/src/backend/parser/parse_coerce.c
@@ -1266,10 +1266,11 @@ coerce_to_specific_type(ParseState *pstate, Node *node,
* Build a NULL constant, then wrap it in CoerceToDomain
* if the desired type is a domain type. This allows any
* NOT NULL domain constraint to be enforced at runtime.
+ * XXX explain need_coerce, maybe change to a better name.
*/
Node *
coerce_null_to_domain(Oid typid, int32 typmod, Oid collation,
- int typlen, bool typbyval)
+ int typlen, bool typbyval, bool need_coerce)
{
Node *result;
Oid baseTypeId;
@@ -1287,7 +1288,7 @@ coerce_null_to_domain(Oid typid, int32 typmod, Oid collation,
(Datum) 0,
true, /* isnull */
typbyval);
- if (typid != baseTypeId)
+ if (typid != baseTypeId && need_coerce)
result = coerce_to_domain(result,
baseTypeId, baseTypeMod,
typid,
diff --git a/src/backend/rewrite/rewriteHandler.c b/src/backend/rewrite/rewriteHandler.c
index f0bce5f9ed9..a0481ad6000 100644
--- a/src/backend/rewrite/rewriteHandler.c
+++ b/src/backend/rewrite/rewriteHandler.c
@@ -990,8 +990,25 @@ rewriteTargetListIU(List *targetList,
/*
* virtual generated column stores a null value; stored generated
* column will be fixed in executor
+ * XXX comments explain why we need special deal with INSERT
*/
new_tle = NULL;
+ if (apply_default && commandType == CMD_INSERT)
+ {
+ Node *new_expr;
+
+ new_expr = coerce_null_to_domain(att_tup->atttypid,
+ att_tup->atttypmod,
+ att_tup->attcollation,
+ att_tup->attlen,
+ att_tup->attbyval,
+ att_tup->attgenerated == '\0');
+ if (new_expr)
+ new_tle = makeTargetEntry((Expr *) new_expr,
+ attrno,
+ pstrdup(NameStr(att_tup->attname)),
+ false);
+ }
}
else if (apply_default)
{
@@ -1015,7 +1032,8 @@ rewriteTargetListIU(List *targetList,
att_tup->atttypmod,
att_tup->attcollation,
att_tup->attlen,
- att_tup->attbyval);
+ att_tup->attbyval,
+ true);
}
if (new_expr)
@@ -1567,7 +1585,8 @@ rewriteValuesRTE(Query *parsetree, RangeTblEntry *rte, int rti,
att_tup->atttypmod,
att_tup->attcollation,
att_tup->attlen,
- att_tup->attbyval);
+ att_tup->attbyval,
+ true);
}
newList = lappend(newList, new_expr);
}
diff --git a/src/backend/rewrite/rewriteManip.c b/src/backend/rewrite/rewriteManip.c
index 98a265cd3d5..09d54d39632 100644
--- a/src/backend/rewrite/rewriteManip.c
+++ b/src/backend/rewrite/rewriteManip.c
@@ -1942,7 +1942,8 @@ ReplaceVarFromTargetList(Var *var,
var->vartypmod,
var->varcollid,
vartyplen,
- vartypbyval);
+ vartypbyval,
+ true);
}
}
elog(ERROR, "could not find replacement targetlist entry for attno %d",
diff --git a/src/include/parser/parse_coerce.h b/src/include/parser/parse_coerce.h
index 8d775c72c59..1bb19c82e1f 100644
--- a/src/include/parser/parse_coerce.h
+++ b/src/include/parser/parse_coerce.h
@@ -64,7 +64,7 @@ extern Node *coerce_to_specific_type_typmod(ParseState *pstate, Node *node,
const char *constructName);
extern Node *coerce_null_to_domain(Oid typid, int32 typmod, Oid collation,
- int typlen, bool typbyval);
+ int typlen, bool typbyval, bool need_coerce);
extern int parser_coercion_errposition(ParseState *pstate,
int coerce_location,
diff --git a/src/test/regress/expected/generated_stored.out b/src/test/regress/expected/generated_stored.out
index 3ce0dd1831c..e1b436a2c02 100644
--- a/src/test/regress/expected/generated_stored.out
+++ b/src/test/regress/expected/generated_stored.out
@@ -843,6 +843,38 @@ CREATE TABLE gtest24r (a int PRIMARY KEY, b gtestdomain1range GENERATED ALWAYS A
INSERT INTO gtest24r (a) VALUES (4); -- ok
INSERT INTO gtest24r (a) VALUES (6); -- error
ERROR: value for domain gtestdomain1 violates check constraint "gtestdomain1_check"
+CREATE DOMAIN dnn AS int NOT NULL;
+CREATE DOMAIN dnn_check AS int CHECK (value IS NOT NULL);
+CREATE TABLE gtest24nn (
+ a int,
+ b dnn GENERATED ALWAYS AS (a + 1) STORED,
+ c dnn GENERATED ALWAYS AS (11) STORED,
+ d dnn_check GENERATED ALWAYS AS (a + 11) STORED
+);
+EXPLAIN (COSTS OFF, VERBOSE) INSERT INTO gtest24nn VALUES (1, DEFAULT, DEFAULT, DEFAULT); --ok
+ QUERY PLAN
+----------------------------------------------------------------
+ Insert on generated_stored_tests.gtest24nn
+ -> Result
+ Output: 1, NULL::integer, NULL::integer, NULL::integer
+(3 rows)
+
+INSERT INTO gtest24nn VALUES (NULL, DEFAULT, DEFAULT, DEFAULT); --error
+ERROR: domain dnn does not allow null values
+INSERT INTO gtest24nn VALUES (1, DEFAULT, DEFAULT, DEFAULT) RETURNING *; --ok
+ a | b | c | d
+---+---+----+----
+ 1 | 2 | 11 | 12
+(1 row)
+
+UPDATE gtest24nn SET b = DEFAULT, c = default, d = default, a = 2 WHERE a = 1 RETURNING *; --ok
+ a | b | c | d
+---+---+----+----
+ 2 | 3 | 11 | 13
+(1 row)
+
+UPDATE gtest24nn SET a = NULL WHERE a = 2 RETURNING *; --error
+ERROR: domain dnn does not allow null values
-- typed tables (currently not supported)
CREATE TYPE gtest_type AS (f1 integer, f2 text, f3 bigint);
CREATE TABLE gtest28 OF gtest_type (f1 WITH OPTIONS GENERATED ALWAYS AS (f2 *2) STORED);
diff --git a/src/test/regress/sql/generated_stored.sql b/src/test/regress/sql/generated_stored.sql
index b7749ce355f..f7988a74acd 100644
--- a/src/test/regress/sql/generated_stored.sql
+++ b/src/test/regress/sql/generated_stored.sql
@@ -416,6 +416,21 @@ CREATE TABLE gtest24r (a int PRIMARY KEY, b gtestdomain1range GENERATED ALWAYS A
INSERT INTO gtest24r (a) VALUES (4); -- ok
INSERT INTO gtest24r (a) VALUES (6); -- error
+CREATE DOMAIN dnn AS int NOT NULL;
+CREATE DOMAIN dnn_check AS int CHECK (value IS NOT NULL);
+CREATE TABLE gtest24nn (
+ a int,
+ b dnn GENERATED ALWAYS AS (a + 1) STORED,
+ c dnn GENERATED ALWAYS AS (11) STORED,
+ d dnn_check GENERATED ALWAYS AS (a + 11) STORED
+);
+
+EXPLAIN (COSTS OFF, VERBOSE) INSERT INTO gtest24nn VALUES (1, DEFAULT, DEFAULT, DEFAULT); --ok
+INSERT INTO gtest24nn VALUES (NULL, DEFAULT, DEFAULT, DEFAULT); --error
+INSERT INTO gtest24nn VALUES (1, DEFAULT, DEFAULT, DEFAULT) RETURNING *; --ok
+UPDATE gtest24nn SET b = DEFAULT, c = default, d = default, a = 2 WHERE a = 1 RETURNING *; --ok
+UPDATE gtest24nn SET a = NULL WHERE a = 2 RETURNING *; --error
+
-- typed tables (currently not supported)
CREATE TYPE gtest_type AS (f1 integer, f2 text, f3 bigint);
CREATE TABLE gtest28 OF gtest_type (f1 WITH OPTIONS GENERATED ALWAYS AS (f2 *2) STORED);
--
2.34.1
