Re: [17] CREATE SUBSCRIPTION ... SERVER

On Wed, Jan 24, 2024 at 7:15 AM Jeff Davis <pgsql(at)j-davis(dot)com> wrote:
>
> On Tue, 2024-01-23 at 15:21 +0530, Ashutosh Bapat wrote:
> > I am with the prefix. The changes it causes make review difficult. If
> > you can separate those changes into a patch that will help.
>
> I ended up just removing the dummy FDW. Real users are likely to want
> to use postgres_fdw, and if not, it's easy enough to issue a CREATE
> FOREIGN DATA WRAPPER. Or I can bring it back if desired.
>
> Updated patch set (patches are renumbered):
>
> * removed dummy FDW and test churn
> * made a new pg_connection_validator function which leaves
> postgresql_fdw_validator in place. (I didn't document the new function
> -- should I?)
> * included your tests improvements
> * removed dependency from the subscription to the user mapping -- we
> don't depend on the user mapping for foreign tables, so we shouldn't
> depend on them here. Of course a change to a user mapping still
> invalidates the subscription worker and it will restart.
> * general cleanup
>

Thanks.

> Overall it's simpler and hopefully easier to review. The patch to
> introduce the pg_create_connection role could use some more discussion,
> but I believe 0001 and 0002 are nearly ready.

0001 commit message says "in preparation of CREATE SUBSCRIPTION" but I
do not see the function being used anywhere except in testcases. Am I
missing something? Is this function necessary for this feature?

But more importantly this function and its minions are closely tied
with libpq and not an FDW. Converting a server and user mapping to
conninfo should be delegated to the FDW being used since that FDW
knows best how to use those options. Similarly options_to_conninfo()
should be delegated to the FDW. I imagine that the FDWs which want to
support subscriptions will need to implement hooks in
WalReceiverFunctionsType which seems to be designed to be pluggable.
--- quote
This API should be considered internal at the moment, but we could open it
up for 3rd party replacements of libpqwalreceiver in the future, allowing
pluggable methods for receiving WAL.
--- unquote
Not all of those hooks are applicable to every FDW since the publisher
may be different and may not provide all the functionality. So we
might need to rethink WalReceiverFunctionsType interface eventually.
But for now, we will need to change postgres_fdw to implement it.

We should mention something about the user mapping that will be used
to connect to SERVER when subscription specifies SERVER. I am not sure
where to mention this. May be we can get some clue from foreign server
documentation.

--
Best Wishes,
Ashutosh Bapat