On Fri, May 22, 2026 at 8:33 PM Rafia Sabih <[email protected]> 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. > 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. Regards, -- Fujii Masao
