On Sat, 1 Apr 2023 at 13:24, Melanie Plageman <melanieplage...@gmail.com> wrote: > Your diff LGTM. > > Earlier upthread in [1], Bharath had mentioned in a review comment about > removing the global variables that he would have expected the analogous > global in analyze.c to also be removed (vac_strategy [and analyze.c also > has anl_context]). > > I looked into doing this, and this is what I found out (see full > rationale in [2]): > > > it is a bit harder to remove it from analyze because acquire_func > > doesn't take the buffer access strategy as a parameter and > > acquire_sample_rows uses the vac_context global variable to pass to > > table_scan_analyze_next_block(). > > I don't know if this is worth mentioning in the commit removing the > other globals? Maybe it will just make it more confusing...
I did look at that, but it seems a little tricky to make work unless the AcquireSampleRowsFunc signature was changed. To me, it just does not seem worth doing that to get rid of the two globals in analyze.c. I pushed the patch with just the vacuum.c changes. David