I was thinking some more about the recent fix to multi-row VALUES
handling in the rewriter (b8f2687fdc), and I realised that there is
another bug in the way DEFAULT values are handled:
In RewriteQuery(), the code assumes that in a multi-row INSERT query,
the VALUES RTE will be the only thing in the query's fromlist. That's
true for the original query, but it's not necessarily the case for
product queries, if the rule action performs a multi-row insert,
leading to a new VALUES RTE that the DEFAULT-handling code might fail
to process. For example:
CREATE TABLE foo(a int);
INSERT INTO foo VALUES (1);
CREATE TABLE foo_log(t timestamptz DEFAULT now(), a int, c text);
CREATE RULE foo_r AS ON UPDATE TO foo
DO ALSO INSERT INTO foo_log VALUES (DEFAULT, old.a, 'old'),
(DEFAULT, new.a, 'new');
UPDATE foo SET a = 2 WHERE a = 1;
ERROR: unrecognized node type: 43
There's a similar example to this in the regression tests, but it
doesn't test DEFAULT-handling.
It's also possible for the current code to cause the same VALUES RTE
to be rewritten multiple times, when recursing into product queries
(if the rule action doesn't add any more stuff to the query's
fromlist). That turns out to be harmless, because the second time
round it will no longer contain any defaults, but it's technically
incorrect, and certainly a waste of cycles.
So I think what the code needs to do is examine the targetlist, and
identify the VALUES RTE that the current query is using as a source,
and rewrite just that RTE (so any original VALUES RTE is rewritten at
the top level, and any VALUES RTEs from rule actions are rewritten
while recursing, and none are rewritten more than once), as in the
attached patch.
While at it, I noticed an XXX code comment questioning whether any of
this applies to MERGE. The answer is "no", because MERGE actions don't
allow multi-row inserts, so I think it's worth updating that comment
to make that clearer.
Regards,
Dean
diff --git a/src/backend/rewrite/rewriteHandler.c b/src/backend/rewrite/rewriteHandler.c
new file mode 100644
index fb0c687..5ec4b91
--- a/src/backend/rewrite/rewriteHandler.c
+++ b/src/backend/rewrite/rewriteHandler.c
@@ -3743,26 +3743,34 @@ RewriteQuery(Query *parsetree, List *rew
*/
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.
+ * unique VALUES RTE referred to by the targetlist. At the top
+ * level, this will be the VALUES lists from the original query,
+ * and while recursively processing each product query, it will be
+ * the VALUES lists from the rule action, if any. As with
+ * rewriteValuesRTE(), we need only worry about targetlist entries
+ * that contain simple Vars referencing the VALUES RTE.
*/
- if (list_length(parsetree->jointree->fromlist) == 1)
+ foreach(lc2, parsetree->targetList)
{
- RangeTblRef *rtr = (RangeTblRef *) linitial(parsetree->jointree->fromlist);
+ TargetEntry *tle = (TargetEntry *) lfirst(lc2);
- if (IsA(rtr, RangeTblRef))
+ if (IsA(tle->expr, Var))
{
- RangeTblEntry *rte = rt_fetch(rtr->rtindex,
+ Var *var = (Var *) tle->expr;
+ RangeTblEntry *rte = rt_fetch(var->varno,
parsetree->rtable);
if (rte->rtekind == RTE_VALUES)
{
values_rte = rte;
- values_rte_index = rtr->rtindex;
+ values_rte_index = var->varno;
}
+ break;
}
}
@@ -3837,7 +3845,11 @@ RewriteQuery(Query *parsetree, List *rew
break;
case CMD_UPDATE:
case CMD_INSERT:
- /* XXX is it possible to have a VALUES clause? */
+
+ /*
+ * MERGE actions do not permit multi-row INSERTs, so
+ * there is no VALUES RTE to deal with here.
+ */
action->targetList =
rewriteTargetListIU(action->targetList,
action->commandType,
diff --git a/src/test/regress/expected/rules.out b/src/test/regress/expected/rules.out
new file mode 100644
index 624d0e5..9c28dd9
--- a/src/test/regress/expected/rules.out
+++ b/src/test/regress/expected/rules.out
@@ -2938,11 +2938,11 @@ select pg_get_viewdef('shoe'::regclass,0
--
-- 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;
@@ -2953,16 +2953,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
@@ -2976,71 +2976,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
new file mode 100644
index bfb5f3b..ada535a
--- a/src/test/regress/sql/rules.sql
+++ b/src/test/regress/sql/rules.sql
@@ -1016,11 +1016,11 @@ select pg_get_viewdef('shoe'::regclass,0
-- 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;
@@ -1028,16 +1028,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
--
