On 2020/03/19 12:02, Amit Langote wrote:
On Thu, Mar 19, 2020 at 11:45 AM Fujii Masao
<masao.fu...@oss.nttdata.com> wrote:
On 2020/03/19 11:32, Amit Langote wrote:
On Thu, Mar 19, 2020 at 11:24 AM Alvaro Herrera
<alvhe...@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?
I see that the latest patch modifies pg_stat_progress_basebackup view
to return NULL, so not exactly. IIUC, Magnus seems to be advocating
to *centralize* this in pg_stat_get_progress_info(), which all views
are based on, which means we need to globally define a NULL param
value, as Alvaro also pointed out.
But...
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.
This is the first instance of needing to display NULL in a progress
view, so a non-general solution may be enough for now. IOW, your
latest patch is good enough for that. :)
Ok, so barring any objection, I will commit the latest patch.
Regards,
--
Fujii Masao
NTT DATA CORPORATION
Advanced Platform Technology Group
Research and Development Headquarters
