| From: | Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> |
|---|---|
| To: | pgsql-committers(at)lists(dot)postgresql(dot)org |
| Subject: | pgsql: Fix failure in WHERE CURRENT OF after rewinding the referenced c |
| Date: | 2018-09-23 20:06:06 |
| Message-ID: | [email protected] |
| Views: | Whole Thread | Raw Message | Download mbox | Resend email |
| Thread: | |
| Lists: | pgsql-committers |
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/[email protected]
Branch
------
REL9_4_STABLE
Details
-------
https://git.postgresql.org/pg/commitdiff/38cb010843636ed27713d54c70b455eb470d06b8
Modified Files
--------------
src/backend/executor/execCurrent.c | 62 +++++++++++++++++++++++--------
src/backend/executor/execScan.c | 6 +++
src/test/regress/expected/portals.out | 70 +++++++++++++++++++++++++++++++++++
src/test/regress/sql/portals.sql | 24 ++++++++++++
4 files changed, 146 insertions(+), 16 deletions(-)
| From | Date | Subject | |
|---|---|---|---|
| Next Message | Tom Lane | 2018-09-23 22:34:25 | pgsql: Doc: warn against using parallel restore with --load-via-partiti |
| Previous Message | Alexander Korotkov | 2018-09-22 13:27:50 | pgsql: Replace CAS loop with single TAS in ProcArrayGroupClearXid() |