Re: pgsql: CREATE SUBSCRIPTION ... SERVER.

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

In response to

Responses

Browse pgsql-committers by date

  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.