Movatterモバイル変換

[0]ホーム
URL:

Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

 Sign up
Appearance settings

Commit6655d93

Browse files
committed
Handle default NULL insertion a little better.
If a column is omitted in an INSERT, and there's no column default,the code in preptlist.c generates a NULL Const to be inserted.Furthermore, if the column is of a domain type, we wrap the Constin CoerceToDomain, so as to throw a run-time error if the domainhas a NOT NULL constraint. That's fine as far as it goes, butthere are two problems:1. We're being sloppy about the type/typmod that the Const islabeled with. It really should have the domain's base type/typmod,since it's the input to CoerceToDomain not the output. This canresult in coerce_to_domain inserting a useless length-coercionfunction (useless because it's being applied to a null). Thecoercion would typically get const-folded away later, but it'dbe better not to create it in the first place.2. We're not applying expression preprocessing (specifically,eval_const_expressions) to the resulting expression tree.The planner's primary expression-preprocessing pass already happened,so that means the length coercion step and CoerceToDomain node misspreprocessing altogether.This is at the least inefficient, since it means the length coercionand CoerceToDomain will actually be executed for each inserted row,though they could be const-folded away in most cases. Worse, itseems possible that missing preprocessing for the length coercioncould result in an invalid plan (for example, due to failing toperform default-function-argument insertion). I'm not aware ofany live bug of that sort with core datatypes, and it might beunreachable for extension types as well because of restrictions ofCREATE CAST, but I'm not entirely convinced that it's unreachable.Hence, it seems worth back-patching the fix (although I only wentback to v14, as the patch doesn't apply cleanly at all in v13).There are several places in the rewriter that are building nulldomain constants the same way as preptlist.c. While those arebefore the planner and hence don't have any reachable bug, they'restill applying a length coercion that will be const-folded awaylater, uselessly wasting cycles. Hence, make a utility routinethat all of these places can call to do it right.Making this code more careful about the typmod assigned to thegenerated NULL constant has visible but cosmetic effects on someof the plans shown in contrib/postgres_fdw's regression tests.Discussion:https://postgr.es/m/1865579.1738113656@sss.pgh.pa.usBackpatch-through: 14
1 parent998c4fc commit6655d93

File tree

6 files changed

+92
-78
lines changed

6 files changed

+92
-78
lines changed

‎contrib/postgres_fdw/expected/postgres_fdw.out

Lines changed: 12 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -4292,13 +4292,13 @@ EXPLAIN (VERBOSE, COSTS OFF) EXECUTE st6;
42924292

42934293
PREPARE st7 AS INSERT INTO ft1 (c1,c2,c3) VALUES (1001,101,'foo');
42944294
EXPLAIN (VERBOSE, COSTS OFF) EXECUTE st7;
4295-
QUERY PLAN
4296-
-------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------
4295+
QUERY PLAN
4296+
-----------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------
42974297
Insert on public.ft1
42984298
Remote SQL: INSERT INTO "S 1"."T 1"("C 1", c2, c3, c4, c5, c6, c7, c8) VALUES ($1, $2, $3, $4, $5, $6, $7, $8)
42994299
Batch Size: 1
43004300
-> Result
4301-
Output: NULL::integer, 1001, 101, 'foo'::text, NULL::timestamp with time zone, NULL::timestamp without time zone, NULL::character varying, 'ft1 '::character(10), NULL::user_enum
4301+
Output: NULL::integer, 1001, 101, 'foo'::text, NULL::timestamp with time zone, NULL::timestamp without time zone, NULL::character varying(10), 'ft1 '::character(10), NULL::user_enum
43024302
(5 rows)
43034303

43044304
ALTER TABLE "S 1"."T 1" RENAME TO "T 0";
@@ -4326,13 +4326,13 @@ EXECUTE st6;
43264326
(9 rows)
43274327

43284328
EXPLAIN (VERBOSE, COSTS OFF) EXECUTE st7;
4329-
QUERY PLAN
4330-
-------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------
4329+
QUERY PLAN
4330+
-----------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------
43314331
Insert on public.ft1
43324332
Remote SQL: INSERT INTO "S 1"."T 0"("C 1", c2, c3, c4, c5, c6, c7, c8) VALUES ($1, $2, $3, $4, $5, $6, $7, $8)
43334333
Batch Size: 1
43344334
-> Result
4335-
Output: NULL::integer, 1001, 101, 'foo'::text, NULL::timestamp with time zone, NULL::timestamp without time zone, NULL::character varying, 'ft1 '::character(10), NULL::user_enum
4335+
Output: NULL::integer, 1001, 101, 'foo'::text, NULL::timestamp with time zone, NULL::timestamp without time zone, NULL::character varying(10), 'ft1 '::character(10), NULL::user_enum
43364336
(5 rows)
43374337

43384338
ALTER TABLE "S 1"."T 0" RENAME TO "T 1";
@@ -4703,13 +4703,13 @@ explain (verbose, costs off) select * from ft3 f, loct3 l
47034703
-- ===================================================================
47044704
EXPLAIN (verbose, costs off)
47054705
INSERT INTO ft2 (c1,c2,c3) SELECT c1+1000,c2+100, c3 || c3 FROM ft2 LIMIT 20;
4706-
QUERY PLAN
4707-
--------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------
4706+
QUERY PLAN
4707+
------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------
47084708
Insert on public.ft2
47094709
Remote SQL: INSERT INTO "S 1"."T 1"("C 1", c2, c3, c4, c5, c6, c7, c8) VALUES ($1, $2, $3, $4, $5, $6, $7, $8)
47104710
Batch Size: 1
47114711
-> Subquery Scan on "*SELECT*"
4712-
Output: "*SELECT*"."?column?", "*SELECT*"."?column?_1", NULL::integer, "*SELECT*"."?column?_2", NULL::timestamp with time zone, NULL::timestamp without time zone, NULL::character varying, 'ft2 '::character(10), NULL::user_enum
4712+
Output: "*SELECT*"."?column?", "*SELECT*"."?column?_1", NULL::integer, "*SELECT*"."?column?_2", NULL::timestamp with time zone, NULL::timestamp without time zone, NULL::character varying(10), 'ft2 '::character(10), NULL::user_enum
47134713
-> Foreign Scan on public.ft2 ft2_1
47144714
Output: (ft2_1.c1 + 1000), (ft2_1.c2 + 100), (ft2_1.c3 || ft2_1.c3)
47154715
Remote SQL: SELECT "C 1", c2, c3 FROM "S 1"."T 1" LIMIT 20::bigint
@@ -5819,14 +5819,14 @@ SELECT c1,c2,c3,c4 FROM ft2 ORDER BY c1;
58195819

58205820
EXPLAIN (verbose, costs off)
58215821
INSERT INTO ft2 (c1,c2,c3) VALUES (1200,999,'foo') RETURNING tableoid::regclass;
5822-
QUERY PLAN
5823-
-------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------
5822+
QUERY PLAN
5823+
-----------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------
58245824
Insert on public.ft2
58255825
Output: (ft2.tableoid)::regclass
58265826
Remote SQL: INSERT INTO "S 1"."T 1"("C 1", c2, c3, c4, c5, c6, c7, c8) VALUES ($1, $2, $3, $4, $5, $6, $7, $8)
58275827
Batch Size: 1
58285828
-> Result
5829-
Output: 1200, 999, NULL::integer, 'foo'::text, NULL::timestamp with time zone, NULL::timestamp without time zone, NULL::character varying, 'ft2 '::character(10), NULL::user_enum
5829+
Output: 1200, 999, NULL::integer, 'foo'::text, NULL::timestamp with time zone, NULL::timestamp without time zone, NULL::character varying(10), 'ft2 '::character(10), NULL::user_enum
58305830
(6 rows)
58315831

58325832
INSERT INTO ft2 (c1,c2,c3) VALUES (1200,999,'foo') RETURNING tableoid::regclass;

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

Lines changed: 14 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -46,7 +46,8 @@
4646
#include"parser/parsetree.h"
4747
#include"utils/rel.h"
4848

49-
staticList*expand_insert_targetlist(List*tlist,Relationrel);
49+
staticList*expand_insert_targetlist(PlannerInfo*root,List*tlist,
50+
Relationrel);
5051

5152

5253
/*
@@ -102,7 +103,7 @@ preprocess_targetlist(PlannerInfo *root)
102103
*/
103104
tlist=parse->targetList;
104105
if (command_type==CMD_INSERT)
105-
tlist=expand_insert_targetlist(tlist,target_relation);
106+
tlist=expand_insert_targetlist(root,tlist,target_relation);
106107
elseif (command_type==CMD_UPDATE)
107108
root->update_colnos=extract_update_targetlist_colnos(tlist);
108109

@@ -148,7 +149,8 @@ preprocess_targetlist(PlannerInfo *root)
148149
ListCell*l2;
149150

150151
if (action->commandType==CMD_INSERT)
151-
action->targetList=expand_insert_targetlist(action->targetList,
152+
action->targetList=expand_insert_targetlist(root,
153+
action->targetList,
152154
target_relation);
153155
elseif (action->commandType==CMD_UPDATE)
154156
action->updateColnos=
@@ -352,7 +354,7 @@ extract_update_targetlist_colnos(List *tlist)
352354
* but now this code is only applied to INSERT targetlists.
353355
*/
354356
staticList*
355-
expand_insert_targetlist(List*tlist,Relationrel)
357+
expand_insert_targetlist(PlannerInfo*root,List*tlist,Relationrel)
356358
{
357359
List*new_tlist=NIL;
358360
ListCell*tlist_item;
@@ -406,26 +408,18 @@ expand_insert_targetlist(List *tlist, Relation rel)
406408
* confuse code comparing the finished plan to the target
407409
* relation, however.
408410
*/
409-
Oidatttype=att_tup->atttypid;
410-
Oidattcollation=att_tup->attcollation;
411411
Node*new_expr;
412412

413413
if (!att_tup->attisdropped)
414414
{
415-
new_expr= (Node*)makeConst(atttype,
416-
-1,
417-
attcollation,
418-
att_tup->attlen,
419-
(Datum)0,
420-
true,/* isnull */
421-
att_tup->attbyval);
422-
new_expr=coerce_to_domain(new_expr,
423-
InvalidOid,-1,
424-
atttype,
425-
COERCION_IMPLICIT,
426-
COERCE_IMPLICIT_CAST,
427-
-1,
428-
false);
415+
new_expr=coerce_null_to_domain(att_tup->atttypid,
416+
att_tup->atttypmod,
417+
att_tup->attcollation,
418+
att_tup->attlen,
419+
att_tup->attbyval);
420+
/* Must run expression preprocessing on any non-const nodes */
421+
if (!IsA(new_expr,Const))
422+
new_expr=eval_const_expressions(root,new_expr);
429423
}
430424
else
431425
{

‎src/backend/parser/parse_coerce.c

Lines changed: 37 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1263,6 +1263,43 @@ coerce_to_specific_type(ParseState *pstate, Node *node,
12631263
constructName);
12641264
}
12651265

1266+
/*
1267+
* coerce_null_to_domain()
1268+
*Build a NULL constant, then wrap it in CoerceToDomain
1269+
*if the desired type is a domain type. This allows any
1270+
*NOT NULL domain constraint to be enforced at runtime.
1271+
*/
1272+
Node*
1273+
coerce_null_to_domain(Oidtypid,int32typmod,Oidcollation,
1274+
inttyplen,booltypbyval)
1275+
{
1276+
Node*result;
1277+
OidbaseTypeId;
1278+
int32baseTypeMod=typmod;
1279+
1280+
/*
1281+
* The constant must appear to have the domain's base type/typmod, else
1282+
* coerce_to_domain() will apply a length coercion which is useless.
1283+
*/
1284+
baseTypeId=getBaseTypeAndTypmod(typid,&baseTypeMod);
1285+
result= (Node*)makeConst(baseTypeId,
1286+
baseTypeMod,
1287+
collation,
1288+
typlen,
1289+
(Datum)0,
1290+
true,/* isnull */
1291+
typbyval);
1292+
if (typid!=baseTypeId)
1293+
result=coerce_to_domain(result,
1294+
baseTypeId,baseTypeMod,
1295+
typid,
1296+
COERCION_IMPLICIT,
1297+
COERCE_IMPLICIT_CAST,
1298+
-1,
1299+
false);
1300+
returnresult;
1301+
}
1302+
12661303
/*
12671304
* parser_coercion_errposition - report coercion error location, if possible
12681305
*

‎src/backend/rewrite/rewriteHandler.c

Lines changed: 10 additions & 32 deletions
Original file line numberDiff line numberDiff line change
@@ -1004,23 +1004,11 @@ rewriteTargetListIU(List *targetList,
10041004
if (commandType==CMD_INSERT)
10051005
new_tle=NULL;
10061006
else
1007-
{
1008-
new_expr= (Node*)makeConst(att_tup->atttypid,
1009-
-1,
1010-
att_tup->attcollation,
1011-
att_tup->attlen,
1012-
(Datum)0,
1013-
true,/* isnull */
1014-
att_tup->attbyval);
1015-
/* this is to catch a NOT NULL domain constraint */
1016-
new_expr=coerce_to_domain(new_expr,
1017-
InvalidOid,-1,
1018-
att_tup->atttypid,
1019-
COERCION_IMPLICIT,
1020-
COERCE_IMPLICIT_CAST,
1021-
-1,
1022-
false);
1023-
}
1007+
new_expr=coerce_null_to_domain(att_tup->atttypid,
1008+
att_tup->atttypmod,
1009+
att_tup->attcollation,
1010+
att_tup->attlen,
1011+
att_tup->attbyval);
10241012
}
10251013

10261014
if (new_expr)
@@ -1582,21 +1570,11 @@ rewriteValuesRTE(Query *parsetree, RangeTblEntry *rte, int rti,
15821570
continue;
15831571
}
15841572

1585-
new_expr= (Node*)makeConst(att_tup->atttypid,
1586-
-1,
1587-
att_tup->attcollation,
1588-
att_tup->attlen,
1589-
(Datum)0,
1590-
true,/* isnull */
1591-
att_tup->attbyval);
1592-
/* this is to catch a NOT NULL domain constraint */
1593-
new_expr=coerce_to_domain(new_expr,
1594-
InvalidOid,-1,
1595-
att_tup->atttypid,
1596-
COERCION_IMPLICIT,
1597-
COERCE_IMPLICIT_CAST,
1598-
-1,
1599-
false);
1573+
new_expr=coerce_null_to_domain(att_tup->atttypid,
1574+
att_tup->atttypmod,
1575+
att_tup->attcollation,
1576+
att_tup->attlen,
1577+
att_tup->attbyval);
16001578
}
16011579
newList=lappend(newList,new_expr);
16021580
}

‎src/backend/rewrite/rewriteManip.c

Lines changed: 16 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,7 @@
2222
#include"parser/parse_relation.h"
2323
#include"parser/parsetree.h"
2424
#include"rewrite/rewriteManip.h"
25+
#include"utils/lsyscache.h"
2526

2627

2728
typedefstruct
@@ -1713,20 +1714,21 @@ ReplaceVarsFromTargetList_callback(Var *var,
17131714
return (Node*)var;
17141715

17151716
caseREPLACEVARS_SUBSTITUTE_NULL:
1716-
1717-
/*
1718-
* If Var is of domain type, we should add a CoerceToDomain
1719-
* node, in case there is a NOT NULL domain constraint.
1720-
*/
1721-
returncoerce_to_domain((Node*)makeNullConst(var->vartype,
1722-
var->vartypmod,
1723-
var->varcollid),
1724-
InvalidOid,-1,
1725-
var->vartype,
1726-
COERCION_IMPLICIT,
1727-
COERCE_IMPLICIT_CAST,
1728-
-1,
1729-
false);
1717+
{
1718+
/*
1719+
* If Var is of domain type, we must add a CoerceToDomain
1720+
* node, in case there is a NOT NULL domain constraint.
1721+
*/
1722+
int16vartyplen;
1723+
boolvartypbyval;
1724+
1725+
get_typlenbyval(var->vartype,&vartyplen,&vartypbyval);
1726+
returncoerce_null_to_domain(var->vartype,
1727+
var->vartypmod,
1728+
var->varcollid,
1729+
vartyplen,
1730+
vartypbyval);
1731+
}
17301732
}
17311733
elog(ERROR,"could not find replacement targetlist entry for attno %d",
17321734
var->varattno);

‎src/include/parser/parse_coerce.h

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -63,6 +63,9 @@ extern Node *coerce_to_specific_type_typmod(ParseState *pstate, Node *node,
6363
OidtargetTypeId,int32targetTypmod,
6464
constchar*constructName);
6565

66+
externNode*coerce_null_to_domain(Oidtypid,int32typmod,Oidcollation,
67+
inttyplen,booltypbyval);
68+
6669
externintparser_coercion_errposition(ParseState*pstate,
6770
intcoerce_location,
6871
Node*input_expr);

0 commit comments

Comments
 (0)
[8]ページ先頭
©2009-2025 Movatter.jp