On Mon, Jun 9, 2025 at 6:34 PM Tomas Vondra <to...@vondra.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


Reply via email to