Re: TABLESAMPLE patch

On 10/03/15 04:43, Amit Kapila wrote:
> On Mon, Mar 9, 2015 at 3:08 PM, Petr Jelinek <petr@2ndquadrant.com
> <mailto:petr@2ndquadrant.com>> wrote:
>  >
>  > On 09/03/15 04:51, Amit Kapila wrote:
>  >>
>  >> On Sat, Mar 7, 2015 at 10:37 PM, Petr Jelinek <petr@2ndquadrant.com
> <mailto:petr@2ndquadrant.com>
>  >>  > Double checking for tuple visibility is the only downside I can think
>  >> of.
>  >>
>  >> That will happen if we use heapgetpage and the way currently
>  >> code is written in patch, however we can easily avoid double
>  >> checking if we don't call heapgetpage and rather do the required
>  >> work at caller's place.
>  >>
>  >
>  > What's the point of pagemode then if the caller code does the
> visibility checks still one by one on each call. I thought one of the
> points of pagemode was to do this in one step (and one buffer lock).
>  >
>
> You only need one buffer lock for doing at caller's location
> as well something like we do in acquire_sample_rows().
>

Ok now I think I finally understand what you are suggesting - you are 
saying let's go over whole page while tsmnexttuple returns something, 
and do the visibility check and other stuff in that code block under the 
buffer lock and cache the resulting valid tuples in some array and then 
return those tuples one by one from that cache?

>  > And if the caller will try to do it in one step and cache the
> visibility info then we'll end up with pretty much same structure as
> rs_vistuples - there isn't saner way to cache this info other than
> ordered vector of tuple offsets, unless we assume that most pages have
> close to MaxOffsetNumber of tuples which they don't, so why not just use
> the heapgetpage directly and do the binary search over rs_vistuples.
>  >
>
> The downside of doing it via heapgetpage is that it will do
> visibility test for tuples which we might not even need (I think
> we should do visibility test for tuples retrurned by tsmnexttuple).
>

Well, heapgetpage can either read visibility data for whole page or not, 
depending on if we want pagemode reading or not. So we can use the 
pagemode for sampling methods where it's feasible (like system) and not 
use pagemode where it's not (like bernoulli) and then either use the 
rs_vistuples or call HeapTupleSatisfiesVisibility individually again 
depending if the method is using pagemode or not. This is how sequential 
scan does it afaik.

--  Petr Jelinek                  http://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Training &
Services