From: Tom Lane <tgl@sss.pgh.pa.us>
To: Yugo Nagata <nagata@sraoss.co.jp>
Cc: Anastasia Lubennikova <a.lubennikova@postgrespro.ru>
Cc: pgsql-hackers@lists.postgresql.org
Subject: Re: CURRENT OF causes an error when IndexOnlyScan is used
Date: Fri, 16 Mar 2018 17:34:06 -0400
Message-ID: <1850.1521236046@sss.pgh.pa.us> (raw)
In-Reply-To: <31553.1521234300@sss.pgh.pa.us>
References: <20180201013349.937dfc5f.nagata@sraoss.co.jp>
<20180201014444.f78358df.nagata@sraoss.co.jp>
<22958.1517451171@sss.pgh.pa.us>
<97dc81d3-0ea9-3032-49d9-7983e35fb010@postgrespro.ru>
<20180220095252.GA18315@e733.localdomain>
<a6ddf99c-6579-e65a-6abd-c0b6259c15c2@postgrespro.ru>
<13902.1520877384@sss.pgh.pa.us>
<20180315170413.abca0c2b.nagata@sraoss.co.jp>
<31553.1521234300@sss.pgh.pa.us>
I wrote:
> In the meantime, we could fix execCurrent.c so that it throws
> FEATURE_NOT_SUPPORTED rather than the current failure if the slot it's
> looking at doesn't contain a physical tuple.
Concretely, I think we should do the attached, so as to cover any other
situations where the scan type doesn't return a physical tuple. I kept
it separate from your patch so it's easy to test (the original case you
gave now throws the appropriate error).
The existing error when execCurrentOf can't figure out what to do with
the plan is ERRCODE_INVALID_CURSOR_STATE with message
"cursor \"%s\" is not a simply updatable scan of table \"%s\""
so for this draft patch I just used that. I'm not sure if it would be
a better idea to throw a different SQLSTATE or different error text
for this case. Any thoughts on that?
regards, tom lane
Attachments:handle-where-current-of-failure-better.patch (text/x-diff, 3.9K) downloaddiff --git a/src/backend/access/common/heaptuple.c b/src/backend/access/common/heaptuple.cindex a2f67f2..c45a488 100644
*** a/src/backend/access/common/heaptuple.c
--- b/src/backend/access/common/heaptuple.c
*************** slot_attisnull(TupleTableSlot *slot, int
*** 1367,1372 ****
--- 1367,1398 ----
}
/*
+ * slot_getsysattr+ * This function fetches a system attribute of the slot's current tuple.+ * Unlike slot_getattr, if the slot does not contain system attributes,+ * this will return false (with a NULL attribute value) instead of+ * throwing an error.+ */+ bool+ slot_getsysattr(TupleTableSlot *slot, int attnum,+ Datum *value, bool *isnull)+ {+ HeapTuple tuple = slot->tts_tuple;+ + Assert(attnum < 0); /* else caller error */+ if (tuple == NULL ||+ tuple == &(slot->tts_minhdr))+ {+ /* No physical tuple, or minimal tuple, so fail */+ *value = (Datum) 0;+ *isnull = true;+ return false;+ }+ *value = heap_getsysattr(tuple, attnum, slot->tts_tupleDescriptor, isnull);+ return true;+ }+ + /*
* heap_freetuple
*/
void
diff --git a/src/backend/executor/execCurrent.c b/src/backend/executor/execCurrent.cindex ce7d4ac..2296c9b 100644
*** a/src/backend/executor/execCurrent.c
--- b/src/backend/executor/execCurrent.c
*************** execCurrentOf(CurrentOfExpr *cexpr,
*** 150,157 ****
else
{
ScanState *scanstate;
bool lisnull;
- Oid tuple_tableoid PG_USED_FOR_ASSERTS_ONLY;
ItemPointer tuple_tid;
/*
--- 150,157 ----
else
{
ScanState *scanstate;
+ Datum ldatum;
bool lisnull;
ItemPointer tuple_tid;
/*
*************** execCurrentOf(CurrentOfExpr *cexpr,
*** 183,201 ****
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;
--- 183,213 ----
if (TupIsNull(scanstate->ss_ScanTupleSlot))
return false;
! /*
! * Try to fetch tableoid and CTID from the scan node's current tuple.
! * If the scan type hasn't provided a physical tuple, we have to fail.
! */
! if (!slot_getsysattr(scanstate->ss_ScanTupleSlot,
! TableOidAttributeNumber,
! &ldatum,
! &lisnull))
! ereport(ERROR,
! (errcode(ERRCODE_INVALID_CURSOR_STATE),
! errmsg("cursor \"%s\" is not a simply updatable scan of table \"%s\"",
! cursor_name, table_name)));
Assert(!lisnull);
+ Assert(DatumGetObjectId(ldatum) == table_oid);
! if (!slot_getsysattr(scanstate->ss_ScanTupleSlot,
! SelfItemPointerAttributeNumber,
! &ldatum,
! &lisnull))
! ereport(ERROR,
! (errcode(ERRCODE_INVALID_CURSOR_STATE),
! errmsg("cursor \"%s\" is not a simply updatable scan of table \"%s\"",
! cursor_name, table_name)));
! Assert(!lisnull);
! tuple_tid = (ItemPointer) DatumGetPointer(ldatum);
*current_tid = *tuple_tid;
diff --git a/src/include/executor/tuptable.h b/src/include/executor/tuptable.hindex 0642a3a..a5779b1 100644
*** a/src/include/executor/tuptable.h
--- b/src/include/executor/tuptable.h
*************** extern Datum slot_getattr(TupleTableSlot
*** 170,174 ****
--- 170,176 ----
extern void slot_getallattrs(TupleTableSlot *slot);
extern void slot_getsomeattrs(TupleTableSlot *slot, int attnum);
extern bool slot_attisnull(TupleTableSlot *slot, int attnum);
+ extern bool slot_getsysattr(TupleTableSlot *slot, int attnum,+ Datum *value, bool *isnull);
#endif /* TUPTABLE_H */
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Reply to all the recipients using the --to and --cc options:
reply via email
To: pgsql-hackers@postgresql.org
Cc: tgl@sss.pgh.pa.us, nagata@sraoss.co.jp, a.lubennikova@postgrespro.ru, pgsql-hackers@lists.postgresql.org
Subject: Re: CURRENT OF causes an error when IndexOnlyScan is used
In-Reply-To: <1850.1521236046@sss.pgh.pa.us>
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
This inbox is served by DDX for PostgreSQL; see mirroring instructions
for how to clone and mirror all data and code used for this inbox