]> git.ipfire.org Git - thirdparty/postgresql.git/commitdiff
Fix MERGE into a plain inheritance parent table.
authorDean Rasheed <dean.a.rasheed@gmail.com>
Sat, 31 May 2025 11:19:37 +0000 (12:19 +0100)
committerDean Rasheed <dean.a.rasheed@gmail.com>
Sat, 31 May 2025 11:19:37 +0000 (12:19 +0100)
When a MERGE's target table is the parent of an inheritance tree, any
INSERT actions insert into the parent table using ModifyTableState's
rootResultRelInfo. However, there are two bugs in the way this is
initialized:

1. ExecInitMerge() incorrectly uses a different ResultRelInfo entry
from ModifyTableState's resultRelInfo array to build the insert
projection, which may not be compatible with rootResultRelInfo.

2. ExecInitModifyTable() does not fully initialize rootResultRelInfo.
Specifically, ri_WithCheckOptions, ri_WithCheckOptionExprs,
ri_returningList, and ri_projectReturning are not initialized.

This can lead to crashes, or incorrect query results due to failing to
check WCO's or process the RETURNING list for INSERT actions.

Fix both these bugs in ExecInitMerge(), noting that it is only
necessary to fully initialize rootResultRelInfo if the MERGE has
INSERT actions and the target table is a plain inheritance parent.

Backpatch to v15, where MERGE was introduced.

Reported-by: Andres Freund <andres@anarazel.de>
Author: Dean Rasheed <dean.a.rasheed@gmail.com>
Reviewed-by: Jian He <jian.universality@gmail.com>
Reviewed-by: Tender Wang <tndrwang@gmail.com>
Discussion: https://postgr.es/m/4rlmjfniiyffp6b3kv4pfy4jw3pciy6mq72rdgnedsnbsx7qe5@j5hlpiwdguvc
Backpatch-through: 15

src/backend/executor/nodeModifyTable.c
src/test/regress/expected/merge.out
src/test/regress/sql/merge.sql

index b8810f8b33e3abd41eee60c68d3bee0b17e2e1bb..a31c24c3cc303e18a8b519cdcf08824783602c9c 100644 (file)
@@ -64,6 +64,7 @@
 #include "nodes/nodeFuncs.h"
 #include "optimizer/optimizer.h"
 #include "rewrite/rewriteHandler.h"
+#include "rewrite/rewriteManip.h"
 #include "storage/bufmgr.h"
 #include "storage/lmgr.h"
 #include "utils/builtins.h"
@@ -3394,6 +3395,7 @@ ExecInitMerge(ModifyTableState *mtstate, EState *estate)
                        switch (action->commandType)
                        {
                                case CMD_INSERT:
+                                       /* INSERT actions always use rootRelInfo */
                                        ExecCheckPlanOutput(rootRelInfo->ri_RelationDesc,
                                                                                action->targetList);
 
@@ -3433,9 +3435,23 @@ ExecInitMerge(ModifyTableState *mtstate, EState *estate)
                                        }
                                        else
                                        {
-                                               /* not partitioned? use the stock relation and slot */
-                                               tgtslot = resultRelInfo->ri_newTupleSlot;
-                                               tgtdesc = RelationGetDescr(resultRelInfo->ri_RelationDesc);
+                                               /*
+                                                * If the MERGE targets an inherited table, we insert
+                                                * into the root table, so we must initialize its
+                                                * "new" tuple slot, if not already done, and use its
+                                                * relation descriptor for the projection.
+                                                *
+                                                * For non-inherited tables, rootRelInfo and
+                                                * resultRelInfo are the same, and the "new" tuple
+                                                * slot will already have been initialized.
+                                                */
+                                               if (rootRelInfo->ri_newTupleSlot == NULL)
+                                                       rootRelInfo->ri_newTupleSlot =
+                                                               table_slot_create(rootRelInfo->ri_RelationDesc,
+                                                                                                 &estate->es_tupleTable);
+
+                                               tgtslot = rootRelInfo->ri_newTupleSlot;
+                                               tgtdesc = RelationGetDescr(rootRelInfo->ri_RelationDesc);
                                        }
 
                                        action_state->mas_proj =
@@ -3468,6 +3484,77 @@ ExecInitMerge(ModifyTableState *mtstate, EState *estate)
                        }
                }
        }
+
+       /*
+        * If the MERGE targets an inherited table, any INSERT actions will use
+        * rootRelInfo, and rootRelInfo will not be in the resultRelInfo array.
+        * Therefore we must initialize its WITH CHECK OPTION constraints, as
+        * ExecInitModifyTable did for the resultRelInfo entries, but there should
+        * be nothing to do for RETURNING, since MERGE does not support RETURNING.
+        *
+        * Note that the planner does not build a withCheckOptionList for the root
+        * relation, but as in ExecInitPartitionInfo, we can use the first
+        * resultRelInfo entry as a reference to calculate the attno's for the
+        * root table.
+        */
+       if (rootRelInfo != mtstate->resultRelInfo &&
+               rootRelInfo->ri_RelationDesc->rd_rel->relkind != RELKIND_PARTITIONED_TABLE &&
+               (mtstate->mt_merge_subcommands & MERGE_INSERT) != 0)
+       {
+               Relation        rootRelation = rootRelInfo->ri_RelationDesc;
+               Relation        firstResultRel = mtstate->resultRelInfo[0].ri_RelationDesc;
+
+               if (node->withCheckOptionLists != NIL)
+               {
+                       List       *wcoList;
+                       List       *wcoExprs = NIL;
+
+                       /* There should be as many WCO lists as result rels */
+                       Assert(list_length(node->withCheckOptionLists) ==
+                                  list_length(node->resultRelations));
+
+                       /*
+                        * Use the first WCO list as a reference. In the most common case,
+                        * this will be for the same relation as rootRelInfo, and so there
+                        * will be no need to adjust its attno's.
+                        */
+                       wcoList = linitial(node->withCheckOptionLists);
+                       if (rootRelation != firstResultRel)
+                       {
+                               int                     firstVarno = mtstate->resultRelInfo[0].ri_RangeTableIndex;
+                               AttrMap    *part_attmap;
+                               bool            found_whole_row;
+
+                               /* Convert any Vars in it to contain the root's attno's */
+                               part_attmap =
+                                       build_attrmap_by_name(RelationGetDescr(rootRelation),
+                                                                                 RelationGetDescr(firstResultRel),
+                                                                                 false);
+
+                               wcoList = (List *)
+                                       map_variable_attnos((Node *) wcoList,
+                                                                               firstVarno, 0,
+                                                                               part_attmap,
+                                                                               RelationGetForm(rootRelation)->reltype,
+                                                                               &found_whole_row);
+                       }
+
+                       foreach(lc, wcoList)
+                       {
+                               WithCheckOption *wco = lfirst_node(WithCheckOption, lc);
+                               ExprState  *wcoExpr = ExecInitQual(castNode(List, wco->qual),
+                                                                                                  &mtstate->ps);
+
+                               wcoExprs = lappend(wcoExprs, wcoExpr);
+                       }
+
+                       rootRelInfo->ri_WithCheckOptions = wcoList;
+                       rootRelInfo->ri_WithCheckOptionExprs = wcoExprs;
+               }
+
+               /* MERGE does not support RETURNING */
+               Assert(node->returningLists == NIL);
+       }
 }
 
 /*
index 5d1be9f6b24589df2a01b255cbf091dc0d002909..fbc47c47f00cab9b5a7a3994f6bf8e785d4b823f 100644 (file)
@@ -2311,6 +2311,64 @@ SELECT * FROM new_measurement ORDER BY city_id, logdate;
        1 | 01-17-2007 |          |          
 (2 rows)
 
+-- MERGE into inheritance root table
+DROP TRIGGER insert_measurement_trigger ON measurement;
+ALTER TABLE measurement ADD CONSTRAINT mcheck CHECK (city_id = 0) NO INHERIT;
+EXPLAIN (COSTS OFF)
+MERGE INTO measurement m
+ USING (VALUES (1, '01-17-2007'::date)) nm(city_id, logdate) ON
+      (m.city_id = nm.city_id and m.logdate=nm.logdate)
+WHEN NOT MATCHED THEN INSERT
+     (city_id, logdate, peaktemp, unitsales)
+   VALUES (city_id - 1, logdate, 25, 100);
+                                QUERY PLAN                                
+--------------------------------------------------------------------------
+ Merge on measurement m
+   Merge on measurement_y2007m01 m_1
+   ->  Nested Loop Left Join
+         ->  Result
+         ->  Seq Scan on measurement_y2007m01 m_1
+               Filter: ((city_id = 1) AND (logdate = '01-17-2007'::date))
+(6 rows)
+
+BEGIN;
+MERGE INTO measurement m
+ USING (VALUES (1, '01-17-2007'::date)) nm(city_id, logdate) ON
+      (m.city_id = nm.city_id and m.logdate=nm.logdate)
+WHEN NOT MATCHED THEN INSERT
+     (city_id, logdate, peaktemp, unitsales)
+   VALUES (city_id - 1, logdate, 25, 100);
+SELECT * FROM ONLY measurement ORDER BY city_id, logdate;
+ city_id |  logdate   | peaktemp | unitsales 
+---------+------------+----------+-----------
+       0 | 07-21-2005 |       25 |        35
+       0 | 01-17-2007 |       25 |       100
+(2 rows)
+
+ROLLBACK;
+ALTER TABLE measurement ENABLE ROW LEVEL SECURITY;
+ALTER TABLE measurement FORCE ROW LEVEL SECURITY;
+CREATE POLICY measurement_p ON measurement USING (peaktemp IS NOT NULL);
+MERGE INTO measurement m
+ USING (VALUES (1, '01-17-2007'::date)) nm(city_id, logdate) ON
+      (m.city_id = nm.city_id and m.logdate=nm.logdate)
+WHEN NOT MATCHED THEN INSERT
+     (city_id, logdate, peaktemp, unitsales)
+   VALUES (city_id - 1, logdate, NULL, 100); -- should fail
+ERROR:  new row violates row-level security policy for table "measurement"
+MERGE INTO measurement m
+ USING (VALUES (1, '01-17-2007'::date)) nm(city_id, logdate) ON
+      (m.city_id = nm.city_id and m.logdate=nm.logdate)
+WHEN NOT MATCHED THEN INSERT
+     (city_id, logdate, peaktemp, unitsales)
+   VALUES (city_id - 1, logdate, 25, 100); -- ok
+SELECT * FROM ONLY measurement ORDER BY city_id, logdate;
+ city_id |  logdate   | peaktemp | unitsales 
+---------+------------+----------+-----------
+       0 | 07-21-2005 |       25 |        35
+       0 | 01-17-2007 |       25 |       100
+(2 rows)
+
 DROP TABLE measurement, new_measurement CASCADE;
 NOTICE:  drop cascades to 3 other objects
 DETAIL:  drop cascades to table measurement_y2006m02
index 4609241b2a580f138fe8a15d37ad77e2026a6433..d357471c35027d5b1d29eecfaa66ab1ecc195000 100644 (file)
@@ -1500,6 +1500,47 @@ WHEN MATCHED THEN DELETE;
 
 SELECT * FROM new_measurement ORDER BY city_id, logdate;
 
+-- MERGE into inheritance root table
+DROP TRIGGER insert_measurement_trigger ON measurement;
+ALTER TABLE measurement ADD CONSTRAINT mcheck CHECK (city_id = 0) NO INHERIT;
+
+EXPLAIN (COSTS OFF)
+MERGE INTO measurement m
+ USING (VALUES (1, '01-17-2007'::date)) nm(city_id, logdate) ON
+      (m.city_id = nm.city_id and m.logdate=nm.logdate)
+WHEN NOT MATCHED THEN INSERT
+     (city_id, logdate, peaktemp, unitsales)
+   VALUES (city_id - 1, logdate, 25, 100);
+
+BEGIN;
+MERGE INTO measurement m
+ USING (VALUES (1, '01-17-2007'::date)) nm(city_id, logdate) ON
+      (m.city_id = nm.city_id and m.logdate=nm.logdate)
+WHEN NOT MATCHED THEN INSERT
+     (city_id, logdate, peaktemp, unitsales)
+   VALUES (city_id - 1, logdate, 25, 100);
+SELECT * FROM ONLY measurement ORDER BY city_id, logdate;
+ROLLBACK;
+
+ALTER TABLE measurement ENABLE ROW LEVEL SECURITY;
+ALTER TABLE measurement FORCE ROW LEVEL SECURITY;
+CREATE POLICY measurement_p ON measurement USING (peaktemp IS NOT NULL);
+
+MERGE INTO measurement m
+ USING (VALUES (1, '01-17-2007'::date)) nm(city_id, logdate) ON
+      (m.city_id = nm.city_id and m.logdate=nm.logdate)
+WHEN NOT MATCHED THEN INSERT
+     (city_id, logdate, peaktemp, unitsales)
+   VALUES (city_id - 1, logdate, NULL, 100); -- should fail
+
+MERGE INTO measurement m
+ USING (VALUES (1, '01-17-2007'::date)) nm(city_id, logdate) ON
+      (m.city_id = nm.city_id and m.logdate=nm.logdate)
+WHEN NOT MATCHED THEN INSERT
+     (city_id, logdate, peaktemp, unitsales)
+   VALUES (city_id - 1, logdate, 25, 100); -- ok
+SELECT * FROM ONLY measurement ORDER BY city_id, logdate;
+
 DROP TABLE measurement, new_measurement CASCADE;
 DROP FUNCTION measurement_insert_trigger();