From: Anastasia Lubennikova <a.lubennikova@postgrespro.ru>
To: Tom Lane <tgl@sss.pgh.pa.us>
To: Yugo Nagata <nagata@sraoss.co.jp>
Cc: pgsql-hackers@postgresql.org
Subject: Re: CURRENT OF causes an error when IndexOnlyScan is used
Date: Mon, 19 Feb 2018 17:36:55 +0300
Message-ID: <97dc81d3-0ea9-3032-49d9-7983e35fb010@postgrespro.ru> (raw)
In-Reply-To: <22958.1517451171@sss.pgh.pa.us>
References: <20180201013349.937dfc5f.nagata@sraoss.co.jp>
<20180201014444.f78358df.nagata@sraoss.co.jp>
<22958.1517451171@sss.pgh.pa.us>
01.02.2018 05:12, Tom Lane:
> Yugo Nagata <nagata@sraoss.co.jp> writes:
>> I'm sorry the patch attached in the previous mail is broken and
>> not raises a compile error. I attached the fixed patch.
> This patch is almost certainly wrong: you can't assume that the scan-level
> state matches the tuple we are currently processing at top level. Any
> sort of delaying action, for instance a sort or materialize node in
> between, would break it.
>
> We need to either fix this aspect:
>
>>> IndexOnlyScan returns a virtual tuple that doesn't have system
>>> column, so we can not get ctid in the same way of other plans.
I'd like to propose the patch that fixes the issue.
We already have a way to return heaptuple from IndexOnlyScan,
but it was not applied to b-tree for some reason.
Attached patch solves the reported bug.
Moreover, it will come in handy for "index with included attributes"
feature [1],
where we can store long (and even TOASTed) attributes in indextuple.
[1] https://commitfest.postgresql.org/17/1350/
--
Anastasia Lubennikova
Postgres Professional: http://www.postgrespro.com
The Russian Postgres Company
Attachments:return_heaptuple_in_btree_indexonlyscan_v1.patch (text/x-patch, 4.9K) downloaddiff --git a/src/backend/access/nbtree/nbtree.c b/src/backend/access/nbtree/nbtree.cindex 8158508..db8a55c 100644--- a/src/backend/access/nbtree/nbtree.c+++ b/src/backend/access/nbtree/nbtree.c@@ -374,6 +374,8 @@ btbeginscan(Relation rel, int nkeys, int norderbys)
so->currTuples = so->markTuples = NULL;
scan->xs_itupdesc = RelationGetDescr(rel);
+ scan->xs_hitupdesc = NULL;+ scan->xs_hitup = NULL;
scan->opaque = so;
diff --git a/src/backend/access/nbtree/nbtsearch.c b/src/backend/access/nbtree/nbtsearch.cindex 51dca64..dd3e8b2 100644--- a/src/backend/access/nbtree/nbtsearch.c+++ b/src/backend/access/nbtree/nbtsearch.c@@ -25,6 +25,7 @@
#include "utils/tqual.h"
+static HeapTuple _bt_fetch_tuple(IndexScanDesc scandesc);
static bool _bt_readpage(IndexScanDesc scan, ScanDirection dir,
OffsetNumber offnum);
static void _bt_saveitem(BTScanOpaque so, int itemIndex,
@@ -38,6 +39,31 @@ static bool _bt_endpoint(IndexScanDesc scan, ScanDirection dir);
static void _bt_drop_lock_and_maybe_pin(IndexScanDesc scan, BTScanPos sp);
static inline void _bt_initialize_more_data(BTScanOpaque so, ScanDirection dir);
+/*+ * Fetch all keys in tuple.+ * Returns a new HeapTuple containing the originally-indexed data.+ */+static HeapTuple+_bt_fetch_tuple(IndexScanDesc scandesc)+{+ Relation index = scandesc->indexRelation;+ Datum fetchatt[INDEX_MAX_KEYS];+ bool isnull[INDEX_MAX_KEYS];+ int i;+ HeapTuple htuple;++ for (i = 0; i < index->rd_att->natts; i++)+ {+ fetchatt[i] = index_getattr(scandesc->xs_itup, i + 1,+ scandesc->xs_itupdesc, &isnull[i]);+ }++ htuple = heap_form_tuple(scandesc->xs_hitupdesc, fetchatt, isnull);+ htuple->t_tableOid = scandesc->heapRelation->rd_id;+++ return htuple;+}
/*
* _bt_drop_lock_and_maybe_pin()
@@ -1105,8 +1131,32 @@ readcomplete:
/* OK, itemIndex says what to return */
currItem = &so->currPos.items[so->currPos.itemIndex];
scan->xs_ctup.t_self = currItem->heapTid;
+
if (scan->xs_want_itup)
+ {+ if (!scan->xs_hitupdesc)+ {+ int natts = RelationGetNumberOfAttributes(scan->indexRelation);+ int attno;+ /*+ * The storage type of the index can be different from the original+ * datatype being indexed, so we cannot just grab the index's tuple+ * descriptor. Instead, construct a descriptor with the original data+ * types.+ */+ scan->xs_hitupdesc = CreateTemplateTupleDesc(natts, false);+ for (attno = 1; attno <= natts; attno++)+ {+ TupleDescInitEntry(scan->xs_hitupdesc, attno, NULL,+ scan->indexRelation->rd_opcintype[attno - 1],+ -1, 0);+ }+ }+
scan->xs_itup = (IndexTuple) (so->currTuples + currItem->tupleOffset);
+ scan->xs_hitup = _bt_fetch_tuple(scan);+ scan->xs_hitup->t_self = currItem->heapTid;+ }
return true;
}
@@ -1155,8 +1205,34 @@ _bt_next(IndexScanDesc scan, ScanDirection dir)
/* OK, itemIndex says what to return */
currItem = &so->currPos.items[so->currPos.itemIndex];
scan->xs_ctup.t_self = currItem->heapTid;
+
if (scan->xs_want_itup)
+ {+ if (!scan->xs_hitupdesc)+ {+ int natts = RelationGetNumberOfAttributes(scan->indexRelation);+ int attno;+ /*+ * The storage type of the index can be different from the original+ * datatype being indexed, so we cannot just grab the index's tuple+ * descriptor. Instead, construct a descriptor with the original data+ * types.+ */+ scan->xs_hitupdesc = CreateTemplateTupleDesc(natts, false);+ for (attno = 1; attno <= natts; attno++)+ {+ TupleDescInitEntry(scan->xs_hitupdesc, attno, NULL,+ scan->indexRelation->rd_opcintype[attno - 1],+ -1, 0);+ }+ }+
scan->xs_itup = (IndexTuple) (so->currTuples + currItem->tupleOffset);
+ if (scan->xs_hitup)+ pfree(scan->xs_hitup);+ scan->xs_hitup = _bt_fetch_tuple(scan);+ scan->xs_hitup->t_self = currItem->heapTid;+ }
return true;
}
@@ -1932,8 +2008,34 @@ _bt_endpoint(IndexScanDesc scan, ScanDirection dir)
/* OK, itemIndex says what to return */
currItem = &so->currPos.items[so->currPos.itemIndex];
scan->xs_ctup.t_self = currItem->heapTid;
+
if (scan->xs_want_itup)
+ {+ if (!scan->xs_hitupdesc)+ {+ int natts = RelationGetNumberOfAttributes(scan->indexRelation);+ int attno;+ /*+ * The storage type of the index can be different from the original+ * datatype being indexed, so we cannot just grab the index's tuple+ * descriptor. Instead, construct a descriptor with the original data+ * types.+ */+ scan->xs_hitupdesc = CreateTemplateTupleDesc(natts, false);+ for (attno = 1; attno <= natts; attno++)+ {+ TupleDescInitEntry(scan->xs_hitupdesc, attno, NULL,+ scan->indexRelation->rd_opcintype[attno - 1],+ -1, 0);+ }+ }+
scan->xs_itup = (IndexTuple) (so->currTuples + currItem->tupleOffset);
+ if (scan->xs_hitup)+ pfree(scan->xs_hitup);+ scan->xs_hitup = _bt_fetch_tuple(scan);+ scan->xs_hitup->t_self = currItem->heapTid;+ }
return true;
}
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: a.lubennikova@postgrespro.ru, tgl@sss.pgh.pa.us, nagata@sraoss.co.jp
Subject: Re: CURRENT OF causes an error when IndexOnlyScan is used
In-Reply-To: <97dc81d3-0ea9-3032-49d9-7983e35fb010@postgrespro.ru>
* 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