From: | Amit Langote <amitlangote09(at)gmail(dot)com> |
---|---|
To: | Fujii Masao <masao(dot)fujii(at)oss(dot)nttdata(dot)com> |
Cc: | Magnus Hagander <magnus(at)hagander(dot)net>, Kyotaro Horiguchi <horikyota(dot)ntt(at)gmail(dot)com>, Peter Eisentraut <peter(dot)eisentraut(at)2ndquadrant(dot)com>, "Shinoda, Noriyoshi (PN Japan A&PS Delivery)" <noriyoshi(dot)shinoda(at)hpe(dot)com>, Masahiko Sawada <masahiko(dot)sawada(at)2ndquadrant(dot)com>, PostgreSQL-development <pgsql-hackers(at)postgresql(dot)org> |
Subject: | Re: pg_stat_progress_basebackup - progress reporting for pg_basebackup, in the server side |
Date: | 2020-03-19 01:02:14 |
Message-ID: | CA+HiwqF+wo5HLHqCV4CAnN9GPMhxkBT1TKP7zV-SdJ8Ca7uSOw@mail.gmail.com |
Views: | Raw Message | Whole Thread | Download mbox | Resend email |
Thread: | |
Lists: | pgsql-hackers |
Hello,
On Thu, Mar 19, 2020 at 1:47 AM Fujii Masao <masao(dot)fujii(at)oss(dot)nttdata(dot)com> wrote:
> On 2020/03/19 1:22, Magnus Hagander wrote:
> >>>>> Would it perhaps be better to return NULL instead of 0 in the
> >>>>> statistics view if there is no data?
> >>>
> >>> Did you miss this comment, or not agree? :)
> >>
> >> Oh, I forgot to attached the patch... Patch attached.
> >> This patch needs to be applied after applying
> >> add_no_estimate_size_v3.patch.
> >
> > :)
> >
> > Hmm. I'm slightly irked by doing the -1 -> NULL conversion in the SQL
> > view. I wonder if it might be worth teaching
> > pg_stat_get_progress_info() about returning NULL?
>
> That's possible by
> - adding the boolean array like st_progress_null[PGSTAT_NUM_PROGRESS_PARAM]
> that indicates whether each column is NULL or not, into PgBackendStatus
> - extending pgstat_progress_update_param() and pgstat_progress_update_multi_param()
> so that they can update the boolean array for NULL
> - updating the progress reporting code so that the extended versions of
> function are used
>
> I didn't adopt this idea because it looks a bit overkill for the purpose.
I tend to agree that this would be too many changes for something only
one place currently needs to use.
> OTOH, this would be good improvement for the progress reporting
> infrastructure and I'm fine to implement it.
Magnus' idea of checking the values in pg_stat_get_progress_info() to
determine whether to return NULL seems fine to me. 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.
--
Thank you,
Amit
From | Date | Subject | |
---|---|---|---|
Next Message | Tomas Vondra | 2020-03-19 02:09:14 | Re: Parallel grouping sets |
Previous Message | Michael Paquier | 2020-03-19 00:54:07 | Re: Thinko in index_concurrently_swap comment |