Re: New vacuum option to do only freezing

Robert Haas <robertmhaas(at)gmail(dot)com> writes:
> On Mon, Apr 15, 2019 at 1:13 PM Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> wrote:
>> I have a very strong feeling that this patch was not fully baked.

> I think you're right, but I don't understand the comment in the
> preceding paragraph. How does this problem prevent tupgone from
> getting set?

My point is that I suspect that tupgone *shouldn't* get set.
It's not (going to be) gone.

> It looks to me like nleft_dead_tuples should be ripped out.

That was pretty much what I was thinking too. It makes more sense
just to treat this case identically to dead-but-not-yet-removable.
I have substantial doubts about nleft_dead_itemids being worth
anything, as well.

> We should do something like:
> if (params->index_cleanup == VACOPT_TERNARY_DISABLED)
> {
> nkeep += tups_vacuumed;
> tups_vacuumed = 0;
> }

No. I'm thinking there should be exactly one test of index_cleanup
in this logic, and what it would be is along the lines of

if (HeapTupleIsHotUpdated(&tuple) ||
HeapTupleIsHeapOnly(&tuple) ||
+ params->index_cleanup == VACOPT_TERNARY_DISABLED)
nkeep += 1;
else

In general, this thing has a strong whiff of "large patch
with a small patch struggling to get out".

regards, tom lane