pg.ddx.io  pgsql-hackers@postgresql.org mailing list archive  
help / color / mirror / Atom feed
From: Yugo Nagata <nagata@sraoss.co.jp>
To: Tom Lane <tgl@sss.pgh.pa.us>
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: Thu, 15 Mar 2018 17:04:13 +0900
Message-ID: <20180315170413.abca0c2b.nagata@sraoss.co.jp> (raw)
In-Reply-To: <13902.1520877384@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>

On Mon, 12 Mar 2018 13:56:24 -0400
Tom Lane <tgl@sss.pgh.pa.us> wrote:

> Anastasia Lubennikova <a.lubennikova@postgrespro.ru> writes:
> > [ return_heaptuple_in_btree_indexonlyscan_v2.patch ]
> 
> I took a quick look at this, but I'm concerned about a couple of points:
> 
> 1. What's the performance penalty of forming (and then deforming) the
> added heap tuple?  We'd be paying it for every index-only scan, whether
> or not any CURRENT OF fetch ever happened.
> 
> 2. The constructed tuple provides tableoid and ctid all right, but it'd
> still have garbage values for other system columns.  As the code stands,
> we will properly error out if some attempt is made to fetch any of those
> other columns, but with this change we'd just return the garbage.  This
> is a minor point, but it doesn't seem negligible to me; it might've been
> hard to identify the bug at hand if we'd not had the cross-check of not
> building a heap tuple.

In addition, this patch fixes only nbtree indexes, but the simillar problem
will occur also on GIST indexes which support index-only scan. If we resolve
this bug by fixing index access methods, I think we need to fix all existing
indexes that support index-only scan and also describe the specification in
the documents, comments, or README, etc. for future indexes.
 
> At this point Yugo-san's original patch is starting to look more
> attractive.  It's still ugly, but at least it's not imposing a performance
> cost on unrelated queries.

I would like to elaborate my patch if needed and possible. Any suggestion
would be appriciated.

Thanks,

> 
> 			regards, tom lane



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




view thread (11+ messages)  latest in thread

Message-ID: <20180315170413.abca0c2b.nagata@sraoss.co.jp>
Permalink:  ../20180315170413.abca0c2b.nagata@sraoss.co.jp/
Also on:    postgresql.org/message-id/20180315170413.abca0c2b.nagata@sraoss.co.jp

 · 

reply

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: nagata@sraoss.co.jp, tgl@sss.pgh.pa.us, a.lubennikova@postgrespro.ru, pgsql-hackers@lists.postgresql.org
  Subject: Re: CURRENT OF causes an error when IndexOnlyScan is used
  In-Reply-To: <20180315170413.abca0c2b.nagata@sraoss.co.jp>

* 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