Re: Add backup_type to pg_stat_progress_basebackup - Mailing list pgsql-hackers

On Mon, Aug 4, 2025 at 1:57 AM Yugo Nagata <nagata@sraoss.co.jp> wrote:
>
> On Fri, 1 Aug 2025 16:12:15 -0700
> Masahiko Sawada <sawada.mshk@gmail.com> wrote:
>
> > The patch seems reasonably simple and looks good to me. I've updated
> > the comment in bbsink_progress_new() and attached the modified version
> > patch (with the commit message). Please review it.
>
>         /*
> -        * Report that a base backup is in progress, and set the total size of the
> -        * backup to -1, which will get translated to NULL. If we're estimating
> -        * the backup size, we'll insert the real estimate when we have it.
> +        * Report that a base backup is in progress, and set the backup type and
> +        * the total size of the backup to -1, which will get translated to NULL,
> +        * and backup. If we're estimating the backup size, we'll insert the real
> +        * estimate when we have it.
>          */
>
> It seems to me that "set the backup type and the total size of the backup to -1"
> is a bit confusing because it could be read that the backup type would be also set
> to -1, and the subsequent sentence describes just the total size.
>
> Therefore, I think it is better to just add "Also, the backup type is set."
> (or similar) to the end of the comment block.
>
> That said, I'm not a native English speaker, so if no one else sees a problem,
> I'm fine with it.

Agreed. I've changed the comment and pushed the patch.

Regards,

--
Masahiko Sawada
Amazon Web Services: https://aws.amazon.com