Skip to content

Commit 7c87284

Browse files
tglsfdctglsfdc
committed
Fix failure for generated column with a not-null domain constraint.
If a GENERATED column is declared to have a domain data type where the domain's constraints disallow null values, INSERT commands failed because we built a targetlist that included coercing a null constant to the domain's type. The failure occurred even when the generated value would have been perfectly OK. This is adjacent to the issues fixed in 0da39aa, but we didn't notice for lack of testing a domain with such a constraint. We aren't going to use the result of the targetlist entry for the generated column --- ExecComputeStoredGenerated will overwrite it. So it's not really necessary that it have the exact datatype of the generated column. This patch fixes the problem by changing the targetlist entry to be a null Const of the domain's base type, which should be sufficiently legal. (We do have to tweak ExecCheckPlanOutput to accept the situation, though.) This has been broken since we implemented generated columns. However, this patch only applies easily as far back as v14, partly because I (tgl) only carried 0da39aa back that far, but mostly because v14 significantly refactored the handling of INSERT/UPDATE targetlists. Given the lack of field complaints and the short remaining support lifetime of v13, I judge the cost-benefit ratio not good for devising a version that would work in v13. Reported-by: jian he <jian.universality@gmail.com> Author: jian he <jian.universality@gmail.com> Reviewed-by: Tom Lane <tgl@sss.pgh.pa.us> Discussion: https://postgr.es/m/CACJufxG59tip2+9h=rEv-ykOFjt0cbsPVchhi0RTij8bABBA0Q@mail.gmail.com Backpatch-through: 14
1 parent f840f8e commit 7c87284

File tree

6 files changed

+90
-27
lines changed

6 files changed

+90
-27
lines changed

‎src/backend/executor/nodeModifyTable.c

Lines changed: 32 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -212,33 +212,53 @@ ExecCheckPlanOutput(Relation resultRel, List *targetList)
212212
attr = TupleDescAttr(resultDesc, attno);
213213
attno++;
214214

215-
if (!attr->attisdropped)
215+
/*
216+
* Special cases here should match planner's expand_insert_targetlist.
217+
*/
218+
if (attr->attisdropped)
216219
{
217-
/* Normal case: demand type match */
218-
if (exprType((Node *) tle->expr) != attr->atttypid)
220+
/*
221+
* For a dropped column, we can't check atttypid (it's likely 0).
222+
* In any case the planner has most likely inserted an INT4 null.
223+
* What we insist on is just *some* NULL constant.
224+
*/
225+
if (!IsA(tle->expr, Const) ||
226+
!((Const *) tle->expr)->constisnull)
219227
ereport(ERROR,
220228
(errcode(ERRCODE_DATATYPE_MISMATCH),
221229
errmsg("table row type and query-specified row type do not match"),
222-
errdetail("Table has type %s at ordinal position %d, but query expects %s.",
223-
format_type_be(attr->atttypid),
224-
attno,
225-
format_type_be(exprType((Node *) tle->expr)))));
230+
errdetail("Query provides a value for a dropped column at ordinal position %d.",
231+
attno)));
226232
}
227-
else
233+
else if (attr->attgenerated)
228234
{
229235
/*
230-
* For a dropped column, we can't check atttypid (it's likely 0).
231-
* In any case the planner has most likely inserted an INT4 null.
232-
* What we insist on is just *some* NULL constant.
236+
* For a generated column, the planner will have inserted a null
237+
* of the column's base type (to avoid possibly failing on domain
238+
* not-null constraints). It doesn't seem worth insisting on that
239+
* exact type though, since a null value is type-independent. As
240+
* above, just insist on *some* NULL constant.
233241
*/
234242
if (!IsA(tle->expr, Const) ||
235243
!((Const *) tle->expr)->constisnull)
236244
ereport(ERROR,
237245
(errcode(ERRCODE_DATATYPE_MISMATCH),
238246
errmsg("table row type and query-specified row type do not match"),
239-
errdetail("Query provides a value for a dropped column at ordinal position %d.",
247+
errdetail("Query provides a value for a generated column at ordinal position %d.",
240248
attno)));
241249
}
250+
else
251+
{
252+
/* Normal case: demand type match */
253+
if (exprType((Node *) tle->expr) != attr->atttypid)
254+
ereport(ERROR,
255+
(errcode(ERRCODE_DATATYPE_MISMATCH),
256+
errmsg("table row type and query-specified row type do not match"),
257+
errdetail("Table has type %s at ordinal position %d, but query expects %s.",
258+
format_type_be(attr->atttypid),
259+
attno,
260+
format_type_be(exprType((Node *) tle->expr)))));
261+
}
242262
}
243263
if (attno != resultDesc->natts)
244264
ereport(ERROR,

‎src/backend/optimizer/prep/preptlist.c

Lines changed: 38 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -44,6 +44,7 @@
4444
#include "optimizer/tlist.h"
4545
#include "parser/parse_coerce.h"
4646
#include "parser/parsetree.h"
47+
#include "utils/lsyscache.h"
4748
#include "utils/rel.h"
4849

4950
static List *expand_insert_targetlist(PlannerInfo *root, List *tlist,
@@ -419,9 +420,8 @@ expand_insert_targetlist(PlannerInfo *root, List *tlist, Relation rel)
419420
*
420421
* INSERTs should insert NULL in this case. (We assume the
421422
* rewriter would have inserted any available non-NULL default
422-
* value.) Also, if the column isn't dropped, apply any domain
423-
* constraints that might exist --- this is to catch domain NOT
424-
* NULL.
423+
* value.) Also, normally we must apply any domain constraints
424+
* that might exist --- this is to catch domain NOT NULL.
425425
*
426426
* When generating a NULL constant for a dropped column, we label
427427
* it INT4 (any other guaranteed-to-exist datatype would do as
@@ -431,21 +431,17 @@ expand_insert_targetlist(PlannerInfo *root, List *tlist, Relation rel)
431431
* representation is datatype-independent. This could perhaps
432432
* confuse code comparing the finished plan to the target
433433
* relation, however.
434+
*
435+
* Another exception is that if the column is generated, the value
436+
* we produce here will be ignored, and we don't want to risk
437+
* throwing an error. So in that case we *don't* want to apply
438+
* domain constraints, so we must produce a NULL of the base type.
439+
* Again, code comparing the finished plan to the target relation
440+
* must account for this.
434441
*/
435442
Node *new_expr;
436443

437-
if (!att_tup->attisdropped)
438-
{
439-
new_expr = coerce_null_to_domain(att_tup->atttypid,
440-
att_tup->atttypmod,
441-
att_tup->attcollation,
442-
att_tup->attlen,
443-
att_tup->attbyval);
444-
/* Must run expression preprocessing on any non-const nodes */
445-
if (!IsA(new_expr, Const))
446-
new_expr = eval_const_expressions(root, new_expr);
447-
}
448-
else
444+
if (att_tup->attisdropped)
449445
{
450446
/* Insert NULL for dropped column */
451447
new_expr = (Node *) makeConst(INT4OID,
@@ -456,6 +452,33 @@ expand_insert_targetlist(PlannerInfo *root, List *tlist, Relation rel)
456452
true, /* isnull */
457453
true /* byval */ );
458454
}
455+
else if (att_tup->attgenerated)
456+
{
457+
/* Generated column, insert a NULL of the base type */
458+
Oid baseTypeId = att_tup->atttypid;
459+
int32 baseTypeMod = att_tup->atttypmod;
460+
461+
baseTypeId = getBaseTypeAndTypmod(baseTypeId, &baseTypeMod);
462+
new_expr = (Node *) makeConst(baseTypeId,
463+
baseTypeMod,
464+
att_tup->attcollation,
465+
att_tup->attlen,
466+
(Datum) 0,
467+
true, /* isnull */
468+
att_tup->attbyval);
469+
}
470+
else
471+
{
472+
/* Normal column, insert a NULL of the column datatype */
473+
new_expr = coerce_null_to_domain(att_tup->atttypid,
474+
att_tup->atttypmod,
475+
att_tup->attcollation,
476+
att_tup->attlen,
477+
att_tup->attbyval);
478+
/* Must run expression preprocessing on any non-const nodes */
479+
if (!IsA(new_expr, Const))
480+
new_expr = eval_const_expressions(root, new_expr);
481+
}
459482

460483
new_tle = makeTargetEntry((Expr *) new_expr,
461484
attrno,

‎src/test/regress/expected/generated_stored.out

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -847,6 +847,11 @@ CREATE TABLE gtest24r (a int PRIMARY KEY, b gtestdomain1range GENERATED ALWAYS A
847847
INSERT INTO gtest24r (a) VALUES (4); -- ok
848848
INSERT INTO gtest24r (a) VALUES (6); -- error
849849
ERROR: value for domain gtestdomain1 violates check constraint "gtestdomain1_check"
850+
CREATE DOMAIN gtestdomainnn AS int CHECK (VALUE IS NOT NULL);
851+
CREATE TABLE gtest24nn (a int, b gtestdomainnn GENERATED ALWAYS AS (a * 2) STORED);
852+
INSERT INTO gtest24nn (a) VALUES (4); -- ok
853+
INSERT INTO gtest24nn (a) VALUES (NULL); -- error
854+
ERROR: value for domain gtestdomainnn violates check constraint "gtestdomainnn_check"
850855
-- typed tables (currently not supported)
851856
CREATE TYPE gtest_type AS (f1 integer, f2 text, f3 bigint);
852857
CREATE TABLE gtest28 OF gtest_type (f1 WITH OPTIONS GENERATED ALWAYS AS (f2 *2) STORED);

‎src/test/regress/expected/generated_virtual.out

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -800,6 +800,11 @@ CREATE TABLE gtest24r (a int PRIMARY KEY, b gtestdomain1range GENERATED ALWAYS A
800800
ERROR: virtual generated column "b" cannot have a domain type
801801
--INSERT INTO gtest24r (a) VALUES (4); -- ok
802802
--INSERT INTO gtest24r (a) VALUES (6); -- error
803+
CREATE DOMAIN gtestdomainnn AS int CHECK (VALUE IS NOT NULL);
804+
CREATE TABLE gtest24nn (a int, b gtestdomainnn GENERATED ALWAYS AS (a * 2) VIRTUAL);
805+
ERROR: virtual generated column "b" cannot have a domain type
806+
--INSERT INTO gtest24nn (a) VALUES (4); -- ok
807+
--INSERT INTO gtest24nn (a) VALUES (NULL); -- error
803808
-- typed tables (currently not supported)
804809
CREATE TYPE gtest_type AS (f1 integer, f2 text, f3 bigint);
805810
CREATE TABLE gtest28 OF gtest_type (f1 WITH OPTIONS GENERATED ALWAYS AS (f2 *2) VIRTUAL);

‎src/test/regress/sql/generated_stored.sql

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -419,6 +419,11 @@ CREATE TABLE gtest24r (a int PRIMARY KEY, b gtestdomain1range GENERATED ALWAYS A
419419
INSERT INTO gtest24r (a) VALUES (4); -- ok
420420
INSERT INTO gtest24r (a) VALUES (6); -- error
421421

422+
CREATE DOMAIN gtestdomainnn AS int CHECK (VALUE IS NOT NULL);
423+
CREATE TABLE gtest24nn (a int, b gtestdomainnn GENERATED ALWAYS AS (a * 2) STORED);
424+
INSERT INTO gtest24nn (a) VALUES (4); -- ok
425+
INSERT INTO gtest24nn (a) VALUES (NULL); -- error
426+
422427
-- typed tables (currently not supported)
423428
CREATE TYPE gtest_type AS (f1 integer, f2 text, f3 bigint);
424429
CREATE TABLE gtest28 OF gtest_type (f1 WITH OPTIONS GENERATED ALWAYS AS (f2 *2) STORED);

‎src/test/regress/sql/generated_virtual.sql

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -453,6 +453,11 @@ CREATE TABLE gtest24r (a int PRIMARY KEY, b gtestdomain1range GENERATED ALWAYS A
453453
--INSERT INTO gtest24r (a) VALUES (4); -- ok
454454
--INSERT INTO gtest24r (a) VALUES (6); -- error
455455

456+
CREATE DOMAIN gtestdomainnn AS int CHECK (VALUE IS NOT NULL);
457+
CREATE TABLE gtest24nn (a int, b gtestdomainnn GENERATED ALWAYS AS (a * 2) VIRTUAL);
458+
--INSERT INTO gtest24nn (a) VALUES (4); -- ok
459+
--INSERT INTO gtest24nn (a) VALUES (NULL); -- error
460+
456461
-- typed tables (currently not supported)
457462
CREATE TYPE gtest_type AS (f1 integer, f2 text, f3 bigint);
458463
CREATE TABLE gtest28 OF gtest_type (f1 WITH OPTIONS GENERATED ALWAYS AS (f2 *2) VIRTUAL);

0 commit comments

Comments
 (0)