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
