Hi Sami,
Your patch should correct the problem. However, given that this function is part of the tableam API, I am wondering if the fix shouldn't be outside heap's copy_for_cluster implementation? I guess it depends on the semantics of num_tuples, but the cluster code seems to allude to interpreting num_tuples as the number of non-removable tuples. If you subtract recently dead from that number within the heap implementation, then it will no longer reflect non-removable tuples and the log message in the cluster function "found %.0f removable, %.0f nonremovable row versions" will no longer be correct.
Surprisingly, tableam.h does not document the num_tuples parameter in the table_relation_copy_for_cluster() function although all other output parameters are documented. So, it is not clear what the intended semantics are. Maybe other hackers on the mailing list have opinions on how to interpret num_tuples?
In any case, assuming num_tuples is supposed to return non-removable tuples, then the fix should be to subtract recently dead tuples when updating pg_class.reltuples. Other TAM's need to treat num_tuples as non-removable tuples as well, and update recently dead if applicable.
I am attaching a patch with these changes, while also including the isolation test in that patch.
Regards,
Erik
> In both cases, he recently dead tuples must be copied to the table or index, but
> they should not be counted towards reltuples. So, I think we need to fix this in
> heapam_relation_copy_for_cluster by probably subtracting
> tups_recently_dead from num_tuples ( which is the value set in
> pg_class.reltuples )
> after we process all the tuples, which looks like the best fix to me.
something like the attached.
--
Sami Imseih
Amazon Web Services (AWS)
