Date: Tue, 14 Mar 2017 16:16:09 -0300
From: Alvaro Herrera <alvherre@2ndquadrant.com>
To: Pavan Deolasee <pavan.deolasee@gmail.com>
Cc: Robert Haas <robertmhaas@gmail.com>, Bruce Momjian <bruce@momjian.us>,
	Jaime Casanova <jaime.casanova@2ndquadrant.com>,
	Haribabu Kommi <kommi.haribabu@gmail.com>,
	Tomas Vondra <tomas.vondra@2ndquadrant.com>,
	pgsql-hackers <pgsql-hackers@postgresql.org>
Subject: Re: Patch: Write Amplification Reduction Method (WARM)
Message-ID: <20170314191609.tzw6bz3uigbqmtif@alvherre.pgsql>
MIME-Version: 1.0
Content-Type: text/plain; charset=iso-8859-1
Content-Disposition: inline
Content-Transfer-Encoding: 8bit
In-Reply-To: 
 <CABOikdPAOmO7m=sFiNPoP6bd24Rq+_0yaRyRLKgdpQMqu7Y13g@mail.gmail.com>
User-Agent: NeoMutt/20170206-195-7adf12 (1.7.2)
Precedence: bulk
Sender: pgsql-hackers-owner@postgresql.org

Pavan Deolasee wrote:
> On Tue, Mar 14, 2017 at 7:17 AM, Alvaro Herrera <alvherre@2ndquadrant.com>
> wrote:

> > I have already commented about the executor involvement in btrecheck();
> > that doesn't seem good.  I previously suggested to pass the EState down
> > from caller, but that's not a great idea either since you still need to
> > do the actual FormIndexDatum.  I now think that a workable option would
> > be to compute the values/isnulls arrays so that btrecheck gets them
> > already computed.
> 
> I agree with your complaint about modularity violation. What I am unclear
> is how passing values/isnulls array will fix that. The way code is
> structured currently, recheck routines are called by index_fetch_heap(). So
> if we try to compute values/isnulls in that function, we'll still need
> access EState, which AFAIU will lead to similar violation. Or am I
> mis-reading your idea?

You're right, it's still a problem.  (Honestly, I think the whole idea
of trying to compute a fake index tuple starting from a just-read heap
tuple is a problem in itself; I just wonder if there's a way to do the
recheck that doesn't involve such a thing.)

> I wonder if we should instead invent something similar to IndexRecheck(),
> but instead of running ExecQual(), this new routine will compare the index
> values by the given HeapTuple against given IndexTuple. ISTM that for this
> to work we'll need to modify all callers of index_getnext() and teach them
> to invoke the AM specific recheck method if xs_tuple_recheck flag is set to
> true by index_getnext().

Yeah, grumble, that idea does sound intrusive, but perhaps it's
workable.  What about bitmap indexscans?  AFAICS we already have a
recheck there natively, so we only need to mark the page as lossy, which
we're already doing anyway.

-- 
Álvaro Herrera                https://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers