Re: Tid scan improvements

On Fri, 21 Dec 2018 at 13:25, David Rowley <david(dot)rowley(at)2ndquadrant(dot)com> wrote:
> On Fri, 21 Dec 2018 at 13:09, Edmund Horner <ejrh00(at)gmail(dot)com> wrote:
> > On Fri, 21 Dec 2018 at 11:21, Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> wrote:
> > > I'm having a hard time wrapping my mind around why you'd bother with
> > > backwards TID scans. The amount of code needed versus the amount of
> > > usefulness seems like a pretty bad cost/benefit ratio, IMO. I can
> > > see that there might be value in knowing that a regular scan has
> > > "ORDER BY ctid ASC" pathkeys (mainly, that it might let us mergejoin
> > > on TID without an explicit sort). It does not, however, follow that
> > > there's any additional value in supporting the DESC case.
> >
> > I have occasionally found myself running "SELECT MAX(ctid) FROM t"
> > when I was curious about why a table is so big after vacuuming.
> >
> > Perhaps that's not a common enough use case to justify the amount of
> > code, especially the changes to heapam.c and explain.c.
> >
> > We'd still need the pathkeys to make good use of forward scans. (And
> > I think the executor still needs to support seeking backward for
> > cursors.)
>
> I think the best thing to do here is separate out all the additional
> backwards scan code into a separate patch to allow it to be easier
> considered and approved, or rejected. I think if there's any hint of
> this blocking the main patch then it should be a separate patch to
> allow it's worth to be considered independently.

Yeah I think you're right. I'll separate those parts into the basic
forward scan, and then the optional backward scan support. I think
we'll still only generate a backward scan if the query_pathkeys makes
use of it.

For the forward scan, I seem to recall, from your merge join example,
that it's useful to set the pathkeys even when there are no
query_pathkeys. We just have to unconditionally set them so that the
larger plan can make use of them.

> Also, my primary interest in this patch is to find tuples that are
> stopping the heap being truncated during a vacuum. Generally, when I'm
> looking for that I have a good idea of what size I expect the relation
> should be, (otherwise I'd not think it was bloated), in which case I'd
> be doing WHERE ctid >= '(N,1)'. However, it might be easier to write
> some auto-bloat-removal script if we could have an ORDER BY ctid DESC
> LIMIT n.