Re: brininsert optimization opportunity - Mailing list pgsql-hackers

From Richard Guo
Subject Re: brininsert optimization opportunity
Date
Msg-id CAMbWs4-w9qC-o9hQox9UHvdVZAYTp8OrPQOKtwbvzWaRejTT=Q@mail.gmail.com
Whole thread Raw
In response to Re: brininsert optimization opportunity  (Tomas Vondra <tomas.vondra@enterprisedb.com>)
Responses Re: brininsert optimization opportunity
List pgsql-hackers

On Sun, Nov 26, 2023 at 4:06 AM Tomas Vondra <tomas.vondra@enterprisedb.com> wrote:
I've done a bit more cleanup on the last version of the patch (renamed
the fields to start with bis_ as agreed, rephrased the comments / docs /
commit message a bit) and pushed.

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

So I wonder if we should check 'bistate' and do the clean up only if
there is an available one, something like below.

--- a/src/backend/access/brin/brin.c
+++ b/src/backend/access/brin/brin.c
@@ -359,7 +359,9 @@ brininsertcleanup(IndexInfo *indexInfo)
 {
    BrinInsertState *bistate = (BrinInsertState *) indexInfo->ii_AmCache;

-   Assert(bistate);
+   /* We don't have an available insert state, nothing to do */
+   if (!bistate)
+       return;

Thanks
Richard

pgsql-hackers by date:

Previous
From: Andrei Lepikhov
Date:
Subject: Re: Postgres picks suboptimal index after building of an extended statistics
Next
From: Michael Paquier
Date:
Subject: Incorrect comment in tableam.h regarding GetHeapamTableAmRoutine()