On Mon, Nov 27, 2023 at 1:53 PM Soumyadeep Chakraborty < soumyadeep2...@gmail.com> wrote:
> On Sun, Nov 26, 2023 at 9:28 PM Richard Guo <guofengli...@gmail.com> > wrote: > > It seems that we have an oversight in this commit. If there is no tuple > > that has been inserted, we wouldn't have an available insert state in > > the clean up phase. So the Assert in brininsertcleanup() is not always > > right. For example: > > > > regression=# update brin_summarize set value = brin_summarize.value; > > server closed the connection unexpectedly > > I wasn't able to repro the issue on > 86b64bafc19c4c60136a4038d2a8d1e6eecc59f2. > with UPDATE/INSERT: > > This could be because since c5b7ba4e67aeb5d6f824b74f94114d99ed6e42b7, > we have moved ExecOpenIndices() > from ExecInitModifyTable() to ExecInsert(). Since we never open the > indices if nothing is > inserted, we would never attempt to close them with ExecCloseIndices() > while the ii_AmCache > is NULL (which is what causes this assertion failure). AFAICS we would also open the indices from ExecUpdate(). So if we update the table in a way that no new tuples are inserted, we will have this issue. As I showed previously, the query below crashes for me on latest master (dc9f8a7983). regression=# update brin_summarize set value = brin_summarize.value; server closed the connection unexpectedly There are other code paths that call ExecOpenIndices(), such as ExecMerge(). I believe it's not hard to create queries that trigger this Assert for those cases. Thanks Richard