From: Ronan Dunklau <ronan.dunklau@aiven.io>
To: Michael Paquier <michael@paquier.xyz>
Cc: pgsql-hackers@lists.postgresql.org, Bharath Rupireddy <bharath.rupireddyforpostgres@gmail.com>
Cc: Kyotaro Horiguchi <horikyota.ntt@gmail.com>
Cc: Masahiko Sawada <sawada.mshk@gmail.com>
Subject: Re: pg_receivewal starting position
Date: Mon, 25 Oct 2021 10:24:46 +0200
Message-ID: <21259852.EfDdHjke4D@aivenronan> (raw)
In-Reply-To: <YXZmZU6c081lRJ7z@paquier.xyz>
References: <CALj2ACW6YA0SfqwPm1-FRRdeNj1e9isiSax3jZ8MmyQYYQQWNw@mail.gmail.com>
<3162614.44csPzL39Z@aivenronan>
<YXZmZU6c081lRJ7z@paquier.xyz>
Le lundi 25 octobre 2021, 10:10:13 CEST Michael Paquier a écrit :
> On Mon, Oct 25, 2021 at 09:50:01AM +0200, Ronan Dunklau wrote:
> > Le lundi 25 octobre 2021, 09:40:10 CEST Michael Paquier a écrit :
> >> On Mon, Oct 25, 2021 at 09:15:32AM +0200, Ronan Dunklau wrote:
> >>> Does it make sense though ? The NULL slot_name case handling is pretty
> >>> straight forward has it will be handled by string formatting, but in the
> >>> case of a null restart_lsn, we have no way of knowing if the command was
> >>> issued at all.
> >>
> >> If I am following your point, I don't think that it matters much here,
> >> and it seems useful to me to be able to pass NULL for both of them, so
> >> as one can check if the slot exists or not with an API designed this
> >> way.
> >
> > You're right, but I'm afraid we would have to check the server version
> > twice in any case different from the basic pg_receivewal on (once in the
> > function itself, and one before calling it if we want a meaningful
> > result). Maybe we should move the version check outside the
> > GetSlotInformation function to avoid this, and let it fail with a syntax
> > error when the server doesn't support it ?
> With the approach taken by the patch, we fall down silently to the
> previous behavior if we connect to a server <= 14, and rely on the new
> behavior with a server >= 15, ensuring compatibility. Why would you
> want to make sure that the command is executed when we should just
> enforce that the old behavior is what happens when there is a slot
> defined and a backend <= 14?
Sorry I haven't been clear. For the use case of this patch, the current
approach is perfect (and we supply the restart_lsn).
However, if we want to support the case of "just check if the slot exists", we
need to make sure the command is actually executed, and check the version
before calling the function, which would make the check executed twice.
What I'm proposing is just that we let the responsibility of checking
PQServerVersion() to the caller, and remove it from GetSlotInformation, ie:
- if (replication_slot != NULL)
+ if (replication_slot != NULL && PQserverVersion(conn) >=
150000)
{
if (!GetSlotInformation(conn, replication_slot,
&stream.startpos,
&stream.timeline))
That way, if we introduce a caller wanting to use this function as an API to
check a slot exists, the usage of checking the server version beforehand will
be consistent.
--
Ronan Dunklau
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: pgsql-hackers@postgresql.org
Cc: ronan.dunklau@aiven.io, michael@paquier.xyz, bharath.rupireddyforpostgres@gmail.com, horikyota.ntt@gmail.com, sawada.mshk@gmail.com
Subject: Re: pg_receivewal starting position
In-Reply-To: <21259852.EfDdHjke4D@aivenronan>
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
This inbox is served by DDX for PostgreSQL; see mirroring instructions
for how to clone and mirror all data and code used for this inbox