Re: pg_stat_progress_basebackup - progress reporting forpg_basebackup, in the server side

Поиск
Список
Период
Сортировка
От Amit Langote
Тема Re: pg_stat_progress_basebackup - progress reporting forpg_basebackup, in the server side
Дата
Msg-id CA+HiwqF+wo5HLHqCV4CAnN9GPMhxkBT1TKP7zV-SdJ8Ca7uSOw@mail.gmail.com
обсуждение исходный текст
Ответ на Re: pg_stat_progress_basebackup - progress reporting forpg_basebackup, in the server side  (Fujii Masao <masao.fujii@oss.nttdata.com>)
Ответы Re: pg_stat_progress_basebackup - progress reporting forpg_basebackup, in the server side  (Alvaro Herrera <alvherre@2ndquadrant.com>)
Список pgsql-hackers
Hello,

On Thu, Mar 19, 2020 at 1:47 AM Fujii Masao <masao.fujii@oss.nttdata.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



В списке pgsql-hackers по дате отправления:

Предыдущее
От: Michael Paquier
Дата:
Сообщение: Re: Thinko in index_concurrently_swap comment
Следующее
От: Tomas Vondra
Дата:
Сообщение: Re: Parallel grouping sets