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
