As part of the MERGE RETURNING patch I noticed a suspicious Assert()
in ExecInitPartitionInfo() that looked like it needed updating for
MERGE.
After more testing, I can confirm that this is indeed a pre-existing
bug, that can be triggered using MERGE into a partitioned table that
has RLS enabled (and hence non-empty withCheckOptionLists to
initialise).
So I think we need something like the attached.
Regards,
Dean
diff --git a/src/backend/executor/execPartition.c b/src/backend/executor/execPartition.c
new file mode 100644
index 651ad24..fd6ca8a
--- a/src/backend/executor/execPartition.c
+++ b/src/backend/executor/execPartition.c
@@ -545,8 +545,8 @@ ExecInitPartitionInfo(ModifyTableState *
* Build WITH CHECK OPTION constraints for the partition. Note that we
* didn't build the withCheckOptionList for partitions within the planner,
* but simple translation of varattnos will suffice. This only occurs for
- * the INSERT case or in the case of UPDATE tuple routing where we didn't
- * find a result rel to reuse.
+ * the INSERT case or in the case of UPDATE/MERGE tuple routing where we
+ * didn't find a result rel to reuse.
*/
if (node && node->withCheckOptionLists != NIL)
{
@@ -557,13 +557,16 @@ ExecInitPartitionInfo(ModifyTableState *
/*
* In the case of INSERT on a partitioned table, there is only one
* plan. Likewise, there is only one WCO list, not one per partition.
- * For UPDATE, there are as many WCO lists as there are plans.
+ * For UPDATE/MERGE, there are as many WCO lists as there are plans.
*/
Assert((node->operation == CMD_INSERT &&
list_length(node->withCheckOptionLists) == 1 &&
list_length(node->resultRelations) == 1) ||
(node->operation == CMD_UPDATE &&
list_length(node->withCheckOptionLists) ==
+ list_length(node->resultRelations)) ||
+ (node->operation == CMD_MERGE &&
+ list_length(node->withCheckOptionLists) ==
list_length(node->resultRelations)));
/*
@@ -619,6 +622,7 @@ ExecInitPartitionInfo(ModifyTableState *
List *returningList;
/* See the comment above for WCO lists. */
+ /* (except no RETURNING support for MERGE yet) */
Assert((node->operation == CMD_INSERT &&
list_length(node->returningLists) == 1 &&
list_length(node->resultRelations) == 1) ||
diff --git a/src/test/regress/expected/merge.out b/src/test/regress/expected/merge.out
new file mode 100644
index bc53b21..ae27f12
--- a/src/test/regress/expected/merge.out
+++ b/src/test/regress/expected/merge.out
@@ -1721,6 +1721,18 @@ SELECT * FROM pa_target ORDER BY tid;
(14 rows)
ROLLBACK;
+-- test RLS enforcement
+BEGIN;
+ALTER TABLE pa_target ENABLE ROW LEVEL SECURITY;
+ALTER TABLE pa_target FORCE ROW LEVEL SECURITY;
+CREATE POLICY pa_target_pol ON pa_target USING (tid != 0);
+MERGE INTO pa_target t
+ USING pa_source s
+ ON t.tid = s.sid AND t.tid IN (1,2,3,4)
+ WHEN MATCHED THEN
+ UPDATE SET tid = tid - 1;
+ERROR: new row violates row-level security policy for table "pa_target"
+ROLLBACK;
DROP TABLE pa_source;
DROP TABLE pa_target CASCADE;
-- Sub-partitioning
diff --git a/src/test/regress/sql/merge.sql b/src/test/regress/sql/merge.sql
new file mode 100644
index fdbcd70..3338d80
--- a/src/test/regress/sql/merge.sql
+++ b/src/test/regress/sql/merge.sql
@@ -1071,6 +1071,18 @@ MERGE INTO pa_target t
SELECT * FROM pa_target ORDER BY tid;
ROLLBACK;
+-- test RLS enforcement
+BEGIN;
+ALTER TABLE pa_target ENABLE ROW LEVEL SECURITY;
+ALTER TABLE pa_target FORCE ROW LEVEL SECURITY;
+CREATE POLICY pa_target_pol ON pa_target USING (tid != 0);
+MERGE INTO pa_target t
+ USING pa_source s
+ ON t.tid = s.sid AND t.tid IN (1,2,3,4)
+ WHEN MATCHED THEN
+ UPDATE SET tid = tid - 1;
+ROLLBACK;
+
DROP TABLE pa_source;
DROP TABLE pa_target CASCADE;
