On Mon, Jun 9, 2025 at 7:37 PM Arseniy Mukhin
<arseniy.mukhin.dev@gmail.com> wrote:
>
> On Mon, Jun 9, 2025 at 6:34 PM Tomas Vondra <tomas@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.
>
Please find a new version in attachments. There are formatted commit
messages and some cosmetic changes in the tests. Please let me know if
anything needs to be changed. Also FWIW points 9th, 10th and 11th from
the report [1] were not addressed in the fixes. I'm not sure about
10th and 11th, but 9th seems like a no-brainer, so I added a patch
deleting an unused field 'parentlsn'. I tried git-am with patches and
it's ok with it. Thank you for the advice, added git-am step in my
patch preparation routine.
> ...
> 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.
>
Also changed the regex pattern for this failing test, hope it is more
robust now.
[1] https://postgr.es/m/CAE7r3MJ611B9TE=YqBBncewp7-k64VWs+sjk7XF6fJUX77uFBA@mail.gmail.com
Best regards,
Arseniy Mukhin
