diff options
author | Tom Lane <tgl@sss.pgh.pa.us> | 2018-09-23 16:05:45 -0400 |
---|---|---|
committer | Tom Lane <tgl@sss.pgh.pa.us> | 2018-09-23 16:05:45 -0400 |
commit | 5ed281e21d363aa1661587c331a022d7e6763d0c (patch) | |
tree | c92c1a1e420b02e7505204221138268ecfaadad3 /src/test/regress | |
parent | 1927e431dd69daece3152f8264df1a7ba1fcea9b (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.out | 70 | ||||
-rw-r--r-- | src/test/regress/sql/portals.sql | 24 |
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; |