| From: | Andres Freund <andres(at)anarazel(dot)de> |
|---|---|
| To: | Peter Eisentraut <peter(at)eisentraut(dot)org> |
| Cc: | Jeff Davis <jdavis(at)postgresql(dot)org>, pgsql-committers(at)lists(dot)postgresql(dot)org |
| Subject: | Re: pgsql: CREATE SUBSCRIPTION ... SERVER. |
| Date: | 2026-03-11 21:49:00 |
| Message-ID: | xvdjrdqnpap3uq7owbaox3r7p5gf7sv62aaqf2ju3vb6yglatr@kvvwhoudrlxq |
| Views: | Whole Thread | Raw Message | Download mbox | Resend email |
| Thread: | |
| Lists: | pgsql-committers |
Hi,
On 2026-03-11 08:37:13 +0100, Peter Eisentraut wrote:
> On 06.03.26 17:44, Jeff Davis wrote:
> > CREATE SUBSCRIPTION ... SERVER.
>
> In src/backend/foreign/foreign.c, this
>
> volatile text *connection_text = NULL;
>
> should probably be
>
> text *volatile connection_text = NULL;
>
> similar to commit 6307b096e25.
Seems like the issue is a bit bigger to me. Isn't the whole way the function
uses PG_TRY / PG_FINALLY just plain odd?
The only reason connection_text needs to be volatile is because it's modified
in PG_TRY and then accessed in PG_FINALLY. But what's the point of the
latter? If an error was thrown, why would we want to construct the 'result'
string, as the error isn't caught, there's no PG_CATCH. So now the function
builds the result string in case of an error, just to throw it away.
Am I missing something?
I'm also rather unconvinced by ForeignServerConnectionString() creating a
temporary memory context. When would you ever use the function in a long lived
memory context?
Greetings,
Andres
| From | Date | Subject | |
|---|---|---|---|
| Next Message | Andres Freund | 2026-03-11 21:56:53 | pgsql: bufmgr: Fix use of wrong variable in GetPrivateRefCountEntrySlow |
| Previous Message | Jeff Davis | 2026-03-11 21:30:48 | pgsql: Fix use of volatile. |