On Sun, Feb 5, 2023 at 3:57 AM Himanshu Upadhyaya
<upadhyaya.himanshu@gmail.com> wrote:
> Thanks, yes it's working fine with Prepared Transaction.
> Please find attached the v9 patch incorporating all the review comments.
I don't know quite how we're still going around in circles about this,
but this code makes no sense to me at all:
/*
* Add data to the predecessor array even if the current or
* successor's LP is not valid. We will not process/validate these
* offset entries while looping over the predecessor array but
* having all entries in the predecessor array will help in
* identifying(and validating) the Root of a chain.
*/
if (!lp_valid[ctx.offnum] || !lp_valid[nextoffnum])
{
predecessor[nextoffnum] = ctx.offnum;
continue;
}
If the current offset number is not for a valid line pointer, then it
makes no sense to talk about the successor. An invalid redirected line
pointer is one that points off the end of the line pointer array, or
to before the beginning of the line pointer array, or to a line
pointer that is unused. An invalid line pointer that is LP_USED is one
which points to a location outside the page, or to a location inside
the page. In none of these cases does it make any sense to talk about
the next tuple. If the line pointer isn't valid, it's pointing to some
invalid location where there cannot possibly be a tuple. In other
words, if lp_valid[ctx.offnum] is false, then nextoffnum is a garbage
value, and therefore referencing predecessor[nextoffnum] is useless
and dangerous.
If the next offset number is not for a valid line pointer, we could in
theory still assign to the predecessor array, as you propose here. In
that case, the tuple or line pointer at ctx.offnum is pointing to the
line pointer at nextoffnum and that is all fine. But what is the
point? The comment claims that the point is that it will help us
identify and validate the root of the hot chain. But if the line
pointer at nextoffnum is not valid, it can't be the root of a hot
chain. When we're talking about the root of a HOT chain, we're
speaking about a tuple. If lp_valid[nextoffnum] is false, there is no
tuple. Instead of pointing to a tuple, that line pointer is pointing
to garbage.
--
Robert Haas
EDB: http://www.enterprisedb.com
