Subject: Re: logical decoding bug: segfault in ReorderBufferToastReplace()
From: Jeremy Schneider <schnjere@amazon.com>
To: Andres Freund <andres@anarazel.de>, "Drouvot, Bertrand"
	<bdrouvot@amazon.com>
CC: Tomas Vondra <tomas.vondra@2ndquadrant.com>, PostgreSQL-development
	<pgsql-hackers@postgresql.org>
References: <E1i74JE-00024V-9J@gemulon.postgresql.org>
 <b1da9abc-bad7-efd6-2f1c-503d508c3877@amazon.com>
 <20191205133836.lulisfo2fphdrjp3@development>
 <EEB686D3-F8A7-4371-9A96-5DF3B72A7734@amazon.com>
 <20191211163522.t2ljtydarlmusoix@alap3.anarazel.de>
 <81182626-e836-061f-8f19-204edac18922@amazon.com>
Message-ID: <c41e5883-aedf-7944-010e-0b48b6c677ba@amazon.com>
Date: Fri, 4 Jun 2021 16:07:02 -0700
User-Agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10.14; rv:78.0)
 Gecko/20100101 Thunderbird/78.10.2
MIME-Version: 1.0
In-Reply-To: <81182626-e836-061f-8f19-204edac18922@amazon.com>
Content-Type: multipart/alternative;
	boundary="------------D44CE0FB5DCABF2F4EA8BB57"
Content-Language: en-US
Archived-At: 
 <https://www.postgresql.org/message-id/c41e5883-aedf-7944-010e-0b48b6c677ba%40amazon.com>
Precedence: bulk

--------------D44CE0FB5DCABF2F4EA8BB57
Content-Type: text/plain; charset="utf-8"
Content-Transfer-Encoding: 8bit

On 12/13/19 16:13, Jeremy Schneider wrote:
> On 12/11/19 08:35, Andres Freund wrote:
>> Seems like we clearly should add an elog(ERROR) here, so we error out,
>> rather than crash.
> done - in the commit that I replied to when I started this thread :)
>
> https://git.postgresql.org/gitweb/?p=postgresql.git;a=commitdiff;h=69f883fef14a3fc5849126799278abcc43f40f56


Another PostgreSQL user ran into this issue. This time on version 12.5 -
so instead of a crash they got the error message from the commit.

ERROR: XX000: could not open relation with OID 0
LOCATION: ReorderBufferToastReplace, reorderbuffer.c:305

Upon seeing this error message, I realized that the base relation OID
would be very useful when the toast relation OID is "0".

Would this patch work to show that?

diff --git a/src/backend/replication/logical/reorderbuffer.c
b/src/backend/replication/logical/reorderbuffer.c
index 2d9e1279bb..b90603b051 100644
--- a/src/backend/replication/logical/reorderbuffer.c
+++ b/src/backend/replication/logical/reorderbuffer.c
@@ -4598,8 +4598,8 @@ ReorderBufferToastReplace(ReorderBuffer *rb,
ReorderBufferTXN *txn,

        toast_rel = RelationIdGetRelation(relation->rd_rel->reltoastrelid);
        if (!RelationIsValid(toast_rel))
-               elog(ERROR, "could not open relation with OID %u",
-                        relation->rd_rel->reltoastrelid);
+               elog(ERROR, "could not open toast relation with OID %u
(base relation with OID %u)",
+                        relation->rd_rel->reltoastrelid,
relation->rd_rel->oid);

        toast_desc = RelationGetDescr(toast_rel);

Thoughts?

-Jeremy

-- 
Jeremy Schneider
Database Engineer
Amazon Web Services


--------------D44CE0FB5DCABF2F4EA8BB57
Content-Type: text/html; charset="utf-8"
Content-Transfer-Encoding: 8bit

<html>
  <head>
    <meta http-equiv="Content-Type" content="text/html; charset=UTF-8">
  </head>
  <body>
    <div class="moz-cite-prefix">On 12/13/19 16:13, Jeremy Schneider
      wrote:<br>
    </div>
    <blockquote type="cite"
      cite="mid:81182626-e836-061f-8f19-204edac18922@amazon.com">
      <meta http-equiv="Content-Type" content="text/html; charset=UTF-8">
      <div class="moz-cite-prefix">On 12/11/19 08:35, Andres Freund
        wrote:<br>
      </div>
      <blockquote type="cite"
        cite="mid:20191211163522.t2ljtydarlmusoix@alap3.anarazel.de">
        <pre class="moz-quote-pre" wrap="">Seems like we clearly should add an elog(ERROR) here, so we error out,
rather than crash.</pre>
      </blockquote>
      done - in the commit that I replied to when I started this thread
      :)<br>
      <br>
      <a class="moz-txt-link-freetext"
href="https://git.postgresql.org/gitweb/?p=postgresql.git;a=commitdiff;h=69f883fef14a3fc5849126799278abcc43f40f56"
        moz-do-not-send="true">https://git.postgresql.org/gitweb/?p=postgresql.git;a=commitdiff;h=69f883fef14a3fc5849126799278abcc43f40f56</a><br>
    </blockquote>
    <br>
    <br>
    Another PostgreSQL user ran into this issue. This time on version
    12.5 - so instead of a crash they got the error message from the
    commit.<br>
    <br>
    <font face="monospace">ERROR: XX000: could not open relation with
      OID 0<br>
      LOCATION: ReorderBufferToastReplace, reorderbuffer.c:305<br>
    </font><br>
    Upon seeing this error message, I realized that the base relation
    OID would be very useful when the toast relation OID is "0".<br>
    <br>
    Would this patch work to show that?<span><span><span class="Linkify"></span></span></span><br>
    <br>
    <font face="monospace">diff --git
      a/src/backend/replication/logical/reorderbuffer.c
      b/src/backend/replication/logical/reorderbuffer.c<br>
      index 2d9e1279bb..b90603b051 100644<br>
      --- a/src/backend/replication/logical/reorderbuffer.c<br>
      +++ b/src/backend/replication/logical/reorderbuffer.c<br>
      @@ -4598,8 +4598,8 @@ ReorderBufferToastReplace(ReorderBuffer *rb,
      ReorderBufferTXN *txn,<br>
      <br>
              toast_rel =
      RelationIdGetRelation(relation-&gt;rd_rel-&gt;reltoastrelid);<br>
              if (!RelationIsValid(toast_rel))<br>
      -               elog(ERROR, "could not open relation with OID %u",<br>
      -                        relation-&gt;rd_rel-&gt;reltoastrelid);<br>
      +               elog(ERROR, "could not open toast relation with
      OID %u (base relation with OID %u)",<br>
      +                        relation-&gt;rd_rel-&gt;reltoastrelid,
      relation-&gt;rd_rel-&gt;oid);<br>
      <br>
              toast_desc = RelationGetDescr(toast_rel);<br>
    </font><br>
    Thoughts?<br>
    <br>
    -Jeremy<br>
    <br>
    <pre class="moz-signature" cols="72">-- 
Jeremy Schneider
Database Engineer
Amazon Web Services</pre>
  </body>
</html>

--------------D44CE0FB5DCABF2F4EA8BB57--