Re: PoC Refactor AM analyse API - Mailing list pgsql-hackers

Andrey, thanks for your feedback!

I agree that AMs with fix sized blocks can have much alike code in acquire_sample_rows() (though it is not a rule). But
thereare several points about current master sampling. 

* It is not perfect - AM developers may want to improve it with other sampling algorithms.
* It is designed with a big influence of heap AM - for example, RelationGetNumberOfBlocks() returns uint32 while other
AMscan have a bigger amount of blocks. 
* heapam_acquire_sample_rows() is a small function - I don't think it is not a big trouble to write something alike for
anyAM developer. 
* Some AMs may have a single level sampling (only algorithm Z from Vitter for example) - why not?

As a result we get a single and clear method to acquire rows for statistics. If we don’t modify but rather extend
currentAPI ( for example in a manner it is done for FDW) the code becomes more complicated and difficult to understand. 

> 8 дек. 2020 г., в 18:42, Andrey Borodin <x4mmm@yandex-team.ru> написал(а):
>
> Hi Denis!
>
>> 7 дек. 2020 г., в 18:23, Смирнов Денис <sd@arenadata.io> написал(а):
>>
>> I suggest a refactoring of analyze AM API as it is too much heap specific at the moment. The problem was inspired by
Greenplum’sanalyze improvement for append-optimized row and column AM with variable size compressed blocks. 
>> Currently we do analyze in two steps.
>>
>> 1. Sample fix size blocks with algorithm S from Knuth (BlockSampler function)
>> 2. Collect tuples into reservoir with algorithm Z from Vitter.
>>
>> So this doesn’t work for AMs using variable sized physical blocks for example. They need weight random sampling
(WRS)algorithms like A-Chao or logical blocks to follow S-Knuth (and have a problem with RelationGetNumberOfBlocks()
estimatinga physical number of blocks). Another problem with columns - they are not passed to analyze begin scan and
can’tbenefit from column storage at ANALYZE TABLE (COL). 
>>
>> The suggestion is to replace table_scan_analyze_next_block() and table_scan_analyze_next_tuple() with a single
function:table_acquire_sample_rows(). The AM implementation of table_acquire_sample_rows() can use the BlockSampler
functionsif it wants to, but if the AM is not block-oriented, it could do something else. This suggestion also passes
VacAttrStatsto table_acquire_sample_rows() for column-oriented AMs and removes PROGRESS_ANALYZE_BLOCKS_TOTAL and
PROGRESS_ANALYZE_BLOCKS_DONEdefinitions as not all AMs can be block-oriented. 
>
> Just few random notes about the idea.
> Heap pages are not of a fixed size, when measured in tuple count. And comment in the codes describes it.
> * Although every row has an equal chance of ending up in the final
> * sample, this sampling method is not perfect: not every possible
> * sample has an equal chance of being selected.  For large relations
> * the number of different blocks represented by the sample tends to be
> * too small.  We can live with that for now.  Improvements are welcome.
>
> Current implementation provide framework with shared code. Though this framework is only suitable for block-of-tuples
AMs.And have statistical downsides when count of tuples varies too much. 
> Maybe can we just provide a richer API? To have both: avoid copying code and provide flexibility.
>
> Best regards, Andrey Borodin.
>