Re: Amcheck verification of GiST and GIN

On Mon, Jun 9, 2025 at 6:34 PM Tomas Vondra <tomas(at)vondra(dot)me> wrote:
>
> On 6/9/25 00:14, Tomas Vondra wrote:
> > ...
> >
> > I propose to split it like this, into three parts, each addressing a
> > particular type of mistake:
> >
> > 1) gin_check_posting_tree_parent_keys_consistency
> >
> > 2) gin_check_parent_keys_consistency / att comparisons
> >
> > 3) gin_check_parent_keys_consistency / setting ptr->parenttup (at the end)
> >
> > Does this make sense to you? If yes, can you split the patch series like
> > this, including a commit message for each part, explaining the fix? We'd
> > need the commit message even with a single patch, ofc.
> >
> The attached v5 patch splits it along these lines, except that the extra
> 0001 part merely adds a multicolumn index into the regression test. The
> 0002-0004 parts are ordered to match the TAP test, i.e. it adds tests.

Great, thank you.

> I've copied the points from the report to the commit messages, but this
> needs cleanup/rephrasing, to make it readable. Could you look into
> that?Of course, if you think the patches should be split differently,
> feel free to move stuff.

Yes, sure, I will do it ASAP.

> And as I said before - if you feel the issues are too intertwined and
> can't be split like this (or it just doesn't make sense), please speak
> up. We can commit that as a single patch. It still needs the commit
> message, though.

The way it splitted seems reasonable to me. Intertwined issues are
grouped together, and patches are more or less independent.

Also the test for 'posting tree parent_key check' that was added last
started failing locally. Don't know what changed, but I rewrote it
so now it relies on child blkno, which is stable (I hope), instead of
concrete TID. Will include it in the new patchset.

Best regards,
Arseniy Mukhin