GetStandbyFlushRecPtr() : OUT param is not optional like elsewhere.

Hi Amul,

-   *tli = replayTLI;
+   if (tli)
+       *tli = replayTLI;

I would guess the difference here is that GetStandbyFlushRecPtr is
static. It is used 3 times in walsender.c and in all cases the
argument can't be NULL.

So I'm not certain what we will gain from the proposed check.

--
Best regards,
Aleksander Alekseev

Hello

On 2022-Jul-20, Amul Sul wrote:

If you look at GetFlushRecPtr() function the OUT parameter for
TimeLineID is optional and this is not only one, see
GetWalRcvFlushRecPtr(), GetXLogReplayRecPtr(), etc.

I think we have missed that for GetStandbyFlushRecPtr(), to be
inlined, we should change this as follow:

This is something we decide mostly on a case-by-case basis. There's no
fixed rule that all out params have to be optional.

If anything is improved by this change, let's see what it is.

--
Álvaro Herrera Breisgau, Deutschland — https://www.EnterpriseDB.com/

Thanks Aleksander and Álvaro for your inputs.

I understand this change is not making any improvement to the current
code. I was a bit concerned regarding the design and consistency of
the function that exists for the server in recovery and for the server
that is not in recovery. I was trying to write following snippet
where I am interested only for XLogRecPtr:

recPtr = RecoveryInProgress() ? GetStandbyFlushRecPtr(NULL) :
GetFlushRecPtr(NULL);

But I can't write this since I have to pass an argument for
GetStandbyFlushRecPtr() but that is not compulsory for
GetFlushRecPtr().

I agree to reject proposed changes since that is not useful
immediately and have no rule for the optional argument for the
similar-looking function.

Regards,
Amul