On Wed, Mar 11, 2020 at 5:53 AM Fujii Masao <masao.fu...@oss.nttdata.com> wrote: > > > > On 2020/03/11 3:39, Magnus Hagander wrote: > > On Tue, Mar 10, 2020 at 6:19 PM Fujii Masao <masao.fu...@oss.nttdata.com> > > wrote: > >> > >> > >> > >> On 2020/03/10 22:43, Amit Langote wrote: > >>> On Tue, Mar 10, 2020 at 6:09 PM Fujii Masao <masao.fu...@oss.nttdata.com> > >>> wrote: > >>>>> So, I will make the patch adding support for --no-estimate-size option > >>>>> in pg_basebackup. > >>>> > >>>> Patch attached. > >>> > >>> Like the idea and the patch looks mostly good. > >> > >> Thanks for reviewing the patch! > >> > >>> + total size. If the estimation is disabled in > >>> + <application>pg_basebackup</application> > >>> + (i.e., <literal>--no-estimate-size</literal> option is specified), > >>> + this is always <literal>0</literal>. > >>> > >>> "always" seems unnecessary. > >> > >> Fixed. > >> > >>> + This option prevents the server from estimating the total > >>> + amount of backup data that will be streamed. In other words, > >>> + <literal>backup_total</literal> column in the > >>> + <structname>pg_stat_progress_basebackup</structname> > >>> + view always indicates <literal>0</literal> if this option is > >>> enabled. > >>> > >>> Here too. > >> > >> Fixed. > >> > >> Attached is the updated version of the patch. > > > > 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? :) > > Also, should it really be the server version that decides how this > > feature behaves, and not the pg_basebackup version? Given that the > > implementation is entirely in the client, it seems that's more > > logical? > > Yeah, you're right. I changed the patch that way. > Attached is the updated version of the patch. The other changes in it look good! //Magnus
