On Tue, Mar 24, 2020 at 1:24 AM Amit Kapila <amit.kapila16@gmail.com> wrote:
>
> On Fri, Mar 20, 2020 at 7:09 AM James Coleman <jtc331@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