Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly
Appearance settings

Commit 8f5ac44

Browse filesBrowse files
tglsfdctglsfdc
committed
Fix WHERE CURRENT OF when the referenced cursor uses an index-only scan.
"UPDATE/DELETE WHERE CURRENT OF cursor_name" failed, with an error message like "cannot extract system attribute from virtual tuple", if the cursor was using a index-only scan for the target table. Fix it by digging the current TID out of the indexscan state. It seems likely that the same failure could occur for CustomScan plans and perhaps some FDW plan types, so that leaving this to be treated as an internal error with an obscure message isn't as good an idea as it first seemed. Hence, add a bit of heaptuple.c infrastructure to let us deliver a more on-topic message. I chose to make the message match what you get for the case where execCurrentOf can't identify the target scan node at all, "cursor "foo" is not a simply updatable scan of table "bar"". Perhaps it should be different, but we can always adjust that later. In the future, it might be nice to provide hooks that would let custom scan providers and/or FDWs deal with this in other ways; but that's not a suitable topic for a back-patchable bug fix. It's been like this all along, so back-patch to all supported branches. Yugo Nagata and Tom Lane Discussion: https://postgr.es/m/20180201013349.937dfc5f.nagata@sraoss.co.jp
1 parent e400840 commit 8f5ac44
Copy full SHA for 8f5ac44

File tree

Expand file treeCollapse file tree

5 files changed

+121
-20
lines changed
Filter options
Expand file treeCollapse file tree

5 files changed

+121
-20
lines changed

‎src/backend/access/common/heaptuple.c

Copy file name to clipboardExpand all lines: src/backend/access/common/heaptuple.c
+26Lines changed: 26 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1366,6 +1366,32 @@ slot_attisnull(TupleTableSlot *slot, int attnum)
13661366
return heap_attisnull(tuple, attnum);
13671367
}
13681368

1369+
/*
1370+
* slot_getsysattr
1371+
* This function fetches a system attribute of the slot's current tuple.
1372+
* Unlike slot_getattr, if the slot does not contain system attributes,
1373+
* this will return false (with a NULL attribute value) instead of
1374+
* throwing an error.
1375+
*/
1376+
bool
1377+
slot_getsysattr(TupleTableSlot *slot, int attnum,
1378+
Datum *value, bool *isnull)
1379+
{
1380+
HeapTuple tuple = slot->tts_tuple;
1381+
1382+
Assert(attnum < 0); /* else caller error */
1383+
if (tuple == NULL ||
1384+
tuple == &(slot->tts_minhdr))
1385+
{
1386+
/* No physical tuple, or minimal tuple, so fail */
1387+
*value = (Datum) 0;
1388+
*isnull = true;
1389+
return false;
1390+
}
1391+
*value = heap_getsysattr(tuple, attnum, slot->tts_tupleDescriptor, isnull);
1392+
return true;
1393+
}
1394+
13691395
/*
13701396
* heap_freetuple
13711397
*/

‎src/backend/executor/execCurrent.c

Copy file name to clipboardExpand all lines: src/backend/executor/execCurrent.c
+59-20Lines changed: 59 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,7 @@
1212
*/
1313
#include "postgres.h"
1414

15+
#include "access/relscan.h"
1516
#include "access/sysattr.h"
1617
#include "catalog/pg_type.h"
1718
#include "executor/executor.h"
@@ -149,16 +150,13 @@ execCurrentOf(CurrentOfExpr *cexpr,
149150
}
150151
else
151152
{
152-
ScanState *scanstate;
153-
bool lisnull;
154-
Oid tuple_tableoid PG_USED_FOR_ASSERTS_ONLY;
155-
ItemPointer tuple_tid;
156-
157153
/*
158154
* Without FOR UPDATE, we dig through the cursor's plan to find the
159155
* scan node. Fail if it's not there or buried underneath
160156
* aggregation.
161157
*/
158+
ScanState *scanstate;
159+
162160
scanstate = search_plan_tree(queryDesc->planstate, table_oid);
163161
if (!scanstate)
164162
ereport(ERROR,
@@ -183,21 +181,62 @@ execCurrentOf(CurrentOfExpr *cexpr,
183181
if (TupIsNull(scanstate->ss_ScanTupleSlot))
184182
return false;
185183

186-
/* Use slot_getattr to catch any possible mistakes */
187-
tuple_tableoid =
188-
DatumGetObjectId(slot_getattr(scanstate->ss_ScanTupleSlot,
189-
TableOidAttributeNumber,
190-
&lisnull));
191-
Assert(!lisnull);
192-
tuple_tid = (ItemPointer)
193-
DatumGetPointer(slot_getattr(scanstate->ss_ScanTupleSlot,
194-
SelfItemPointerAttributeNumber,
195-
&lisnull));
196-
Assert(!lisnull);
197-
198-
Assert(tuple_tableoid == table_oid);
199-
200-
*current_tid = *tuple_tid;
184+
/*
185+
* Extract TID of the scan's current row. The mechanism for this is
186+
* in principle scan-type-dependent, but for most scan types, we can
187+
* just dig the TID out of the physical scan tuple.
188+
*/
189+
if (IsA(scanstate, IndexOnlyScanState))
190+
{
191+
/*
192+
* For IndexOnlyScan, the tuple stored in ss_ScanTupleSlot may be
193+
* a virtual tuple that does not have the ctid column, so we have
194+
* to get the TID from xs_ctup.t_self.
195+
*/
196+
IndexScanDesc scan = ((IndexOnlyScanState *) scanstate)->ioss_ScanDesc;
197+
198+
*current_tid = scan->xs_ctup.t_self;
199+
}
200+
else
201+
{
202+
/*
203+
* Default case: try to fetch TID from the scan node's current
204+
* tuple. As an extra cross-check, verify tableoid in the current
205+
* tuple. If the scan hasn't provided a physical tuple, we have
206+
* to fail.
207+
*/
208+
Datum ldatum;
209+
bool lisnull;
210+
ItemPointer tuple_tid;
211+
212+
#ifdef USE_ASSERT_CHECKING
213+
if (!slot_getsysattr(scanstate->ss_ScanTupleSlot,
214+
TableOidAttributeNumber,
215+
&ldatum,
216+
&lisnull))
217+
ereport(ERROR,
218+
(errcode(ERRCODE_INVALID_CURSOR_STATE),
219+
errmsg("cursor \"%s\" is not a simply updatable scan of table \"%s\"",
220+
cursor_name, table_name)));
221+
Assert(!lisnull);
222+
Assert(DatumGetObjectId(ldatum) == table_oid);
223+
#endif
224+
225+
if (!slot_getsysattr(scanstate->ss_ScanTupleSlot,
226+
SelfItemPointerAttributeNumber,
227+
&ldatum,
228+
&lisnull))
229+
ereport(ERROR,
230+
(errcode(ERRCODE_INVALID_CURSOR_STATE),
231+
errmsg("cursor \"%s\" is not a simply updatable scan of table \"%s\"",
232+
cursor_name, table_name)));
233+
Assert(!lisnull);
234+
tuple_tid = (ItemPointer) DatumGetPointer(ldatum);
235+
236+
*current_tid = *tuple_tid;
237+
}
238+
239+
Assert(ItemPointerIsValid(current_tid));
201240

202241
return true;
203242
}

‎src/include/executor/tuptable.h

Copy file name to clipboardExpand all lines: src/include/executor/tuptable.h
+2Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -170,5 +170,7 @@ extern Datum slot_getattr(TupleTableSlot *slot, int attnum, bool *isnull);
170170
extern void slot_getallattrs(TupleTableSlot *slot);
171171
extern void slot_getsomeattrs(TupleTableSlot *slot, int attnum);
172172
extern bool slot_attisnull(TupleTableSlot *slot, int attnum);
173+
extern bool slot_getsysattr(TupleTableSlot *slot, int attnum,
174+
Datum *value, bool *isnull);
173175

174176
#endif /* TUPTABLE_H */

‎src/test/regress/expected/portals.out

Copy file name to clipboardExpand all lines: src/test/regress/expected/portals.out
+24Lines changed: 24 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1245,6 +1245,30 @@ FETCH FROM c1;
12451245

12461246
DELETE FROM ucview WHERE CURRENT OF c1; -- fail, views not supported
12471247
ERROR: WHERE CURRENT OF on a view is not implemented
1248+
ROLLBACK;
1249+
-- Check WHERE CURRENT OF with an index-only scan
1250+
BEGIN;
1251+
EXPLAIN (costs off)
1252+
DECLARE c1 CURSOR FOR SELECT stringu1 FROM onek WHERE stringu1 = 'DZAAAA';
1253+
QUERY PLAN
1254+
---------------------------------------------
1255+
Index Only Scan using onek_stringu1 on onek
1256+
Index Cond: (stringu1 = 'DZAAAA'::name)
1257+
(2 rows)
1258+
1259+
DECLARE c1 CURSOR FOR SELECT stringu1 FROM onek WHERE stringu1 = 'DZAAAA';
1260+
FETCH FROM c1;
1261+
stringu1
1262+
----------
1263+
DZAAAA
1264+
(1 row)
1265+
1266+
DELETE FROM onek WHERE CURRENT OF c1;
1267+
SELECT stringu1 FROM onek WHERE stringu1 = 'DZAAAA';
1268+
stringu1
1269+
----------
1270+
(0 rows)
1271+
12481272
ROLLBACK;
12491273
-- Make sure snapshot management works okay, per bug report in
12501274
-- 235395b90909301035v7228ce63q392931f15aa74b31@mail.gmail.com

‎src/test/regress/sql/portals.sql

Copy file name to clipboardExpand all lines: src/test/regress/sql/portals.sql
+10Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -462,6 +462,16 @@ FETCH FROM c1;
462462
DELETE FROM ucview WHERE CURRENT OF c1; -- fail, views not supported
463463
ROLLBACK;
464464

465+
-- Check WHERE CURRENT OF with an index-only scan
466+
BEGIN;
467+
EXPLAIN (costs off)
468+
DECLARE c1 CURSOR FOR SELECT stringu1 FROM onek WHERE stringu1 = 'DZAAAA';
469+
DECLARE c1 CURSOR FOR SELECT stringu1 FROM onek WHERE stringu1 = 'DZAAAA';
470+
FETCH FROM c1;
471+
DELETE FROM onek WHERE CURRENT OF c1;
472+
SELECT stringu1 FROM onek WHERE stringu1 = 'DZAAAA';
473+
ROLLBACK;
474+
465475
-- Make sure snapshot management works okay, per bug report in
466476
-- 235395b90909301035v7228ce63q392931f15aa74b31@mail.gmail.com
467477
BEGIN;

0 commit comments

Comments
0 (0)
Morty Proxy This is a proxified and sanitized view of the page, visit original site.