hi.
new patch attached.
rewriteTargetListIU, expand_insert_targetlist these two places can
make a null Const TargetEntry for the generated column in an INSERT
operation.
but since this problem only occurs in INSERT, so i placed the logic
within expand_insert_targetlist would be appropriate?
The following are excerpts of the commit message.
--------------------------------
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
For INSERT operation, for Query->targetList, we should not make a
generated column
over domain with constraint to a CoerceToDomain node, instead, we make it as a
simple null Const over domain's base type.
When a column is a generated column in an INSERT, expand_insert_targetlist
should unconditionally generate a null Const to be inserted. If we are not
doing this way, we might end up wrapping the null Const in a CoerceToDomain
node, which may trigger runtime error earlier if the domain has a NOT NULL
constraint. That's not fine, as generated columns are already handled in
ExecComputeStoredGenerated.
--------------------------------
From 610ffedb4ee55207489397ad139a712c54dcdb1a Mon Sep 17 00:00:00 2001
From: jian he <[email protected]>
Date: Thu, 10 Apr 2025 16:47:37 +0800
Subject: [PATCH v3 1/1] fix INSERT generated column over domain with
constraints
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit
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
For INSERT operation, for Query->targetList, we should not make a generated column
over domain with constraint to a CoerceToDomain node, instead, we make it as a
simple null Const over domain's base type.
When a column is a generated column in an INSERT, expand_insert_targetlist
should unconditionally generate a null Const to be inserted. If we are not
doing this way, we might end up wrapping the null Const in a CoerceToDomain
node, which may trigger runtime error earlier if the domain has a NOT NULL
constraint. That's not fine, as generated columns are already handled in
ExecComputeStoredGenerated.
context: https://git.postgresql.org/cgit/postgresql.git/commit/?id=0da39aa7667b06e16189d318f7850d559d446d52
discussion: https://postgr.es/m/CACJufxG59tip2+9h=rev-ykofjt0cbspvchhi0rtij8babb...@mail.gmail.com
---
src/backend/executor/nodeModifyTable.c | 36 ++++++++++++++----
src/backend/optimizer/prep/preptlist.c | 37 ++++++++++++++++++-
.../regress/expected/generated_stored.out | 32 ++++++++++++++++
src/test/regress/sql/generated_stored.sql | 15 ++++++++
4 files changed, 112 insertions(+), 8 deletions(-)
diff --git a/src/backend/executor/nodeModifyTable.c b/src/backend/executor/nodeModifyTable.c
index 309e27f8b5f..5bd80456c3e 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,34 @@ 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.",
- format_type_be(attr->atttypid),
- attno,
- format_type_be(exprType((Node *) tle->expr)))));
+ {
+ 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
+ {
+ /* see rewriteTargetListIU comments for domain over generated column */
+ Oid baseTypeId;
+ int32 baseTypeMod = attr->atttypmod;
+ baseTypeId = getBaseTypeAndTypmod(attr->atttypid, &baseTypeMod);
+
+ if (exprType((Node *) tle->expr) != baseTypeId)
+ 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
{
diff --git a/src/backend/optimizer/prep/preptlist.c b/src/backend/optimizer/prep/preptlist.c
index c2a77503d4b..d28ea7ed071 100644
--- a/src/backend/optimizer/prep/preptlist.c
+++ b/src/backend/optimizer/prep/preptlist.c
@@ -44,6 +44,7 @@
#include "optimizer/tlist.h"
#include "parser/parse_coerce.h"
#include "parser/parsetree.h"
+#include "utils/lsyscache.h"
#include "utils/rel.h"
static List *expand_insert_targetlist(PlannerInfo *root, List *tlist,
@@ -434,7 +435,7 @@ expand_insert_targetlist(PlannerInfo *root, List *tlist, Relation rel)
*/
Node *new_expr;
- if (!att_tup->attisdropped)
+ if (!att_tup->attisdropped && att_tup->attgenerated == '\0')
{
new_expr = coerce_null_to_domain(att_tup->atttypid,
att_tup->atttypmod,
@@ -445,6 +446,40 @@ expand_insert_targetlist(PlannerInfo *root, List *tlist, Relation rel)
if (!IsA(new_expr, Const))
new_expr = eval_const_expressions(root, new_expr);
}
+ else if (!att_tup->attisdropped && att_tup->attgenerated != '\0')
+ {
+ /*
+ * During an INSERT, we first process the target list (tlist) via
+ * Result node for projection and input datatype checks. Later we
+ * do evaluation of generation expression in
+ * ExecComputeStoredGenerated. The target list (tlist) entries may
+ * have generated column over on domain types. Normally, this works
+ * fine.
+ *
+ * However, if that domain type has a NOT NULL constraint or a
+ * CHECK constraint is logically equivalent to NOT NULL, the
+ * INSERT may throw run-time error earlier due to domain not-null
+ * violation. This happens because generated column entries in
+ * the target list may initialized as null Const warpped inside
+ * CoerceToDomain node, and we actually evaulate it before
+ * ExecComputeStoredGenerated.
+ *
+ * To avoid it, the generated column's TargetEntry is
+ * unconditionally set as a null Const, So no need to worry about
+ * run-time error while evaluating CoerceToDomain earlier.
+ */
+ Oid baseTypeId;
+ int32 baseTypeMod = att_tup->atttypmod;
+
+ baseTypeId = getBaseTypeAndTypmod(att_tup->atttypid, &baseTypeMod);
+ new_expr = (Node *) makeConst(baseTypeId,
+ baseTypeMod,
+ att_tup->attcollation,
+ att_tup->attlen,
+ (Datum) 0,
+ true, /* isnull */
+ att_tup->attbyval);
+ }
else
{
/* Insert NULL for dropped column */
diff --git a/src/test/regress/expected/generated_stored.out b/src/test/regress/expected/generated_stored.out
index 8cccd1d7fe9..ffa844ca903 100644
--- a/src/test/regress/expected/generated_stored.out
+++ b/src/test/regress/expected/generated_stored.out
@@ -847,6 +847,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 50e94e5c673..ba8ae62dea0 100644
--- a/src/test/regress/sql/generated_stored.sql
+++ b/src/test/regress/sql/generated_stored.sql
@@ -419,6 +419,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
