Location via proxy:   [ UP ]  
[Report a bug]   [Manage cookies]                
Skip to content

Commit 4af6e61

Browse files
anarazelanarazel
committed
Fix ON CONFLICT bugs that manifest when used in rules.
Specifically the tlist and rti of the pseudo "excluded" relation weren't properly treated by expression_tree_walker, which lead to errors when excluded was referenced inside a rule because the varnos where not properly adjusted. Similar omissions in OffsetVarNodes and expression_tree_mutator had less impact, but should obviously be fixed nonetheless. A couple tests of for ON CONFLICT UPDATE into INSERT rule bearing relations have been added. In passing I updated a couple comments.
1 parent 5c7df74 commit 4af6e61

File tree

6 files changed

+151
-17
lines changed

6 files changed

+151
-17
lines changed

‎src/backend/executor/nodeModifyTable.c

+1
Original file line numberDiff line numberDiff line change
@@ -1675,6 +1675,7 @@ ExecInitModifyTable(ModifyTable *node, EState *estate, int eflags)
16751675
ExecSetSlotDescriptor(mtstate->mt_existing,
16761676
resultRelInfo->ri_RelationDesc->rd_att);
16771677

1678+
/* carried forward solely for the benefit of explain */
16781679
mtstate->mt_excludedtlist = node->exclRelTlist;
16791680

16801681
/* create target slot for UPDATE SET projection */

‎src/backend/nodes/nodeFuncs.c

+3
Original file line numberDiff line numberDiff line change
@@ -1922,6 +1922,8 @@ expression_tree_walker(Node *node,
19221922
return true;
19231923
if (walker(onconflict->onConflictWhere, context))
19241924
return true;
1925+
if (walker(onconflict->exclRelTlist, context))
1926+
return true;
19251927
}
19261928
break;
19271929
case T_JoinExpr:
@@ -2642,6 +2644,7 @@ expression_tree_mutator(Node *node,
26422644
MUTATE(newnode->arbiterWhere, oc->arbiterWhere, Node *);
26432645
MUTATE(newnode->onConflictSet, oc->onConflictSet, List *);
26442646
MUTATE(newnode->onConflictWhere, oc->onConflictWhere, Node *);
2647+
MUTATE(newnode->exclRelTlist, oc->exclRelTlist, List *);
26452648

26462649
return (Node *) newnode;
26472650
}

‎src/backend/optimizer/plan/setrefs.c

+6-3
Original file line numberDiff line numberDiff line change
@@ -740,9 +740,9 @@ set_plan_refs(PlannerInfo *root, Plan *plan, int rtoffset)
740740

741741
/*
742742
* We treat ModifyTable with ON CONFLICT as a form of 'pseudo
743-
* join', where the inner side is the EXLUDED tuple. Therefore
744-
* use fix_join_expr to setup the relevant variables to
745-
* INNER_VAR. We explicitly don't create any OUTER_VARs as
743+
* join', where the inner side is the EXCLUDED tuple.
744+
* Therefore use fix_join_expr to setup the relevant variables
745+
* to INNER_VAR. We explicitly don't create any OUTER_VARs as
746746
* those are already used by RETURNING and it seems better to
747747
* be non-conflicting.
748748
*/
@@ -763,6 +763,9 @@ set_plan_refs(PlannerInfo *root, Plan *plan, int rtoffset)
763763
NULL, itlist,
764764
linitial_int(splan->resultRelations),
765765
rtoffset);
766+
767+
splan->exclRelTlist =
768+
fix_scan_list(root, splan->exclRelTlist, rtoffset);
766769
}
767770

768771
splan->nominalRelation += rtoffset;

‎src/backend/rewrite/rewriteManip.c

+12-3
Original file line numberDiff line numberDiff line change
@@ -426,16 +426,20 @@ OffsetVarNodes(Node *node, int offset, int sublevels_up)
426426
/*
427427
* If we are starting at a Query, and sublevels_up is zero, then we
428428
* must also fix rangetable indexes in the Query itself --- namely
429-
* resultRelation and rowMarks entries. sublevels_up cannot be zero
430-
* when recursing into a subquery, so there's no need to have the same
431-
* logic inside OffsetVarNodes_walker.
429+
* resultRelation, exclRelIndex and rowMarks entries. sublevels_up
430+
* cannot be zero when recursing into a subquery, so there's no need
431+
* to have the same logic inside OffsetVarNodes_walker.
432432
*/
433433
if (sublevels_up == 0)
434434
{
435435
ListCell *l;
436436

437437
if (qry->resultRelation)
438438
qry->resultRelation += offset;
439+
440+
if (qry->onConflict && qry->onConflict->exclRelIndex)
441+
qry->onConflict->exclRelIndex += offset;
442+
439443
foreach(l, qry->rowMarks)
440444
{
441445
RowMarkClause *rc = (RowMarkClause *) lfirst(l);
@@ -617,6 +621,11 @@ ChangeVarNodes(Node *node, int rt_index, int new_index, int sublevels_up)
617621

618622
if (qry->resultRelation == rt_index)
619623
qry->resultRelation = new_index;
624+
625+
/* this is unlikely to ever be used, but ... */
626+
if (qry->onConflict && qry->onConflict->exclRelIndex == rt_index)
627+
qry->onConflict->exclRelIndex = new_index;
628+
620629
foreach(l, qry->rowMarks)
621630
{
622631
RowMarkClause *rc = (RowMarkClause *) lfirst(l);

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

+96-9
Original file line numberDiff line numberDiff line change
@@ -2817,25 +2817,112 @@ CREATE RULE hat_upsert AS ON INSERT TO hats
28172817
INSERT INTO hat_data VALUES (
28182818
NEW.hat_name,
28192819
NEW.hat_color)
2820-
ON CONFLICT (hat_name) DO UPDATE SET hat_color = 'Orange' RETURNING *;
2820+
ON CONFLICT (hat_name)
2821+
DO UPDATE
2822+
SET hat_name = hat_data.hat_name, hat_color = excluded.hat_color
2823+
WHERE excluded.hat_color <> 'forbidden'
2824+
RETURNING *;
28212825
-- Works (does upsert)
2822-
INSERT INTO hats VALUES ('h7', 'black') RETURNING *;
2826+
INSERT INTO hats VALUES ('h8', 'black') RETURNING *;
2827+
hat_name | hat_color
2828+
------------+------------
2829+
h8 | black
2830+
(1 row)
2831+
2832+
SELECT * FROM hat_data WHERE hat_name = 'h8';
2833+
hat_name | hat_color
2834+
------------+------------
2835+
h8 | black
2836+
(1 row)
2837+
2838+
INSERT INTO hats VALUES ('h8', 'white') RETURNING *;
2839+
hat_name | hat_color
2840+
------------+------------
2841+
h8 | white
2842+
(1 row)
2843+
2844+
SELECT * FROM hat_data WHERE hat_name = 'h8';
2845+
hat_name | hat_color
2846+
------------+------------
2847+
h8 | white
2848+
(1 row)
2849+
2850+
INSERT INTO hats VALUES ('h8', 'forbidden') RETURNING *;
2851+
hat_name | hat_color
2852+
----------+-----------
2853+
(0 rows)
2854+
2855+
SELECT * FROM hat_data WHERE hat_name = 'h8';
28232856
hat_name | hat_color
28242857
------------+------------
2825-
h7 | Orange
2858+
h8 | white
28262859
(1 row)
28272860

28282861
SELECT tablename, rulename, definition FROM pg_rules
28292862
WHERE tablename = 'hats';
2830-
tablename | rulename | definition
2831-
-----------+------------+-----------------------------------------------------------------------------------------------
2832-
hats | hat_upsert | CREATE RULE hat_upsert AS +
2833-
| | ON INSERT TO hats DO INSTEAD INSERT INTO hat_data (hat_name, hat_color) +
2834-
| | VALUES (new.hat_name, new.hat_color) ON CONFLICT DO UPDATE SET hat_color = 'Orange'::bpchar+
2835-
| | RETURNING hat_data.hat_name, +
2863+
tablename | rulename | definition
2864+
-----------+------------+-------------------------------------------------------------------------------------------------------------------------------
2865+
hats | hat_upsert | CREATE RULE hat_upsert AS +
2866+
| | ON INSERT TO hats DO INSTEAD INSERT INTO hat_data (hat_name, hat_color) +
2867+
| | VALUES (new.hat_name, new.hat_color) ON CONFLICT DO UPDATE SET hat_name = hat_data.hat_name, hat_color = excluded.hat_color+
2868+
| | WHERE (excluded.hat_color <> 'forbidden'::bpchar) +
2869+
| | RETURNING hat_data.hat_name, +
28362870
| | hat_data.hat_color;
28372871
(1 row)
28382872

2873+
-- ensure explain works for on insert conflict rules
2874+
explain (costs off) INSERT INTO hats VALUES ('h8', 'forbidden') RETURNING *;
2875+
QUERY PLAN
2876+
----------------------------------------------------------------
2877+
Insert on hat_data
2878+
Conflict Resolution: UPDATE
2879+
Conflict Arbiter Indexes: hat_data_pkey
2880+
Conflict Filter: (excluded.hat_color <> 'forbidden'::bpchar)
2881+
-> Result
2882+
(5 rows)
2883+
2884+
-- ensure upserting into a rule, with a CTE (different offsets!) works
2885+
WITH data(hat_name, hat_color) AS (
2886+
VALUES ('h8', 'green'),
2887+
('h9', 'blue'),
2888+
('h7', 'forbidden')
2889+
)
2890+
INSERT INTO hats
2891+
SELECT * FROM data
2892+
RETURNING *;
2893+
hat_name | hat_color
2894+
------------+------------
2895+
h8 | green
2896+
h9 | blue
2897+
(2 rows)
2898+
2899+
EXPLAIN (costs off) WITH data(hat_name, hat_color) AS (
2900+
VALUES ('h8', 'green'),
2901+
('h9', 'blue'),
2902+
('h7', 'forbidden')
2903+
)
2904+
INSERT INTO hats
2905+
SELECT * FROM data
2906+
RETURNING *;
2907+
QUERY PLAN
2908+
----------------------------------------------------------------
2909+
Insert on hat_data
2910+
Conflict Resolution: UPDATE
2911+
Conflict Arbiter Indexes: hat_data_pkey
2912+
Conflict Filter: (excluded.hat_color <> 'forbidden'::bpchar)
2913+
CTE data
2914+
-> Values Scan on "*VALUES*"
2915+
-> CTE Scan on data
2916+
(7 rows)
2917+
2918+
SELECT * FROM hat_data WHERE hat_name IN ('h8', 'h9', 'h7') ORDER BY hat_name;
2919+
hat_name | hat_color
2920+
------------+------------
2921+
h7 | black
2922+
h8 | green
2923+
h9 | blue
2924+
(3 rows)
2925+
28392926
DROP RULE hat_upsert ON hats;
28402927
drop table hats;
28412928
drop table hat_data;

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

+33-2
Original file line numberDiff line numberDiff line change
@@ -1074,12 +1074,43 @@ CREATE RULE hat_upsert AS ON INSERT TO hats
10741074
INSERT INTO hat_data VALUES (
10751075
NEW.hat_name,
10761076
NEW.hat_color)
1077-
ON CONFLICT (hat_name) DO UPDATE SET hat_color = 'Orange' RETURNING *;
1077+
ON CONFLICT (hat_name)
1078+
DO UPDATE
1079+
SET hat_name = hat_data.hat_name, hat_color = excluded.hat_color
1080+
WHERE excluded.hat_color <> 'forbidden'
1081+
RETURNING *;
10781082

10791083
-- Works (does upsert)
1080-
INSERT INTO hats VALUES ('h7', 'black') RETURNING *;
1084+
INSERT INTO hats VALUES ('h8', 'black') RETURNING *;
1085+
SELECT * FROM hat_data WHERE hat_name = 'h8';
1086+
INSERT INTO hats VALUES ('h8', 'white') RETURNING *;
1087+
SELECT * FROM hat_data WHERE hat_name = 'h8';
1088+
INSERT INTO hats VALUES ('h8', 'forbidden') RETURNING *;
1089+
SELECT * FROM hat_data WHERE hat_name = 'h8';
10811090
SELECT tablename, rulename, definition FROM pg_rules
10821091
WHERE tablename = 'hats';
1092+
-- ensure explain works for on insert conflict rules
1093+
explain (costs off) INSERT INTO hats VALUES ('h8', 'forbidden') RETURNING *;
1094+
1095+
-- ensure upserting into a rule, with a CTE (different offsets!) works
1096+
WITH data(hat_name, hat_color) AS (
1097+
VALUES ('h8', 'green'),
1098+
('h9', 'blue'),
1099+
('h7', 'forbidden')
1100+
)
1101+
INSERT INTO hats
1102+
SELECT * FROM data
1103+
RETURNING *;
1104+
EXPLAIN (costs off) WITH data(hat_name, hat_color) AS (
1105+
VALUES ('h8', 'green'),
1106+
('h9', 'blue'),
1107+
('h7', 'forbidden')
1108+
)
1109+
INSERT INTO hats
1110+
SELECT * FROM data
1111+
RETURNING *;
1112+
SELECT * FROM hat_data WHERE hat_name IN ('h8', 'h9', 'h7') ORDER BY hat_name;
1113+
10831114
DROP RULE hat_upsert ON hats;
10841115

10851116
drop table hats;

0 commit comments

Comments
 (0)