Re: BUG #16162: create index using gist_trgm_ops leads to panic

On 15/12/2019 19:57, Jeff Janes wrote:
> On Sat, Dec 14, 2019 at 2:00 PM Alexander Lakhin <exclusion(at)gmail(dot)com
> <mailto:exclusion(at)gmail(dot)com>> wrote:
>
> 14.12.2019 1:02, Heikki Linnakangas wrote:
> >
> > Committed a fix. Many thanks for the reproducer scripts, Andreas and
> > Alexander!
> The script that I presented in [0] still reproduces assertion failure
> for me on REL_12_STABLE (fd005e1a):
>
> I've verified that this was introduced by 9155580f and that Heikki's
> latest commit a7ee7c851 has not fixed it.  I've reproduced it without
> cassert.

So there was still one instance of basically the same bug. In the same
function, there are two calls to gistinserttuples():

> /*
> * insert downlinks for the siblings from right to left, until there are
> * only two siblings left.
> */
> for (int pos = list_length(splitinfo) - 1; pos > 1; pos--)
> {
> right = (GISTPageSplitInfo *) list_nth(splitinfo, pos);
> left = (GISTPageSplitInfo *) list_nth(splitinfo, pos - 1);
>
> if (gistinserttuples(state, stack->parent, giststate,
> &right->downlink, 1,
> InvalidOffsetNumber,
> left->buf, right->buf, false, false))
> {
> /*
> * If the parent page was split, need to relocate the original
> * parent pointer.
> */
> stack->downlinkoffnum = InvalidOffsetNumber;
> gistFindCorrectParent(state->r, stack);
> }
> /* gistinserttuples() released the lock on right->buf. */
> }
>
> right = (GISTPageSplitInfo *) lsecond(splitinfo);
> left = (GISTPageSplitInfo *) linitial(splitinfo);
>
> /*
> * Finally insert downlink for the remaining right page and update the
> * downlink for the original page to not contain the tuples that were
> * moved to the new pages.
> */
> tuples[0] = left->downlink;
> tuples[1] = right->downlink;
> gistinserttuples(state, stack->parent, giststate,
> tuples, 2,
> stack->downlinkoffnum,
> left->buf, right->buf,
> true, /* Unlock parent */
> unlockbuf /* Unlock stack->buffer if caller wants that */
> );
> Assert(left->buf == stack->buffer);

I added the "stack-downlinkoffnum = InvalidOffsetNumber" to the first
call on Friday. But the second call needs the same treatment, because
it's possible that we reuse the same stack on a later call to
gistfinishsplit(), if a grandchild page is split twice, for the same
leaf insert. I feel pretty dumb, because I looked at that second call on
Friday too, thinking if it also needs to reset 'downlinkoffnum', but
somehow I concluded it does not. Clearly it does, as this test case
demonstrates.

Fix committed. Thanks again for the report!

- Heikki