Hi,
On 2014-09-24 14:26:37 +0530, Abhijit Menon-Sen wrote:
> Thanks for your comments, and I'm sorry it's taken me so long to
> respond.
>
> At 2014-08-03 11:18:57 +0530, amit.kapila16@gmail.com wrote:
> >
> > After looking at code, I also felt that it is better to add this as a
> > version of pg_stattuple.
>
> I started off trying to do that, but now I'm afraid I strongly disagree.
> First, pg_stattuple works on relations and indexes, whereas fastbloat
> only works on relations (because of the VM/FSM use). Second, the code
> began to look hideous when mushed together, with too many ifs to avoid
> locking I didn't need and so on. The logic is just too different.
Why not add it to the stattuple extension, but as an independent
function (and file)? I don't really see a need for a completely new
extension, but a separate extension seems wasteful.
> > 7.
> > HeapTupleSatisfiesVacuumNoHint()
> > a. Why can't we use HeapTupleSatisfiesVisibility() with dirty snapshot
> > as we use for pgstattuple?
I think it's completely unacceptable to copy a visibility routine. And I
don't believe for a second that avoiding hint bit setting makes it legal
to not acquire a content lock for this. What's your reasoning for that
being safe? If you argue that all possible corruption has limited impact
you need to do that *much* more explicitly and verbosely.
Greetings,
Andres Freund