Hi,
On 2019-03-25 11:04:05 -0700, Andres Freund wrote:
> Hi,
>
> On 2019-03-24 23:54:53 +1300, David Rowley wrote:
> > This results in an Assert failure on master and an elog ERROR prior to
> > c2fe139c201:
> >
> > create role test_role with login;
> > create table ref(a int primary key);
> > grant references on ref to test_role;
> > set role test_role;
> > create table t1(a int, b int);
> > insert into t1 values(1,1);
> > alter table t1 add constraint t1_b_key foreign key (b) references ref(a);
> > server closed the connection unexpectedly
> > This probably means the server terminated abnormally
> > before or while processing the request.
> > The connection to the server was lost. Attempting reset: Failed.
> >
> > Fails in heapam_tuple_satisfies_snapshot() at
> > Assert(BufferIsValid(bslot->buffer));
> >
> > c2fe139c201~1:
> > ERROR: expected buffer tuple
> >
> > The test case is just a variation of the case in [1], but a different
> > bug, so reporting it on a different thread.
> >
> > I've not looked into the cause or when it started happening.
>
> I think the cause is stupidity of mine. In
> validateForeignKeyConstraint() I passed true to the materialize argument
> of ExecFetchSlotHeapTuple(). Which therefore is made independent of
> buffers. Which this assert then notices. Just changing that to false,
> which is correct, fixes the issue for me.
>
> I'm a bit confused as to how we have no tests for this code? Is it just
> that the left join codepath is "too good"?
>
> I've also noticed that we should free the tuple - that doesn't matter
> for heap, but it sure does for other callers. But uh, is it actually ok
> to validate an entire table's worth of foreign keys without a memory
> context reset? I.e. shouldn't we have a memory context that we reset
> after each iteration?
>
> Also, why's there no CHECK_FOR_INTERRUPTS()? heap has some internally on
> a page level, but that doesn't seem all that granular?
Tom pushed a part of this earlier in
commit 46e3442c9ec858071d60a1c0fae2e9868aeaa0c8
Author: Tom Lane <tgl@sss.pgh.pa.us>
Date: 2019-04-06 15:09:09 -0400
Fix failures in validateForeignKeyConstraint's slow path.
I've now added a fixed version of the memory context portion of this
patch.
Greetings,
Andres Freund