On Mon, Oct 25, 2021 at 10:24:46AM +0200, Ronan Dunklau wrote:
> 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.
Ah, good point. My apologies for not following. Indeed, the patch
makes this part of the routine a bit blurry. It is fine by me to do
as you suggest, and let the caller do the version check as you
propose, while making the routine fail if directly called for an older
server.
--
Michael