| From: | Jeff Davis <pgsql(at)j-davis(dot)com> |
|---|---|
| To: | Andres Freund <andres(at)anarazel(dot)de>, 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-12 00:59:40 |
| Message-ID: | 03d52b80fd264bb683fb2c46ad7a2934cb656f1a.camel@j-davis.com |
| Views: | Whole Thread | Raw Message | Download mbox | Resend email |
| Thread: | |
| Lists: | pgsql-committers |
On Wed, 2026-03-11 at 17:49 -0400, Andres Freund wrote:
> 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.
I believe it would be correct if the variable was not volatile at all,
because it's only set in the last statement of PG_TRY(), so the
PG_FINALLY() will never need to read the modified variable on the error
path. I added the volatile to follow the expected pattern, in case the
code is adjusted later.
That leaves some awkwardness, but I don't immediately see a cleaner way
to do it. Suggestions welcome.
> 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?
It's called by GetSubscription(), and the context is ApplyContext.
Regards,
Jeff Davis
| From | Date | Subject | |
|---|---|---|---|
| Next Message | Michael Paquier | 2026-03-12 02:48:57 | pgsql: Use streaming read for VACUUM cleanup of GIN |
| Previous Message | Richard Guo | 2026-03-12 00:46:25 | pgsql: Convert NOT IN sublinks to anti-joins when safe |