> On Mar 7, 2023, at 10:16 AM, Robert Haas <robertmhaas@gmail.com> wrote:
>
> On Mon, Mar 6, 2023 at 12:36 PM Robert Haas <robertmhaas@gmail.com> wrote:
>> So it seems that we still don't have a patch where the
>> value of a variable called lp_valid corresponds to whether or not the
>> L.P. is valid.
>
> Here's a worked-over version of this patch. Changes:
>
> - I got rid of the code that sets lp_valid in funny places and instead
> arranged to have check_tuple_visibility() pass up the information on
> the XID status. That's important, because we can't casually apply
> operations like TransactionIdIsCommitted() to XIDs that, for all we
> know, might not even be in the range covered by CLOG. In such cases,
> we should not perform any HOT chain validation because we can't do it
> sensibly; the new code accomplishes this, and also reduces the number
> of CLOG lookups as compared with your version.
>
> - I moved most of the HOT chain checks from the loop over the
> predecessor[] array to the loop over the successor[] array. It didn't
> seem to have any value to put them in the third loop; it forces us to
> expend extra code to distinguish between redirects and tuples,
> information that we already had in the second loop. The only check
> that seems to make sense to do in that last loop is the one for a HOT
> chain that starts with a HOT tuple, which can't be done any earlier.
>
> - I realized that your patch had a guard against setting the
> predecessor[] when it was set already only for tuples, not for
> redirects. That means if a redirect pointed into the middle of a HOT
> chain we might not report corruption appropriately. I fixed this and
> reworded the associated messages a bit.
>
> - Assorted cosmetic and comment changes.
>
> I think this is easier to follow and more nearly correct, but what do
> you (and others) think?
Thanks, Robert. Quickly skimming over this patch, it looks like something reviewable. Your changes to
t/004_verify_heapam.plappear to be consistent with how that test was intended to function.
Note that I have not tried any of this yet.
—
Mark Dilger
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company
