Re: BUG #15378: SP-GIST memory context screwup?

[CCing Teodor as committer of ccd6eb49a]

>>>>> "PG" == PG Bug reporting form <noreply(at)postgresql(dot)org> writes:

PG> I found this while analyzing a report from IRC that initially
PG> looked like a PostGIS bug, but which I now think is breakage in
PG> spgist:

PG> spgrescan starts out by doing
PG> MemoryContextReset(so->traversalCxt);

PG> then later it calls resetSpGistScanOpaque(so);
PG> which calls freeScanStack(so)
PG> which calls freeScanStackEntry(so)
PG> which does:

PG> if (stackEntry->traversalValue)
PG> pfree(stackEntry->traversalValue);

PG> But stackEntry->traversalValue, if not NULL, is supposed to have
PG> been allocated in so->traversalCxt, and so it's already gone.
[...]
PG> Unfortunately I don't think this can be demonstrated with the
PG> built-in spgist opclasses, which don't allocate traversalValues.

Turns out I was looking in the wrong place, and in fact we can reproduce
this easily (at least on an assert build):

create table boxes (b box);
insert into boxes
select box(point(i,j),point(i+s,j+s))
from generate_series(1,100,5) i,
generate_series(1,100,5) j,
generate_series(1,10) s;
create index on boxes using spgist (b);
select *
from (values (box(point(5,5),point(8,8))),(box(point(2,2),point(12,12)))) v(b)
cross join lateral (select * from boxes where boxes.b && v.b limit 1) pt;

So this logic was added in ccd6eb49a and was wrong from the start.
Testing suggests that removing the offending pfree does indeed fix the
issue; any objections?

--
Andrew (irc:RhodiumToad)