Re: Set notice receiver before libpq connection startup

> On May 22, 2026, at 21:42, Fujii Masao <masao(dot)fujii(at)gmail(dot)com> wrote:
>
> On Fri, May 22, 2026 at 8:33 PM Rafia Sabih <rafia(dot)pghackers(at)gmail(dot)com> wrote:
>> Here are my two cents,
>> we need not to check conn is null here
>> + conn = libpqsrv_connect_start(connstr);
>> + if (conn != NULL)
>> + PQsetNoticeReceiver(conn, libpqsrv_notice_receiver,
>> + "received message via remote connection");
>> because it is done so in PQsetNoticeReceiver anyway. Also, since there is no else here so it doesn't make sense more, because if it is null then also we will just continue with the next function call.
>
> Yes, but I'm fine with the current code in the patch. That code makes
> the intent explicit, i.e., install the notice receiver only when a connection
> object actually exists. That said, I'm also OK with simply calling
> PQsetNoticeReceiver() without that check.
>

Every PG**() function checks if conn is NULL, so I am okay to remove the check.

>
>> Another point is, in pg_connect_server I don't get the value of adding another PGConn variable start_conn, can't we use conn itself...?
>> I hope this helps.
>
> Not only connect_pg_server() but libpqsrv_connect_complete() has
> a PG_TRY/PG_CATCH block. So if start_conn were not used, an error thrown
> in libpqsrv_connect_complete() could cause the current connection (conn) to
> be cleaned up twice unexpectedly: once in libpqsrv_connect_complete() and
> again in connect_pg_server(). I guess that's why Chao introduced start_conn.
>

Exactly. With introducing start_conn, when libpqsrv_connect_complete() raises an error, conn is still NULL, so that PG_CATCH clause won’t cleanup conn, which keeps the same behavior as the old code.

PFA v4, just removed conn NULL check.

Best regards,
--
Chao Li (Evan)
HighGo Software Co., Ltd.
https://www.highgo.com/