Content-Type: multipart/alternative;
 boundary="------------1gcARj8k7L2B0xoutDzV8Ftr"
Message-ID: <a121b735-70f5-49d9-8774-22a465008545@gmail.com>
Date: Mon, 9 Feb 2026 14:15:34 +0200
MIME-Version: 1.0
User-Agent: Mozilla Thunderbird
Subject: Re: Propagate XLogFindNextRecord error to callers
To: Anthonin Bonnefoy <anthonin.bonnefoy@datadoghq.com>
Cc: PostgreSQL Hackers <pgsql-hackers@postgresql.org>,
 Japin Li <japinli@hotmail.com>
References: <CAO6_XqoxJXddcT4wkd9Xd+cD6Sz-fyspRGuV4Bq-wbXG4pVNzA@mail.gmail.com>
 <0cbf5d34-f117-456f-bcc0-50fa9a8eafba@gmail.com>
 <CAO6_Xqqd316HhigkBsE-rC8oVsAprc_mVh5Z312D+aUOzW-5Jg@mail.gmail.com>
 <MEAPR01MB3031690E672656BB573711B3B665A@MEAPR01MB3031.ausprd01.prod.outlook.com>
 <CAO6_Xqqv5F2cdgs5zCVzLJMOuO6zStsC97gcm41dFNSWOh6B8A@mail.gmail.com>
Content-Language: en-GB
From: Mircea Cadariu <cadariu.mircea@gmail.com>
In-Reply-To: <CAO6_Xqqv5F2cdgs5zCVzLJMOuO6zStsC97gcm41dFNSWOh6B8A@mail.gmail.com>
Archived-At: <https://www.postgresql.org/message-id/a121b735-70f5-49d9-8774-22a465008545%40gmail.com>
Precedence: bulk

This is a multi-part message in MIME format.
--------------1gcARj8k7L2B0xoutDzV8Ftr
Content-Type: text/plain; charset=UTF-8; format=flowed
Content-Transfer-Encoding: 8bit

Hi Anthonin,

Thanks for the updated patch.

I have noticed this code added in XLogFindNextRecord in the patch, 
appears also in XLogNextRecord (line 334).

> +	if (state->errormsg_deferred)
> +	{
> +		if (state->errormsg_buf[0] != '\0')
> +			*errormsg = state->errormsg_buf;
> +		state->errormsg_deferred = false;
> +	}
> +

In XLogNextRecord, right before the above code, we do *errormsg = NULL. 
Should this be done also in XLogFindNextRecord in the patch?

If so, what about even extracting a helper method which will be called 
from both places?

A nit for the commit message: Propage -> Propagate

-- 
Thanks,
Mircea Cadariu

--------------1gcARj8k7L2B0xoutDzV8Ftr
Content-Type: text/html; charset=UTF-8
Content-Transfer-Encoding: 8bit

<!DOCTYPE html>
<html>
  <head>
    <meta http-equiv="Content-Type" content="text/html; charset=UTF-8">
  </head>
  <body>
    <p>Hi Anthonin, </p>
    <p>Thanks for the updated patch. </p>
    <p>I have noticed this code added in XLogFindNextRecord in the
      patch, appears also in XLogNextRecord (line 334).</p>
    <blockquote type="cite"
cite="mid:CAO6_Xqqv5F2cdgs5zCVzLJMOuO6zStsC97gcm41dFNSWOh6B8A@mail.gmail.com">
      <pre wrap="" class="moz-quote-pre">+	if (state-&gt;errormsg_deferred)
+	{
+		if (state-&gt;errormsg_buf[0] != '\0')
+			*errormsg = state-&gt;errormsg_buf;
+		state-&gt;errormsg_deferred = false;
+	}
+</pre>
    </blockquote>
    <p>In XLogNextRecord, right before the above code, we do *errormsg =
      NULL. Should this be done also in XLogFindNextRecord in the
      patch? </p>
    <p>If so, what about even extracting a helper method which will be
      called from both places? </p>
    <p>A nit for the commit message: Propage -&gt; Propagate</p>
    <p><span style="white-space: pre-wrap">
</span></p>
    <pre class="moz-signature" cols="72">-- 
Thanks,
Mircea Cadariu</pre>
  </body>
</html>

--------------1gcARj8k7L2B0xoutDzV8Ftr--