Re: pg_stat_progress_basebackup - progress reporting for pg_basebackup, in the server side

On 2020/03/19 11:32, Amit Langote wrote:
> On Thu, Mar 19, 2020 at 11:24 AM Alvaro Herrera
> <alvherre@2ndquadrant.com> wrote:
>> On 2020-Mar-19, Amit Langote wrote:
>>
>>> Magnus' idea of checking the values in pg_stat_get_progress_info() to
>>> determine whether to return NULL seems fine to me.

So you think that the latest patch is good enough?

>>>  We will need to
>>> update the documentation of st_progress_param, because it currently
>>> says:
>>>
>>>       *  ...but the meaning of each element in the
>>>       * st_progress_param array is command-specific.
>>>       */
>>>      ProgressCommandType st_progress_command;
>>>      Oid         st_progress_command_target;
>>>      int64       st_progress_param[PGSTAT_NUM_PROGRESS_PARAM];
>>> } PgBackendStatus;
>>>
>>> If we are to define -1 in st_progress_param[] as NULL to the users,
>>> that must be mentioned here.
>>
>> Hmm, why -1?  It seems like a value that we might want to use for other
>> purposes in other params.  Maybe INT64_MIN is a better choice?
> 
> Yes, maybe.

I don't think that we need to define the specific value like -1 as NULL globally.
Which value should be used for that purpose may vary by each command. Only for
pg_stat_progress_basebackup.backup_total, IMO using -1 as special value for
NULL is not so bad idea.

Regards,

-- 
Fujii Masao
NTT DATA CORPORATION
Advanced Platform Technology Group
Research and Development Headquarters