public inbox for [email protected]
help / color / mirror / Atom feedFrom: Ashutosh Bapat <[email protected]>
To: Jeff Davis <[email protected]>
Cc: Bharath Rupireddy <[email protected]>
Cc: Joe Conway <[email protected]>
Cc: [email protected]
Subject: Re: [17] CREATE SUBSCRIPTION ... SERVER
Date: Tue, 30 Jan 2024 16:17:55 +0530
Message-ID: <CAExHW5uirmv8tj7LJ3zDo2tfaP_fffAeRbKT--3vBe7pa0in0g@mail.gmail.com> (raw)
In-Reply-To: <[email protected]>
References: <[email protected]>
<CAExHW5vv-78ixJs3arsuZ+12A4AyqehJnDPhejp0m4FAKG6JZA@mail.gmail.com>
<[email protected]>
<CAExHW5sAo4Gv3AwuxQNGSQ2Z45B8_MZEb-SG-B8D-hLMdrquAg@mail.gmail.com>
<[email protected]>
<[email protected]>
<CAExHW5unvpDv6yMSmqurHP7Du1PqoJFWVxeK-4YNm5EnoNJiSQ@mail.gmail.com>
<[email protected]>
<CAExHW5sQVJeCgCGMkum_k4Z4VPaK5GkdAjHW-d+F3MngBZ9qBA@mail.gmail.com>
<[email protected]>
<[email protected]>
<[email protected]>
<CALj2ACXDua2Az15Kj3OZFaRm49G8-faemiEEv8t9GNCcsxv8Hw@mail.gmail.com>
<[email protected]>
<CAExHW5v5poohyfNRVQp3yjVE56ej0dWvVMbysSSVtg21aHT3uQ@mail.gmail.com>
<[email protected]>
<CAExHW5vF+cp9efQj=-W+Rhbr9f9ZZqt3XFnP0NB5jKsm=C=5tw@mail.gmail.com>
<[email protected]>
<[email protected]>
<CAExHW5uCzS-VeSYQHTHxFSdQik-f_O892xmzrzm2fuO+ro+otA@mail.gmail.com>
<[email protected]>
<CAExHW5vDPBB_PShBmSj=v=Jf1K=vSmWdPZTzCJsLpqDgqek+sA@mail.gmail.com>
<[email protected]>
On Wed, Jan 24, 2024 at 7:15 AM Jeff Davis <[email protected]> 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
view thread (63+ messages) latest in thread
reply
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Reply to all the recipients using the --to and --cc options:
reply via email
To: [email protected]
Cc: [email protected], [email protected], [email protected], [email protected]
Subject: Re: [17] CREATE SUBSCRIPTION ... SERVER
In-Reply-To: <CAExHW5uirmv8tj7LJ3zDo2tfaP_fffAeRbKT--3vBe7pa0in0g@mail.gmail.com>
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
This inbox is served by agora; see mirroring instructions
for how to clone and mirror all data and code used for this inbox