On Fri, 2026-03-06 at 21:49 +0530, Ashutosh Bapat wrote:
> I don't think we need a separate ForeignServerName function. In
> AlterSubscription() we already have ForeignSever object which has
> server name in it. Other two callers invoke
> ForeignServerConnectionString() which in turn fetches ForeignServer
> object. Those callers instead may fetch ForeignServer object
> themselves and pass it to ForeignServerConnectionString() and use it
> in the error message.
Done.
> The patch has changes to pg_dump.c but there is no corresponding
> test.
> But I don't think we need a separate test. If the objects created in
> regress/*.sql tests are not dropped, 002_pg_upgrade.pl would test
> dump/restore of subscriptions with server.
It seems that foreign_data.sql expects there to be zero FDWs, servers,
and user mappings, so it's not quite that simple. I'm not entirely sure
why that is, but I suppose it's meant to be tested in 002_pg_dump.pl
instead.
I wrote the tests there (attached), which revealed that CREATE FOREIGN
DATA WRAPPER ... CONNECTION wasn't being dumped properly. I attached a
separate fix for that.
Unfortunately I don't think we can rely on regress.so being available
when 002_pg_dump.pl runs. Do you have an idea how I can effectively
test the FDW (which is needed to test the server and subscription)? I
suppose I could make it a built-in function, and that wouldn't be so
bad, but not ideal. Right now this test is failing for CI on debian
autoconf.
> I think we need tests for testing changes in connection when ALTER
> SUBSCRIPTION ... SERVER is executed and also those for switching
> between SERVER and CONNECTION.
Done.
Attached series including patches to address Andres's and Amit's
comments, too.
Thank you!
Regards,
Jeff Davis
