On Tue, Mar 24, 2020 at 1:24 AM Amit Kapila <amit.kapil...@gmail.com> wrote:
>
> On Fri, Mar 20, 2020 at 7:09 AM James Coleman <jtc...@gmail.com> wrote:
> >
> > Awesome, thanks for confirming with an actual plan.
> >
> > > I don't think it matters in nontext mode, but at least in text mode, I 
> > > think
> > > maybe the Unfetched blocks should be output after the exact and lossy 
> > > blocks,
> > > in case someone is parsing it, and because bitmap-only is a relatively new
> > > feature.  Its output is probably less common than exact/lossy.
> >
> > I tweaked that (and a comment that didn't reference the change); see 
> > attached.
> >
>
> Few comments:
> 1.
> -
> - if (tbmres->ntuples >= 0)
> + else if (tbmres->ntuples >= 0)
>   node->exact_pages++;
>
> How is this change related to this patch?

<already answered by Justin>

> 2.
> + * unfetched_pages    total number of pages not retrieved due to vm
>   * prefetch_iterator  iterator for prefetching ahead of current page
>   * prefetch_pages    # pages prefetch iterator is ahead of current
>   * prefetch_target    current target prefetch distance
> @@ -1591,6 +1592,7 @@ typedef struct BitmapHeapScanState
>   Buffer pvmbuffer;
>   long exact_pages;
>   long lossy_pages;
> + long unfetched_pages;
>
> Can we name it as skipped_pages?

That seems easy enough to do.

> 3. Can we add a test or two for this functionality?

>From what I can tell the current lossy page count isn't tested either;
would we expect the explain output from such a test to be stable
across different architectures etc.?

James


Reply via email to