Re: pg_control_recovery() return value when not in recovery

On 2017-10-13 16:31:37 -0700, Joe Conway wrote:
> On 09/17/2017 11:29 PM, Andres Freund wrote:
> > On 2017-09-18 07:24:43 +0100, Simon Riggs wrote:
> >> On 18 September 2017 at 05:50, Andres Freund <andres(at)anarazel(dot)de> wrote:
> >> > Hi,
> >> >
> >> > Just noticed that we're returning the underlying values for
> >> > pg_control_recovery() without any checks:
> >> > postgres[14388][1]=# SELECT * FROM pg_control_recovery();
> >> > ┌──────────────────────┬───────────────────────────┬──────────────────┬────────────────┬───────────────────────────────┐
> >> > │ min_recovery_end_lsn │ min_recovery_end_timeline │ backup_start_lsn │ backup_end_lsn │ end_of_backup_record_required │
> >> > ├──────────────────────┼───────────────────────────┼──────────────────┼────────────────┼───────────────────────────────┤
> >> > │ 0/0 │ 0 │ 0/0 │ 0/0 │ f │
> >> > └──────────────────────┴───────────────────────────┴──────────────────┴────────────────┴───────────────────────────────┘
> >> > (1 row)
> >>
> >> Yes, that would have made sense for these to be NULL
> >
> > Yea, that's what I think was well. Joe, IIRC that's your code, do you
> > agree as well?
>
> Sorry for the slow response, but thinking back on this now, the idea of
> these functions, in my mind at least, was to provide as close to the
> same output as possible to what pg_controldata outputs. So:
>
> # pg_controldata
> ...
> Minimum recovery ending location: 0/0
> Min recovery ending loc's timeline: 0
> Backup start location: 0/0
> Backup end location: 0/0
> End-of-backup record required: no
> ...
>
> So if we make a change here, do we also change pg_controldata?

I'm unconvince that they need to be kept that close. For one, text
output doesn't have the concept of NULLs. Secondly, pg_controldata
output also the cluster state at the same time.

Greetings,

Andres Freund