Date: Thu, 1 Feb 2018 01:33:49 +0900
From: Yugo Nagata <nagata@sraoss.co.jp>
To: pgsql-hackers@postgresql.org
Subject: CURRENT OF causes an error when IndexOnlyScan is used
Message-Id: <20180201013349.937dfc5f.nagata@sraoss.co.jp>
Mime-Version: 1.0
Content-Type: multipart/mixed;
 boundary="Multipart=_Thu__1_Feb_2018_01_33_49_+0900_ySa7wxe+A=kZ5Lu="
Precedence: bulk

This is a multi-part message in MIME format.

--Multipart=_Thu__1_Feb_2018_01_33_49_+0900_ySa7wxe+A=kZ5Lu=
Content-Type: text/plain; charset=US-ASCII
Content-Transfer-Encoding: 7bit

Hi,

I found that updating a cursor by using CURRENT OF causes the
following error when the query is executed by IndexOnlyScan. 

 ERROR:  cannot extract system attribute from virtual tuple

IndexOnlyScan returns a virtual tuple that doesn't have system
column, so we can not get ctid in the same way of other plans.
However, the error message is not convinient and users would
not understand why the error occurs.

Attached is a patch to fix this. By this fix, execCurrentOf
get ctid from IndexScanDesc->xs_ctup.t_self when the plan is
IndexOnlyScan, and it works sucessfully without errors.


Here is the example of the error:

=======
postgres=# create table test (i int primary key);
CREATE TABLE
postgres=# insert into test values(1);
INSERT 0 1
postgres=# set enable_seqscan to off;
SET

postgres=# explain select * from test where i = 1;
                                QUERY PLAN                                 
---------------------------------------------------------------------------
 Index Only Scan using test_pkey on test  (cost=0.15..8.17 rows=1 width=4)
   Index Cond: (i = 1)
(2 rows)

postgres=# begin;
BEGIN
postgres=# declare c cursor for select * from test where i = 1;
DECLARE CURSOR
postgres=# fetch from c;
 i 
---
 1
(1 row)

postgres=# update test set i=i+1 where current of c;
ERROR:  cannot extract system attribute from virtual tuple
=======

The patch fixes the error and allows this update successfully.

Regards,

-- 
Yugo Nagata <nagata@sraoss.co.jp>

--Multipart=_Thu__1_Feb_2018_01_33_49_+0900_ySa7wxe+A=kZ5Lu=
Content-Type: text/x-diff;
 name="current_of_index_only_scan.patch"
Content-Disposition: attachment;
 filename="current_of_index_only_scan.patch"
Content-Transfer-Encoding: 7bit

diff --git a/src/backend/executor/execCurrent.c b/src/backend/executor/execCurrent.c
index ce7d4ac..b37cf3d 100644
--- a/src/backend/executor/execCurrent.c
+++ b/src/backend/executor/execCurrent.c
@@ -19,6 +19,7 @@
 #include "utils/lsyscache.h"
 #include "utils/portal.h"
 #include "utils/rel.h"
+#include "access/relscan.h"
 
 
 static char *fetch_cursor_param_value(ExprContext *econtext, int paramId);
@@ -183,21 +184,35 @@ execCurrentOf(CurrentOfExpr *cexpr,
 		if (TupIsNull(scanstate->ss_ScanTupleSlot))
 			return false;
 
-		/* Use slot_getattr to catch any possible mistakes */
-		tuple_tableoid =
-			DatumGetObjectId(slot_getattr(scanstate->ss_ScanTupleSlot,
-										  TableOidAttributeNumber,
-										  &lisnull));
-		Assert(!lisnull);
-		tuple_tid = (ItemPointer)
-			DatumGetPointer(slot_getattr(scanstate->ss_ScanTupleSlot,
-										 SelfItemPointerAttributeNumber,
-										 &lisnull));
-		Assert(!lisnull);
-
-		Assert(tuple_tableoid == table_oid);
-
-		*current_tid = *tuple_tid;
+		/* In IndexOnlyScan case, the tuple stored in ss_ScanTupleSlot is a
+		 * virtual tuple that does not have ctid column, so we have to get TID
+		 * from xs_ctup.t_self. */
+		if (IsA(scanstate, IndexOnlyScanState))
+		{
+			IndexScanDesc scan = ((IndexOnlyScanState *)scanstate)->ScanDesc;
+
+			Assert(RelationGetRelid(scan.heapRelation) == table_oid);
+
+			*current_tid = scan->xs_ctup.t_self;
+		}
+		else
+		{
+			/* Use slot_getattr to catch any possible mistakes */
+			tuple_tableoid =
+				DatumGetObjectId(slot_getattr(scanstate->ss_ScanTupleSlot,
+											  TableOidAttributeNumber,
+											  &lisnull));
+			Assert(!lisnull);
+			tuple_tid = (ItemPointer)
+				DatumGetPointer(slot_getattr(scanstate->ss_ScanTupleSlot,
+											 SelfItemPointerAttributeNumber,
+											 &lisnull));
+			Assert(!lisnull);
+
+			Assert(tuple_tableoid == table_oid);
+
+			*current_tid = *tuple_tid;
+		}
 
 		return true;
 	}

--Multipart=_Thu__1_Feb_2018_01_33_49_+0900_ySa7wxe+A=kZ5Lu=--