On Tue, Sep 29, 2020 at 2:57 PM Heikki Linnakangas <hlinnaka@iki.fi> wrote:
On 29/09/2020 11:49, Hamid Akhtar wrote: > So, not actually random replacement here, rather a change with > baserel->allvisfrac taken into consideration (as given below): > ---- > index_random_page_cost = Min(spc_seq_page_cost + spc_random_page_cost * > (1.0 - baserel->allvisfrac), spc_random_page_cost); > ---- > > Does this make sense?
No. genericcostestimate() is only concerned with accesses to the index, not the the heap accesses that are needed with Index Scans. 'allvisfrac' should not affect the number of *index* pages fetched in any way.
- Heikki
Currently, the costing for indexonlyscan only differs based on 'allvisfrac'. IIUC, the current implementation changes the number of pages being fetched based on 'allvisfrac'.
This patch actually makes indexonlyscan specific changes to genericcostestimate function. Currently, regardless of the value of 'allvisfrac', it is being assumed that the cost of fetching index pages is random page cost. That is not aligned with the current cost calculation for indexonlyscan. Therefore, I'm suggesting to reduce the random page in a similar fashion in case of indexonlyscan.
I'm adding this to the commitfest.
Retrospectively looking at the patch, I see your point. Your criticism is valid. I'll revalidate this issue and rework the patch if necessary.