From 30f9b03a08bde2c4235ef5f1812ae194b32fb7e3 Mon Sep 17 00:00:00 2001 From: Dean Rasheed Date: Sat, 3 Dec 2022 12:20:02 +0000 Subject: [PATCH] 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 --- src/backend/rewrite/rewriteHandler.c | 47 ++++++++++++--- src/test/regress/expected/rules.out | 89 ++++++++++++++++------------ src/test/regress/sql/rules.sql | 15 +++-- 3 files changed, 98 insertions(+), 53 deletions(-) diff --git a/src/backend/rewrite/rewriteHandler.c b/src/backend/rewrite/rewriteHandler.c index 02e4146e84..0304a68759 100644 --- a/src/backend/rewrite/rewriteHandler.c +++ b/src/backend/rewrite/rewriteHandler.c @@ -408,6 +408,10 @@ rewriteRuleAction(Query *parsetree, * NOTE: because planner will destructively alter rtable, we must ensure * that rule action's rtable is separate and shares no substructure with * the main rtable. Hence do a deep copy here. + * + * Note also that RewriteQuery() relies on the fact that RT entries from + * the original query appear at the start of the expanded rtable, so + * beware of changing this. */ sub_action->rtable = list_concat(copyObject(parsetree->rtable), sub_action->rtable); @@ -3463,9 +3467,13 @@ rewriteTargetView(Query *parsetree, Relation view) * * rewrite_events is a list of open query-rewrite actions, so we can detect * infinite recursion. + * + * orig_rt_length is the length of the originating query's rtable, for product + * queries created by fireRules(), and 0 otherwise. This is used to skip any + * already-processed VALUES RTEs from the original query. */ static List * -RewriteQuery(Query *parsetree, List *rewrite_events) +RewriteQuery(Query *parsetree, List *rewrite_events, int orig_rt_length) { CmdType event = parsetree->commandType; bool instead = false; @@ -3489,7 +3497,7 @@ RewriteQuery(Query *parsetree, List *rewrite_events) if (ctequery->commandType == CMD_SELECT) continue; - newstuff = RewriteQuery(ctequery, rewrite_events); + newstuff = RewriteQuery(ctequery, rewrite_events, 0); /* * Currently we can only handle unconditional, single-statement DO @@ -3563,6 +3571,7 @@ RewriteQuery(Query *parsetree, List *rewrite_events) RangeTblEntry *rt_entry; Relation rt_entry_relation; List *locks; + int product_orig_rt_length; List *product_queries; bool hasUpdate = false; int values_rte_index = 0; @@ -3584,23 +3593,30 @@ RewriteQuery(Query *parsetree, List *rewrite_events) */ if (event == CMD_INSERT) { + ListCell *lc2; RangeTblEntry *values_rte = NULL; /* - * If it's an INSERT ... VALUES (...), (...), ... there will be a - * single RTE for the VALUES targetlists. + * Test if it's a multi-row INSERT ... VALUES (...), (...), ... by + * looking for a VALUES RTE in the fromlist. For product queries, + * we must ignore any already-processed VALUES RTEs from the + * original query. These appear at the start of the rangetable. */ - if (list_length(parsetree->jointree->fromlist) == 1) + foreach(lc2, parsetree->jointree->fromlist) { - RangeTblRef *rtr = (RangeTblRef *) linitial(parsetree->jointree->fromlist); + RangeTblRef *rtr = (RangeTblRef *) lfirst(lc2); - if (IsA(rtr, RangeTblRef)) + if (IsA(rtr, RangeTblRef) && rtr->rtindex > orig_rt_length) { RangeTblEntry *rte = rt_fetch(rtr->rtindex, parsetree->rtable); if (rte->rtekind == RTE_VALUES) { + /* should not find more than one VALUES RTE */ + if (values_rte != NULL) + elog(ERROR, "more than one VALUES RTE found"); + values_rte = rte; values_rte_index = rtr->rtindex; } @@ -3664,6 +3680,7 @@ RewriteQuery(Query *parsetree, List *rewrite_events) locks = matchLocks(event, rt_entry_relation->rd_rules, result_relation, parsetree, &hasUpdate); + product_orig_rt_length = list_length(parsetree->rtable); product_queries = fireRules(parsetree, result_relation, event, @@ -3820,7 +3837,19 @@ RewriteQuery(Query *parsetree, List *rewrite_events) Query *pt = (Query *) lfirst(n); List *newstuff; - newstuff = RewriteQuery(pt, rewrite_events); + /* + * For an updatable view, pt might be the rewritten version of + * the original query, in which case we pass on orig_rt_length + * to finish processing any VALUES RTE it contained. + * + * Otherwise, we have a product query created by fireRules(). + * Any VALUES RTEs from the original query have been fully + * processed, and must be skipped when we recurse. + */ + newstuff = RewriteQuery(pt, rewrite_events, + pt == parsetree ? + orig_rt_length : + product_orig_rt_length); rewritten = list_concat(rewritten, newstuff); } @@ -3972,7 +4001,7 @@ QueryRewrite(Query *parsetree) * * Apply all non-SELECT rules possibly getting 0 or many queries */ - querylist = RewriteQuery(parsetree, NIL); + querylist = RewriteQuery(parsetree, NIL, 0); /* * Step 2 diff --git a/src/test/regress/expected/rules.out b/src/test/regress/expected/rules.out index 3c814e2d33..9077df1ae0 100644 --- a/src/test/regress/expected/rules.out +++ b/src/test/regress/expected/rules.out @@ -2783,11 +2783,11 @@ select pg_get_viewdef('shoe'::regclass,0) as prettier; -- -- check multi-row VALUES in rules -- -create table rules_src(f1 int, f2 int); -create table rules_log(f1 int, f2 int, tag text); +create table rules_src(f1 int, f2 int default 0); +create table rules_log(f1 int, f2 int, tag text, id serial); insert into rules_src values(1,2), (11,12); create rule r1 as on update to rules_src do also - insert into rules_log values(old.*, 'old'), (new.*, 'new'); + insert into rules_log values(old.*, 'old', default), (new.*, 'new', default); update rules_src set f2 = f2 + 1; update rules_src set f2 = f2 * 10; select * from rules_src; @@ -2798,16 +2798,16 @@ select * from rules_src; (2 rows) select * from rules_log; - f1 | f2 | tag -----+-----+----- - 1 | 2 | old - 1 | 3 | new - 11 | 12 | old - 11 | 13 | new - 1 | 3 | old - 1 | 30 | new - 11 | 13 | old - 11 | 130 | new + f1 | f2 | tag | id +----+-----+-----+---- + 1 | 2 | old | 1 + 1 | 3 | new | 2 + 11 | 12 | old | 3 + 11 | 13 | new | 4 + 1 | 3 | old | 5 + 1 | 30 | new | 6 + 11 | 13 | old | 7 + 11 | 130 | new | 8 (8 rows) create rule r2 as on update to rules_src do also @@ -2821,71 +2821,84 @@ update rules_src set f2 = f2 / 10; 11 | 13 | new (4 rows) +create rule r3 as on insert to rules_src do also + insert into rules_log values(null, null, '-', default), (new.*, 'new', default); +insert into rules_src values(22,23), (33,default); select * from rules_src; f1 | f2 ----+---- 1 | 3 11 | 13 -(2 rows) + 22 | 23 + 33 | 0 +(4 rows) select * from rules_log; - f1 | f2 | tag -----+-----+----- - 1 | 2 | old - 1 | 3 | new - 11 | 12 | old - 11 | 13 | new - 1 | 3 | old - 1 | 30 | new - 11 | 13 | old - 11 | 130 | new - 1 | 30 | old - 1 | 3 | new - 11 | 130 | old - 11 | 13 | new -(12 rows) + f1 | f2 | tag | id +----+-----+-----+---- + 1 | 2 | old | 1 + 1 | 3 | new | 2 + 11 | 12 | old | 3 + 11 | 13 | new | 4 + 1 | 3 | old | 5 + 1 | 30 | new | 6 + 11 | 13 | old | 7 + 11 | 130 | new | 8 + 1 | 30 | old | 9 + 1 | 3 | new | 10 + 11 | 130 | old | 11 + 11 | 13 | new | 12 + | | - | 13 + 22 | 23 | new | 14 + | | - | 15 + 33 | 0 | new | 16 +(16 rows) -create rule r3 as on delete to rules_src do notify rules_src_deletion; +create rule r4 as on delete to rules_src do notify rules_src_deletion; \d+ rules_src Table "public.rules_src" Column | Type | Collation | Nullable | Default | Storage | Stats target | Description --------+---------+-----------+----------+---------+---------+--------------+------------- f1 | integer | | | | plain | | - f2 | integer | | | | plain | | + f2 | integer | | | 0 | plain | | Rules: r1 AS - 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) + 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) r2 AS ON UPDATE TO rules_src DO VALUES (old.f1,old.f2,'old'::text), (new.f1,new.f2,'new'::text) r3 AS + 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) + r4 AS ON DELETE TO rules_src DO NOTIFY rules_src_deletion -- -- Ensure an aliased target relation for insert is correctly deparsed. -- -create rule r4 as on insert to rules_src do instead insert into rules_log AS trgt SELECT NEW.* RETURNING trgt.f1, trgt.f2; -create rule r5 as on update to rules_src do instead UPDATE rules_log AS trgt SET tag = 'updated' WHERE trgt.f1 = new.f1; +create rule r5 as on insert to rules_src do instead insert into rules_log AS trgt SELECT NEW.* RETURNING trgt.f1, trgt.f2; +create rule r6 as on update to rules_src do instead UPDATE rules_log AS trgt SET tag = 'updated' WHERE trgt.f1 = new.f1; \d+ rules_src Table "public.rules_src" Column | Type | Collation | Nullable | Default | Storage | Stats target | Description --------+---------+-----------+----------+---------+---------+--------------+------------- f1 | integer | | | | plain | | - f2 | integer | | | | plain | | + f2 | integer | | | 0 | plain | | Rules: r1 AS - 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) + 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) r2 AS ON UPDATE TO rules_src DO VALUES (old.f1,old.f2,'old'::text), (new.f1,new.f2,'new'::text) r3 AS + 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) + r4 AS ON DELETE TO rules_src DO NOTIFY rules_src_deletion - r4 AS + r5 AS ON INSERT TO rules_src DO INSTEAD INSERT INTO rules_log AS trgt (f1, f2) SELECT new.f1, new.f2 RETURNING trgt.f1, trgt.f2 - r5 AS + r6 AS ON UPDATE TO rules_src DO INSTEAD UPDATE rules_log trgt SET tag = 'updated'::text WHERE trgt.f1 = new.f1 diff --git a/src/test/regress/sql/rules.sql b/src/test/regress/sql/rules.sql index 92b66cdb5f..cf7a98d2bf 100644 --- a/src/test/regress/sql/rules.sql +++ b/src/test/regress/sql/rules.sql @@ -1015,11 +1015,11 @@ select pg_get_viewdef('shoe'::regclass,0) as prettier; -- check multi-row VALUES in rules -- -create table rules_src(f1 int, f2 int); -create table rules_log(f1 int, f2 int, tag text); +create table rules_src(f1 int, f2 int default 0); +create table rules_log(f1 int, f2 int, tag text, id serial); insert into rules_src values(1,2), (11,12); create rule r1 as on update to rules_src do also - insert into rules_log values(old.*, 'old'), (new.*, 'new'); + insert into rules_log values(old.*, 'old', default), (new.*, 'new', default); update rules_src set f2 = f2 + 1; update rules_src set f2 = f2 * 10; select * from rules_src; @@ -1027,16 +1027,19 @@ select * from rules_log; create rule r2 as on update to rules_src do also values(old.*, 'old'), (new.*, 'new'); update rules_src set f2 = f2 / 10; +create rule r3 as on insert to rules_src do also + insert into rules_log values(null, null, '-', default), (new.*, 'new', default); +insert into rules_src values(22,23), (33,default); select * from rules_src; select * from rules_log; -create rule r3 as on delete to rules_src do notify rules_src_deletion; +create rule r4 as on delete to rules_src do notify rules_src_deletion; \d+ rules_src -- -- Ensure an aliased target relation for insert is correctly deparsed. -- -create rule r4 as on insert to rules_src do instead insert into rules_log AS trgt SELECT NEW.* RETURNING trgt.f1, trgt.f2; -create rule r5 as on update to rules_src do instead UPDATE rules_log AS trgt SET tag = 'updated' WHERE trgt.f1 = new.f1; +create rule r5 as on insert to rules_src do instead insert into rules_log AS trgt SELECT NEW.* RETURNING trgt.f1, trgt.f2; +create rule r6 as on update to rules_src do instead UPDATE rules_log AS trgt SET tag = 'updated' WHERE trgt.f1 = new.f1; \d+ rules_src --