Fix rowmark handling for non-relation RTEs during executor init

Commit cbc127917e introduced tracking of unpruned relids to skip
processing of pruned partitions. PlannedStmt.unprunableRelids is
computed as the difference between PlannerGlobal.allRelids and
prunableRelids, but allRelids only contains RTE_RELATION entries.
This means non-relation RTEs (VALUES, subqueries, CTEs, etc.) are
never included in unprunableRelids, and consequently not in
es_unpruned_relids at runtime.

As a result, rowmarks attached to non-relation RTEs were incorrectly
skipped during executor initialization. This affects any DML statement
that has rowmarks on such RTEs, including MERGE with a VALUES or
subquery source, and UPDATE/DELETE with joins against subqueries or
CTEs. When a concurrent update triggers an EPQ recheck, the missing
rowmark leads to incorrect results.

Fix by restricting the es_unpruned_relids membership check to
RTE_RELATION entries only, since partition pruning only applies to
actual relations. Rowmarks for other RTE kinds are now always
processed.

Bug: #19355
Reported-by: Bihua Wang <wangbihua.cn@gmail.com>
Diagnosed-by: Dean Rasheed <dean.a.rasheed@gmail.com>
Diagnosed-by: Tender Wang <tndrwang@gmail.com>
Author: Dean Rasheed <dean.a.rasheed@gmail.com>
Discussion: https://postgr.es/m/19355-57d7d52ea4980dc6@postgresql.org
Backpatch-through: 18

diff --git a/src/backend/executor/execMain.c b/src/backend/executor/execMain.c
index 27c9eec697b188941b767054eb868429b0990dd0..f4d2f41e28bb54b3c077cfd1fe15ecc9258049fa 100644 (file)
--- a/src/backend/executor/execMain.c
+++ b/src/backend/executor/execMain.c
@@ -880,21 +880,25 @@ InitPlan(QueryDesc *queryDesc, int eflags)
                foreach(l, plannedstmt->rowMarks)
                {
                        PlanRowMark *rc = (PlanRowMark *) lfirst(l);
+                       RangeTblEntry *rte = exec_rt_fetch(rc->rti, estate);
                        Oid                     relid;
                        Relation        relation;
                        ExecRowMark *erm;
 
+                       /* ignore "parent" rowmarks; they are irrelevant at runtime */
+                       if (rc->isParent)
+                               continue;
+
                        /*
-                        * Ignore "parent" rowmarks, because they are irrelevant at
-                        * runtime.  Also ignore the rowmarks belonging to child tables
-                        * that have been pruned in ExecDoInitialPruning().
+                        * Also ignore rowmarks belonging to child tables that have been
+                        * pruned in ExecDoInitialPruning().
                         */
-                       if (rc->isParent ||
+                       if (rte->rtekind == RTE_RELATION &&
                                !bms_is_member(rc->rti, estate->es_unpruned_relids))
                                continue;
 
                        /* get relation's OID (will produce InvalidOid if subquery) */
-                       relid = exec_rt_fetch(rc->rti, estate)->relid;
+                       relid = rte->relid;
 
                        /* open relation, if we need to access it for this mark type */
                        switch (rc->markType)

diff --git a/src/backend/executor/nodeLockRows.c b/src/backend/executor/nodeLockRows.c
index a8afbf93b4882c9023a60f7571964d28ae81831c..5d38bd4095ccf207079a63e52468097f975c3c9c 100644 (file)
--- a/src/backend/executor/nodeLockRows.c
+++ b/src/backend/executor/nodeLockRows.c
@@ -344,15 +344,19 @@ ExecInitLockRows(LockRows *node, EState *estate, int eflags)
        foreach(lc, node->rowMarks)
        {
                PlanRowMark *rc = lfirst_node(PlanRowMark, lc);
+               RangeTblEntry *rte = exec_rt_fetch(rc->rti, estate);
                ExecRowMark *erm;
                ExecAuxRowMark *aerm;
 
+               /* ignore "parent" rowmarks; they are irrelevant at runtime */
+               if (rc->isParent)
+                       continue;
+
                /*
-                * Ignore "parent" rowmarks, because they are irrelevant at runtime.
-                * Also ignore the rowmarks belonging to child tables that have been
+                * Also ignore rowmarks belonging to child tables that have been
                 * pruned in ExecDoInitialPruning().
                 */
-               if (rc->isParent ||
+               if (rte->rtekind == RTE_RELATION &&
                        !bms_is_member(rc->rti, estate->es_unpruned_relids))
                        continue;
 

diff --git a/src/backend/executor/nodeModifyTable.c b/src/backend/executor/nodeModifyTable.c
index 00429326c34a7ce3021f97efc8ee26ae445c7b61..48353d29ff62afd8443e756e69dcaf925308b157 100644 (file)
--- a/src/backend/executor/nodeModifyTable.c
+++ b/src/backend/executor/nodeModifyTable.c
@@ -5093,15 +5093,19 @@ ExecInitModifyTable(ModifyTable *node, EState *estate, int eflags)
        foreach(l, node->rowMarks)
        {
                PlanRowMark *rc = lfirst_node(PlanRowMark, l);
+               RangeTblEntry *rte = exec_rt_fetch(rc->rti, estate);
                ExecRowMark *erm;
                ExecAuxRowMark *aerm;
 
+               /* ignore "parent" rowmarks; they are irrelevant at runtime */
+               if (rc->isParent)
+                       continue;
+
                /*
-                * Ignore "parent" rowmarks, because they are irrelevant at runtime.
-                * Also ignore the rowmarks belonging to child tables that have been
+                * Also ignore rowmarks belonging to child tables that have been
                 * pruned in ExecDoInitialPruning().
                 */
-               if (rc->isParent ||
+               if (rte->rtekind == RTE_RELATION &&
                        !bms_is_member(rc->rti, estate->es_unpruned_relids))
                        continue;
 

diff --git a/src/test/isolation/expected/eval-plan-qual.out b/src/test/isolation/expected/eval-plan-qual.out
index 05fffe0d5708f281067e0e4f8f01ea791590768e..a122047fd2aef4573c41e48122e82968cc0bc551 100644 (file)
--- a/src/test/isolation/expected/eval-plan-qual.out
+++ b/src/test/isolation/expected/eval-plan-qual.out
@@ -1473,3 +1473,23 @@ step s2pp4: DELETE FROM another_parttbl WHERE a = (SELECT 1); <waiting ...>
 step c1: COMMIT;
 step s2pp4: <... completed>
 step c2: COMMIT;
+
+starting permutation: updateformergevalues mergevalues c1 c2 read
+step updateformergevalues: UPDATE accounts SET balance = balance + 100;
+step mergevalues: 
+       MERGE INTO accounts
+       USING (VALUES ('checking', 610), ('savings', 620)) v(accountid, balance)
+       ON v.accountid = accounts.accountid
+       WHEN MATCHED THEN UPDATE SET balance = v.balance
+       WHEN NOT MATCHED THEN INSERT VALUES ('unmatched', -1);
+ <waiting ...>
+step c1: COMMIT;
+step mergevalues: <... completed>
+step c2: COMMIT;
+step read: SELECT * FROM accounts ORDER BY accountid;
+accountid|balance|balance2
+---------+-------+--------
+checking |    610|    1220
+savings  |    620|    1240
+(2 rows)
+

diff --git a/src/test/isolation/specs/eval-plan-qual.spec b/src/test/isolation/specs/eval-plan-qual.spec
index 80e1e6bb307b3361aa4030ee6267d35301164ca5..fb57fb237ddfd0ea844bd0119dd98e17cc39ad36 100644 (file)
--- a/src/test/isolation/specs/eval-plan-qual.spec
+++ b/src/test/isolation/specs/eval-plan-qual.spec
@@ -206,6 +206,8 @@ step sys1   {
 
 step s1pp1 { UPDATE another_parttbl SET b = b + 1 WHERE a = 1; }
 
+step updateformergevalues { UPDATE accounts SET balance = balance + 100; }
+
 session s2
 setup          { BEGIN ISOLATION LEVEL READ COMMITTED; }
 step wx2       { UPDATE accounts SET balance = balance + 450 WHERE accountid = 'checking' RETURNING balance; }
@@ -318,6 +320,14 @@ step s2pp2 { PREPARE epd AS DELETE FROM another_parttbl WHERE a = $1; }
 step s2pp3 { EXECUTE epd(1); }
 step s2pp4 { DELETE FROM another_parttbl WHERE a = (SELECT 1); }
 
+step mergevalues {
+       MERGE INTO accounts
+       USING (VALUES ('checking', 610), ('savings', 620)) v(accountid, balance)
+       ON v.accountid = accounts.accountid
+       WHEN MATCHED THEN UPDATE SET balance = v.balance
+       WHEN NOT MATCHED THEN INSERT VALUES ('unmatched', -1);
+}
+
 session s3
 setup          { BEGIN ISOLATION LEVEL READ COMMITTED; }
 step read      { SELECT * FROM accounts ORDER BY accountid; }
@@ -425,3 +435,6 @@ permutation sys1 sysmerge2 c1 c2
 # Exercise run-time partition pruning code in an EPQ recheck
 permutation s1pp1 s2pp1 s2pp2 s2pp3 c1 c2
 permutation s1pp1 s2pp4 c1 c2
+
+# test EPQ recheck in MERGE from VALUES_RTE, cf bug #19355
+permutation updateformergevalues mergevalues c1 c2 read