On Wed, Mar 8, 2023 at 7:06 PM Robert Haas <robertmh...@gmail.com> wrote:
> > > + /* HOT chains should not intersect. */ > > + if (predecessor[nextoffnum] != InvalidOffsetNumber) > > + { > > + report_corruption(&ctx, > > + psprintf("redirect line pointer > > points to offset %u, but offset %u also points there", > > + (unsigned) nextoffnum, > > (unsigned) predecessor[nextoffnum])); > > + continue; > > + } > > ``` > > > > This type of corruption doesn't seem to be test-covered. > > Himanshu, would you be able to try to write a test case for this? I > think you need something like this: update a tuple with a lower TID to > produce a tuple with a higher TID, e.g. (0,10) is updated to produce > (0,11). But then have a redirect line pointer that also points to the > result of the update, in this case (0,11). > > Sure Robert, I will work on this. > > ``` > > + /* > > + * If the next line pointer is a redirect, or if it's a > tuple > > + * but the XMAX of this tuple doesn't match the XMIN of the > next > > + * tuple, then the two aren't part of the same update chain > and > > + * there is nothing more to do. > > + */ > > + if (ItemIdIsRedirected(next_lp)) > > + continue; > > ``` > > > > lcov shows that the `continue` path is never executed. This is > > probably not a big deal however. > > It might be good to have a negative test case for this, though. Let's > say we, e.g. update (0,1) to produce (0,2), but then abort. The page > is HOT-pruned. Then we add insert a new tuple at (0,2), HOT-update it > to produce (0,3), and commit. Then we HOT-prune again. Possibly we > could try to write a test case that verifies that this does NOT > produce any corruption indication. > > will work on this too. > > ``` > > +$node->append_conf('postgresql.conf','max_prepared_transactions=100'); > > ``` > > > > From what I can tell this line is not needed. > > That surprises me, because the new test cases involve preparing a > transaction, and by default max_prepared_transactions=0. So it seems > to me (without testing) that this ought to be required. Did you test > that it works without this setting? > > The value of 100 seems a bit excessive, though. Most TAP tests seem to use > 10. > > We need this for prepare transaction, will change it to 10. -- Regards, Himanshu Upadhyaya EnterpriseDB: http://www.enterprisedb.com