Re: BUG #18203: Logical Replication initial sync failure

On Tue, 21 Nov 2023 at 03:38, Peter Smith <smithpb2250@gmail.com> wrote:
>
> On Mon, Nov 20, 2023 at 8:23 PM vignesh C <vignesh21@gmail.com> wrote:
> >
> ...
> > Thanks for reporting this issue, I was able to reproduce the issue.
> > This issue is happening because we are trying to specify the column
> > list while the table sync worker is copying data for the table.
> > I felt we should not specify the column list when the table has no columns.
> > Attached patch has the changes to handle the same.
> >
>
> Hi Vignesh,
>
> One small comment about the patch:
>
> - appendStringInfo(&cmd, "COPY %s (",
> + appendStringInfo(&cmd, "COPY %s ",
>   quote_qualified_identifier(lrel.nspname, lrel.relname));
>
> - /*
> - * XXX Do we need to list the columns in all cases? Maybe we're
> - * replicating all columns?
> - */
> - for (int i = 0; i < lrel.natts; i++)
> + /* If the table has columns, then specify the columns */
> + if (lrel.natts)
>   {
> - if (i > 0)
> - appendStringInfoString(&cmd, ", ");
> + appendStringInfoString(&cmd, "(");
>
> For consistent whitespace handling, and to prevent double-spacing when
> there are no columns, I think that the trailing space of "COPY %s "
> should be removed/replaced by a leading space for the "(".

This is fixed in the v2 version patch attached at:
https://www.postgresql.org/message-id/CALDaNm2kSJCVree6R1iKxjbA%3DSg4raFi3Fi_%2BwfF2xn2roxqCg%40mail.gmail.com

Regards,
Vignesh