Skip to content

Commit c67204d

Browse files
deanrasheeddeanrasheed
committed
Fix DEFAULT handling for multi-row INSERT rules.
When updating a relation with a rule whose action performed an INSERT from a multi-row VALUES list, the rewriter might skip processing the VALUES list, and therefore fail to replace any DEFAULTs in it. This would lead to an "unrecognized node type" error. The reason was that RewriteQuery() assumed that a query doing an INSERT from a multi-row VALUES list would necessarily only have one item in its fromlist, pointing to the VALUES RTE to read from. That assumption is correct for the original query, but not for product queries produced for rule actions. In such cases, there may be multiple items in the fromlist, possibly including multiple VALUES RTEs. What is required instead is for RewriteQuery() to skip any RTEs from the product query's originating query, which might include one or more already-processed VALUES RTEs. What's left should then include at most one VALUES RTE (from the rule action) to be processed. Patch by me. Thanks to Tom Lane for reviewing. Back-patch to all supported branches. Discussion: https://postgr.es/m/CAEZATCV39OOW7LAR_Xq4i%2BLc1Byux%3DeK3Q%3DHD_pF1o9LBt%3DphA%40mail.gmail.com
1 parent c6a6047 commit c67204d

File tree

3 files changed

+104
-55
lines changed

3 files changed

+104
-55
lines changed

‎src/backend/rewrite/rewriteHandler.c

Lines changed: 43 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -418,6 +418,10 @@ rewriteRuleAction(Query *parsetree,
418418
* NOTE: because planner will destructively alter rtable, we must ensure
419419
* that rule action's rtable is separate and shares no substructure with
420420
* the main rtable. Hence do a deep copy here.
421+
*
422+
* Note also that RewriteQuery() relies on the fact that RT entries from
423+
* the original query appear at the start of the expanded rtable, so
424+
* beware of changing this.
421425
*/
422426
sub_action->rtable = list_concat(copyObject(parsetree->rtable),
423427
sub_action->rtable);
@@ -3622,9 +3626,13 @@ rewriteTargetView(Query *parsetree, Relation view)
36223626
*
36233627
* rewrite_events is a list of open query-rewrite actions, so we can detect
36243628
* infinite recursion.
3629+
*
3630+
* orig_rt_length is the length of the originating query's rtable, for product
3631+
* queries created by fireRules(), and 0 otherwise. This is used to skip any
3632+
* already-processed VALUES RTEs from the original query.
36253633
*/
36263634
static List *
3627-
RewriteQuery(Query *parsetree, List *rewrite_events)
3635+
RewriteQuery(Query *parsetree, List *rewrite_events, int orig_rt_length)
36283636
{
36293637
CmdType event = parsetree->commandType;
36303638
bool instead = false;
@@ -3648,7 +3656,7 @@ RewriteQuery(Query *parsetree, List *rewrite_events)
36483656
if (ctequery->commandType == CMD_SELECT)
36493657
continue;
36503658

3651-
newstuff = RewriteQuery(ctequery, rewrite_events);
3659+
newstuff = RewriteQuery(ctequery, rewrite_events, 0);
36523660

36533661
/*
36543662
* Currently we can only handle unconditional, single-statement DO
@@ -3722,6 +3730,7 @@ RewriteQuery(Query *parsetree, List *rewrite_events)
37223730
RangeTblEntry *rt_entry;
37233731
Relation rt_entry_relation;
37243732
List *locks;
3733+
int product_orig_rt_length;
37253734
List *product_queries;
37263735
bool hasUpdate = false;
37273736
int values_rte_index = 0;
@@ -3743,23 +3752,30 @@ RewriteQuery(Query *parsetree, List *rewrite_events)
37433752
*/
37443753
if (event == CMD_INSERT)
37453754
{
3755+
ListCell *lc2;
37463756
RangeTblEntry *values_rte = NULL;
37473757

37483758
/*
3749-
* If it's an INSERT ... VALUES (...), (...), ... there will be a
3750-
* single RTE for the VALUES targetlists.
3759+
* Test if it's a multi-row INSERT ... VALUES (...), (...), ... by
3760+
* looking for a VALUES RTE in the fromlist. For product queries,
3761+
* we must ignore any already-processed VALUES RTEs from the
3762+
* original query. These appear at the start of the rangetable.
37513763
*/
3752-
if (list_length(parsetree->jointree->fromlist) == 1)
3764+
foreach(lc2, parsetree->jointree->fromlist)
37533765
{
3754-
RangeTblRef *rtr = (RangeTblRef *) linitial(parsetree->jointree->fromlist);
3766+
RangeTblRef *rtr = (RangeTblRef *) lfirst(lc2);
37553767

3756-
if (IsA(rtr, RangeTblRef))
3768+
if (IsA(rtr, RangeTblRef) && rtr->rtindex > orig_rt_length)
37573769
{
37583770
RangeTblEntry *rte = rt_fetch(rtr->rtindex,
37593771
parsetree->rtable);
37603772

37613773
if (rte->rtekind == RTE_VALUES)
37623774
{
3775+
/* should not find more than one VALUES RTE */
3776+
if (values_rte != NULL)
3777+
elog(ERROR, "more than one VALUES RTE found");
3778+
37633779
values_rte = rte;
37643780
values_rte_index = rtr->rtindex;
37653781
}
@@ -3837,7 +3853,11 @@ RewriteQuery(Query *parsetree, List *rewrite_events)
38373853
break;
38383854
case CMD_UPDATE:
38393855
case CMD_INSERT:
3840-
/* XXX is it possible to have a VALUES clause? */
3856+
3857+
/*
3858+
* MERGE actions do not permit multi-row INSERTs, so
3859+
* there is no VALUES RTE to deal with here.
3860+
*/
38413861
action->targetList =
38423862
rewriteTargetListIU(action->targetList,
38433863
action->commandType,
@@ -3864,6 +3884,7 @@ RewriteQuery(Query *parsetree, List *rewrite_events)
38643884
locks = matchLocks(event, rt_entry_relation->rd_rules,
38653885
result_relation, parsetree, &hasUpdate);
38663886

3887+
product_orig_rt_length = list_length(parsetree->rtable);
38673888
product_queries = fireRules(parsetree,
38683889
result_relation,
38693890
event,
@@ -4020,7 +4041,19 @@ RewriteQuery(Query *parsetree, List *rewrite_events)
40204041
Query *pt = (Query *) lfirst(n);
40214042
List *newstuff;
40224043

4023-
newstuff = RewriteQuery(pt, rewrite_events);
4044+
/*
4045+
* For an updatable view, pt might be the rewritten version of
4046+
* the original query, in which case we pass on orig_rt_length
4047+
* to finish processing any VALUES RTE it contained.
4048+
*
4049+
* Otherwise, we have a product query created by fireRules().
4050+
* Any VALUES RTEs from the original query have been fully
4051+
* processed, and must be skipped when we recurse.
4052+
*/
4053+
newstuff = RewriteQuery(pt, rewrite_events,
4054+
pt == parsetree ?
4055+
orig_rt_length :
4056+
product_orig_rt_length);
40244057
rewritten = list_concat(rewritten, newstuff);
40254058
}
40264059

@@ -4172,7 +4205,7 @@ QueryRewrite(Query *parsetree)
41724205
*
41734206
* Apply all non-SELECT rules possibly getting 0 or many queries
41744207
*/
4175-
querylist = RewriteQuery(parsetree, NIL);
4208+
querylist = RewriteQuery(parsetree, NIL, 0);
41764209

41774210
/*
41784211
* Step 2

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

Lines changed: 52 additions & 39 deletions
Original file line numberDiff line numberDiff line change
@@ -2926,11 +2926,11 @@ select pg_get_viewdef('shoe'::regclass,0) as prettier;
29262926
--
29272927
-- check multi-row VALUES in rules
29282928
--
2929-
create table rules_src(f1 int, f2 int);
2930-
create table rules_log(f1 int, f2 int, tag text);
2929+
create table rules_src(f1 int, f2 int default 0);
2930+
create table rules_log(f1 int, f2 int, tag text, id serial);
29312931
insert into rules_src values(1,2), (11,12);
29322932
create rule r1 as on update to rules_src do also
2933-
insert into rules_log values(old.*, 'old'), (new.*, 'new');
2933+
insert into rules_log values(old.*, 'old', default), (new.*, 'new', default);
29342934
update rules_src set f2 = f2 + 1;
29352935
update rules_src set f2 = f2 * 10;
29362936
select * from rules_src;
@@ -2941,16 +2941,16 @@ select * from rules_src;
29412941
(2 rows)
29422942

29432943
select * from rules_log;
2944-
f1 | f2 | tag
2945-
----+-----+-----
2946-
1 | 2 | old
2947-
1 | 3 | new
2948-
11 | 12 | old
2949-
11 | 13 | new
2950-
1 | 3 | old
2951-
1 | 30 | new
2952-
11 | 13 | old
2953-
11 | 130 | new
2944+
f1 | f2 | tag | id
2945+
----+-----+-----+----
2946+
1 | 2 | old | 1
2947+
1 | 3 | new | 2
2948+
11 | 12 | old | 3
2949+
11 | 13 | new | 4
2950+
1 | 3 | old | 5
2951+
1 | 30 | new | 6
2952+
11 | 13 | old | 7
2953+
11 | 130 | new | 8
29542954
(8 rows)
29552955

29562956
create rule r2 as on update to rules_src do also
@@ -2964,71 +2964,84 @@ update rules_src set f2 = f2 / 10;
29642964
11 | 13 | new
29652965
(4 rows)
29662966

2967+
create rule r3 as on insert to rules_src do also
2968+
insert into rules_log values(null, null, '-', default), (new.*, 'new', default);
2969+
insert into rules_src values(22,23), (33,default);
29672970
select * from rules_src;
29682971
f1 | f2
29692972
----+----
29702973
1 | 3
29712974
11 | 13
2972-
(2 rows)
2975+
22 | 23
2976+
33 | 0
2977+
(4 rows)
29732978

29742979
select * from rules_log;
2975-
f1 | f2 | tag
2976-
----+-----+-----
2977-
1 | 2 | old
2978-
1 | 3 | new
2979-
11 | 12 | old
2980-
11 | 13 | new
2981-
1 | 3 | old
2982-
1 | 30 | new
2983-
11 | 13 | old
2984-
11 | 130 | new
2985-
1 | 30 | old
2986-
1 | 3 | new
2987-
11 | 130 | old
2988-
11 | 13 | new
2989-
(12 rows)
2990-
2991-
create rule r3 as on delete to rules_src do notify rules_src_deletion;
2980+
f1 | f2 | tag | id
2981+
----+-----+-----+----
2982+
1 | 2 | old | 1
2983+
1 | 3 | new | 2
2984+
11 | 12 | old | 3
2985+
11 | 13 | new | 4
2986+
1 | 3 | old | 5
2987+
1 | 30 | new | 6
2988+
11 | 13 | old | 7
2989+
11 | 130 | new | 8
2990+
1 | 30 | old | 9
2991+
1 | 3 | new | 10
2992+
11 | 130 | old | 11
2993+
11 | 13 | new | 12
2994+
| | - | 13
2995+
22 | 23 | new | 14
2996+
| | - | 15
2997+
33 | 0 | new | 16
2998+
(16 rows)
2999+
3000+
create rule r4 as on delete to rules_src do notify rules_src_deletion;
29923001
\d+ rules_src
29933002
Table "public.rules_src"
29943003
Column | Type | Collation | Nullable | Default | Storage | Stats target | Description
29953004
--------+---------+-----------+----------+---------+---------+--------------+-------------
29963005
f1 | integer | | | | plain | |
2997-
f2 | integer | | | | plain | |
3006+
f2 | integer | | | 0 | plain | |
29983007
Rules:
29993008
r1 AS
3000-
ON UPDATE TO rules_src DO INSERT INTO rules_log (f1, f2, tag) VALUES (old.f1,old.f2,'old'::text), (new.f1,new.f2,'new'::text)
3009+
ON UPDATE TO rules_src DO INSERT INTO rules_log (f1, f2, tag, id) VALUES (old.f1,old.f2,'old'::text,DEFAULT), (new.f1,new.f2,'new'::text,DEFAULT)
30013010
r2 AS
30023011
ON UPDATE TO rules_src DO VALUES (old.f1,old.f2,'old'::text), (new.f1,new.f2,'new'::text)
30033012
r3 AS
3013+
ON INSERT TO rules_src DO INSERT INTO rules_log (f1, f2, tag, id) VALUES (NULL::integer,NULL::integer,'-'::text,DEFAULT), (new.f1,new.f2,'new'::text,DEFAULT)
3014+
r4 AS
30043015
ON DELETE TO rules_src DO
30053016
NOTIFY rules_src_deletion
30063017

30073018
--
30083019
-- Ensure an aliased target relation for insert is correctly deparsed.
30093020
--
3010-
create rule r4 as on insert to rules_src do instead insert into rules_log AS trgt SELECT NEW.* RETURNING trgt.f1, trgt.f2;
3011-
create rule r5 as on update to rules_src do instead UPDATE rules_log AS trgt SET tag = 'updated' WHERE trgt.f1 = new.f1;
3021+
create rule r5 as on insert to rules_src do instead insert into rules_log AS trgt SELECT NEW.* RETURNING trgt.f1, trgt.f2;
3022+
create rule r6 as on update to rules_src do instead UPDATE rules_log AS trgt SET tag = 'updated' WHERE trgt.f1 = new.f1;
30123023
\d+ rules_src
30133024
Table "public.rules_src"
30143025
Column | Type | Collation | Nullable | Default | Storage | Stats target | Description
30153026
--------+---------+-----------+----------+---------+---------+--------------+-------------
30163027
f1 | integer | | | | plain | |
3017-
f2 | integer | | | | plain | |
3028+
f2 | integer | | | 0 | plain | |
30183029
Rules:
30193030
r1 AS
3020-
ON UPDATE TO rules_src DO INSERT INTO rules_log (f1, f2, tag) VALUES (old.f1,old.f2,'old'::text), (new.f1,new.f2,'new'::text)
3031+
ON UPDATE TO rules_src DO INSERT INTO rules_log (f1, f2, tag, id) VALUES (old.f1,old.f2,'old'::text,DEFAULT), (new.f1,new.f2,'new'::text,DEFAULT)
30213032
r2 AS
30223033
ON UPDATE TO rules_src DO VALUES (old.f1,old.f2,'old'::text), (new.f1,new.f2,'new'::text)
30233034
r3 AS
3035+
ON INSERT TO rules_src DO INSERT INTO rules_log (f1, f2, tag, id) VALUES (NULL::integer,NULL::integer,'-'::text,DEFAULT), (new.f1,new.f2,'new'::text,DEFAULT)
3036+
r4 AS
30243037
ON DELETE TO rules_src DO
30253038
NOTIFY rules_src_deletion
3026-
r4 AS
3039+
r5 AS
30273040
ON INSERT TO rules_src DO INSTEAD INSERT INTO rules_log AS trgt (f1, f2) SELECT new.f1,
30283041
new.f2
30293042
RETURNING trgt.f1,
30303043
trgt.f2
3031-
r5 AS
3044+
r6 AS
30323045
ON UPDATE TO rules_src DO INSTEAD UPDATE rules_log trgt SET tag = 'updated'::text
30333046
WHERE trgt.f1 = new.f1
30343047

‎src/test/regress/sql/rules.sql

Lines changed: 9 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -1016,28 +1016,31 @@ select pg_get_viewdef('shoe'::regclass,0) as prettier;
10161016
-- check multi-row VALUES in rules
10171017
--
10181018

1019-
create table rules_src(f1 int, f2 int);
1020-
create table rules_log(f1 int, f2 int, tag text);
1019+
create table rules_src(f1 int, f2 int default 0);
1020+
create table rules_log(f1 int, f2 int, tag text, id serial);
10211021
insert into rules_src values(1,2), (11,12);
10221022
create rule r1 as on update to rules_src do also
1023-
insert into rules_log values(old.*, 'old'), (new.*, 'new');
1023+
insert into rules_log values(old.*, 'old', default), (new.*, 'new', default);
10241024
update rules_src set f2 = f2 + 1;
10251025
update rules_src set f2 = f2 * 10;
10261026
select * from rules_src;
10271027
select * from rules_log;
10281028
create rule r2 as on update to rules_src do also
10291029
values(old.*, 'old'), (new.*, 'new');
10301030
update rules_src set f2 = f2 / 10;
1031+
create rule r3 as on insert to rules_src do also
1032+
insert into rules_log values(null, null, '-', default), (new.*, 'new', default);
1033+
insert into rules_src values(22,23), (33,default);
10311034
select * from rules_src;
10321035
select * from rules_log;
1033-
create rule r3 as on delete to rules_src do notify rules_src_deletion;
1036+
create rule r4 as on delete to rules_src do notify rules_src_deletion;
10341037
\d+ rules_src
10351038

10361039
--
10371040
-- Ensure an aliased target relation for insert is correctly deparsed.
10381041
--
1039-
create rule r4 as on insert to rules_src do instead insert into rules_log AS trgt SELECT NEW.* RETURNING trgt.f1, trgt.f2;
1040-
create rule r5 as on update to rules_src do instead UPDATE rules_log AS trgt SET tag = 'updated' WHERE trgt.f1 = new.f1;
1042+
create rule r5 as on insert to rules_src do instead insert into rules_log AS trgt SELECT NEW.* RETURNING trgt.f1, trgt.f2;
1043+
create rule r6 as on update to rules_src do instead UPDATE rules_log AS trgt SET tag = 'updated' WHERE trgt.f1 = new.f1;
10411044
\d+ rules_src
10421045

10431046
--

0 commit comments

Comments
 (0)