summaryrefslogtreecommitdiff
path: root/src/test/regress
diff options
context:
space:
mode:
authorTom Lane <tgl@sss.pgh.pa.us>2018-09-23 16:05:45 -0400
committerTom Lane <tgl@sss.pgh.pa.us>2018-09-23 16:05:45 -0400
commit5ed281e21d363aa1661587c331a022d7e6763d0c (patch)
treec92c1a1e420b02e7505204221138268ecfaadad3 /src/test/regress
parent1927e431dd69daece3152f8264df1a7ba1fcea9b (diff)
Fix failure in WHERE CURRENT OF after rewinding the referenced cursor.
In a case where we have multiple relation-scan nodes in a cursor plan, such as a scan of an inheritance tree, it's possible to fetch from a given scan node, then rewind the cursor and fetch some row from an earlier scan node. In such a case, execCurrent.c mistakenly thought that the later scan node was still active, because ExecReScan hadn't done anything to make it look not-active. We'd get some sort of failure in the case of a SeqScan node, because the node's scan tuple slot would be pointing at a HeapTuple whose t_self gets reset to invalid by heapam.c. But it seems possible that for other relation scan node types we'd actually return a valid tuple TID to the caller, resulting in updating or deleting a tuple that shouldn't have been considered current. To fix, forcibly clear the ScanTupleSlot in ExecScanReScan. Another issue here, which seems only latent at the moment but could easily become a live bug in future, is that rewinding a cursor does not necessarily lead to *immediately* applying ExecReScan to every scan-level node in the plan tree. Upper-level nodes will think that they can postpone that call if their child node is already marked with chgParam flags. I don't see a way for that to happen today in a plan tree that's simple enough for execCurrent.c's search_plan_tree to understand, but that's one heck of a fragile assumption. So, add some logic in search_plan_tree to detect chgParam flags being set on nodes that it descended to/through, and assume that that means we should consider lower scan nodes to be logically reset even if their ReScan call hasn't actually happened yet. Per bug #15395 from Matvey Arye. This has been broken for a long time, so back-patch to all supported branches. Discussion: https://postgr.es/m/153764171023.14986.280404050547008575@wrigleys.postgresql.org
Diffstat (limited to 'src/test/regress')
-rw-r--r--src/test/regress/expected/portals.out70
-rw-r--r--src/test/regress/sql/portals.sql24
2 files changed, 94 insertions, 0 deletions
diff --git a/src/test/regress/expected/portals.out b/src/test/regress/expected/portals.out
index 048b2fc3e3a..dc0d2ef7dd8 100644
--- a/src/test/regress/expected/portals.out
+++ b/src/test/regress/expected/portals.out
@@ -1270,6 +1270,76 @@ SELECT stringu1 FROM onek WHERE stringu1 = 'DZAAAA';
(0 rows)
ROLLBACK;
+-- Check behavior with rewinding to a previous child scan node,
+-- as per bug #15395
+BEGIN;
+CREATE TABLE current_check (currentid int, payload text);
+CREATE TABLE current_check_1 () INHERITS (current_check);
+CREATE TABLE current_check_2 () INHERITS (current_check);
+INSERT INTO current_check_1 SELECT i, 'p' || i FROM generate_series(1,9) i;
+INSERT INTO current_check_2 SELECT i, 'P' || i FROM generate_series(10,19) i;
+DECLARE c1 SCROLL CURSOR FOR SELECT * FROM current_check;
+-- This tests the fetch-backwards code path
+FETCH ABSOLUTE 12 FROM c1;
+ currentid | payload
+-----------+---------
+ 12 | P12
+(1 row)
+
+FETCH ABSOLUTE 8 FROM c1;
+ currentid | payload
+-----------+---------
+ 8 | p8
+(1 row)
+
+DELETE FROM current_check WHERE CURRENT OF c1 RETURNING *;
+ currentid | payload
+-----------+---------
+ 8 | p8
+(1 row)
+
+-- This tests the ExecutorRewind code path
+FETCH ABSOLUTE 13 FROM c1;
+ currentid | payload
+-----------+---------
+ 13 | P13
+(1 row)
+
+FETCH ABSOLUTE 1 FROM c1;
+ currentid | payload
+-----------+---------
+ 1 | p1
+(1 row)
+
+DELETE FROM current_check WHERE CURRENT OF c1 RETURNING *;
+ currentid | payload
+-----------+---------
+ 1 | p1
+(1 row)
+
+SELECT * FROM current_check;
+ currentid | payload
+-----------+---------
+ 2 | p2
+ 3 | p3
+ 4 | p4
+ 5 | p5
+ 6 | p6
+ 7 | p7
+ 9 | p9
+ 10 | P10
+ 11 | P11
+ 12 | P12
+ 13 | P13
+ 14 | P14
+ 15 | P15
+ 16 | P16
+ 17 | P17
+ 18 | P18
+ 19 | P19
+(17 rows)
+
+ROLLBACK;
-- Make sure snapshot management works okay, per bug report in
-- 235395b90909301035v7228ce63q392931f15aa74b31@mail.gmail.com
BEGIN;
diff --git a/src/test/regress/sql/portals.sql b/src/test/regress/sql/portals.sql
index d1a589094ea..52560ac0275 100644
--- a/src/test/regress/sql/portals.sql
+++ b/src/test/regress/sql/portals.sql
@@ -472,6 +472,30 @@ DELETE FROM onek WHERE CURRENT OF c1;
SELECT stringu1 FROM onek WHERE stringu1 = 'DZAAAA';
ROLLBACK;
+-- Check behavior with rewinding to a previous child scan node,
+-- as per bug #15395
+BEGIN;
+CREATE TABLE current_check (currentid int, payload text);
+CREATE TABLE current_check_1 () INHERITS (current_check);
+CREATE TABLE current_check_2 () INHERITS (current_check);
+INSERT INTO current_check_1 SELECT i, 'p' || i FROM generate_series(1,9) i;
+INSERT INTO current_check_2 SELECT i, 'P' || i FROM generate_series(10,19) i;
+
+DECLARE c1 SCROLL CURSOR FOR SELECT * FROM current_check;
+
+-- This tests the fetch-backwards code path
+FETCH ABSOLUTE 12 FROM c1;
+FETCH ABSOLUTE 8 FROM c1;
+DELETE FROM current_check WHERE CURRENT OF c1 RETURNING *;
+
+-- This tests the ExecutorRewind code path
+FETCH ABSOLUTE 13 FROM c1;
+FETCH ABSOLUTE 1 FROM c1;
+DELETE FROM current_check WHERE CURRENT OF c1 RETURNING *;
+
+SELECT * FROM current_check;
+ROLLBACK;
+
-- Make sure snapshot management works okay, per bug report in
-- 235395b90909301035v7228ce63q392931f15aa74b31@mail.gmail.com
BEGIN;