On Thu, Mar 2, 2023 at 6:50 PM Önder Kalacı <onderkal...@gmail.com> wrote: > >> >> >> In the above profile number of calls to index_fetch_heap(), >> heapam_index_fetch_tuple() explains the reason for the regression you >> are seeing with the index scan. Because the update will generate dead >> tuples in the same transaction and those dead tuples won't be removed, >> we get those from the index and then need to perform >> index_fetch_heap() to find out whether the tuple is dead or not. Now, >> for sequence scan also we need to scan those dead tuples but there we >> don't need to do back-and-forth between index and heap. > > > Thanks for the insights, I think what you describe makes a lot of sense. > ... ... > > I think we figured out the cause of the performance regression. I think it is > not small > enough for some scenarios like the above. But those scenarios seem like > synthetic > test cases, with not much user impacting implications. Still, I think you are > better suited > to comment on this. > > If you consider that this is a significant issue, we could consider the > second patch as well > such that for this unlikely scenario users could disable index scans. >
I think we can't completely ignore this regression because the key point of this patch is to pick one of the non-unique indexes to perform scan and now it will be difficult to predict how many duplicates (and or dead rows) some index has without more planner support. Personally, I feel it is better to have a table-level option for this so that users have some knob to avoid regressions in particular cases. In general, I agree that it will be a win in more number of cases than it can regress. -- With Regards, Amit Kapila.