Re: improve transparency of bitmap-only heap scans

On Fri, Mar 27, 2020 at 9:24 PM Amit Kapila <amit(dot)kapila16(at)gmail(dot)com> wrote:
>
> On Wed, Mar 25, 2020 at 5:44 PM James Coleman <jtc331(at)gmail(dot)com> wrote:
> >
> > On Tue, Mar 24, 2020 at 11:02 PM Amit Kapila <amit(dot)kapila16(at)gmail(dot)com> wrote:
> > >
> > > On Wed, Mar 25, 2020 at 12:44 AM Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> wrote:
> > > >
> > > > I took a quick look through this patch. While I see nothing to complain
> > > > about implementation-wise, I'm a bit befuddled as to why we need this
> > > > reporting when there is no comparable data provided for regular index-only
> > > > scans. Over there, you just get "Heap Fetches: n", and the existing
> > > > counts for bitmap scans seem to cover the same territory.
> > > >
> > >
> > > Isn't deducing similar information ("Skipped Heap Fetches: n") there
> > > is a bit easier than it is here?
> >
> > While I'm not the patch author so can't speak to the original thought
> > process, I do think it makes sense to show it.
> >
>
> Yeah, I also see this information could be useful. It seems Tom Lane
> is not entirely convinced of this. I am not sure if this is the right
> time to seek more opinions as we are already near the end of CF. So,
> we should either decide to move this to the next CF if we think of
> getting the opinion of others or simply reject it and see a better way
> for EXPLAIN to identify an index-only BMS.

I'm curious if Tom's objection is mostly on the grounds that we should
be consistent in what's displayed, or that he thinks the information
is likely to be useless.

If consistency is the goal you might e.g., do something that just
changes the node type output, but in favor of changing that, it seems
to me that showing "how well did the optimization" is actually more
valuable than "did we do the optimization at all". Additionally I
think showing it as an optimization of an existing node is actually
likely less confusing anyway.

One other thing: my understanding is that this actually matches the
underlying code split too. For the index only scan case, we actually
have a separate node (it's not just an optimization of the standard
index scan). There are discussions about whether that's a good thing,
but it's what we have. In contrast, the bitmap scan actually has it as
a pure optimization of the existing bitmap heap scan node.

James