On Tue, Feb 9, 2021 at 12:06 AM Matthias van de Meent <boekewurm+postg...@gmail.com> wrote: > With [0] we got COPY progress reporting. Before the column names of > this newly added view are effectively set in stone with the release of > pg14, I propose the following set of relatively small patches. These > are v2, because it is a patchset that is based on a set of patches > that I previously posted in [0].
Thanks for working on the patches. Here are some comments: 0001 - +1 to add tuples_excluded and the patch LGTM. 0002 - Yes, the tuples_processed or tuples_excluded makes more sense to me than lines_processed and lines_excluded. The patch LGTM. 0003 - Instead of just adding the progress reporting to "See also" sections in the footer of the respective pages analyze, cluster and others, it would be nice if we have a mention of it in the description as pg_basebackup has something like below: <para> Whenever <application>pg_basebackup</application> is taking a base backup, the server's <structname>pg_stat_progress_basebackup</structname> view will report the progress of the backup. See <xref linkend="basebackup-progress-reporting"/> for details. 0004 - 1) How about PROGRESS_COPY_COMMAND_TYPE instead of PROGRESS_COPY_COMMAND? The names looks bit confusing with the existing PROGRESS_COMMAND_COPY. 0005 - 1) How about + or <literal>CALLBACK</literal> (used in the table synchronization background + worker). instead of + or <literal>CALLBACK</literal> (used in the tablesync background + worker). Because "table synchronization" is being used in logical-replication.sgml. 2) I think cstate->copy_src = COPY_CALLBACK is assigned after the switch case added in copyfrom.c if (data_source_cb) { cstate->copy_src = COPY_CALLBACK; cstate->data_source_cb = data_source_cb; } Also, you can add this to the current commitfest. With Regards, Bharath Rupireddy. EnterpriseDB: http://www.enterprisedb.com