Re: Logical Replication of sequences

On Thu, 6 Nov 2025 at 10:10, Amit Kapila <amit(dot)kapila16(at)gmail(dot)com> wrote:
>
> On Thu, Nov 6, 2025 at 8:00 AM Shinya Kato <shinya11(dot)kato(at)gmail(dot)com> wrote:
> >
> >
> > I discovered that the sequence sync worker fails for sequences
> > containing single quotes.
> > ---
> > 2025-11-06 10:22:50.335 JST [1096008] ERROR: could not fetch sequence
> > information from the publisher: ERROR: syntax error at or near
> > "quote"
> > LINE 4: FROM ( VALUES ('public', 'regress'quote', 0), ('public', 'ho...
> > ^
> > 2025-11-06 10:22:50.335 JST [1088168] LOG: background worker "logical
> > replication sequencesync worker" (PID 1096008) exited with exit code 1
> > ---
> >
> > I haven't read all the threads, so I might be mistaken, but I've
> > created a patch.

Thanks Kato-san for finding this issue and sharing the changes.

>
> 1.
> + nsp_literal = quote_literal_cstr(seqinfo->nspname);
> + seq_literal = quote_literal_cstr(seqinfo->seqname);
> +
> + appendStringInfo(seqstr, "(%s, %s, %d)",
> + nsp_literal, seq_literal, idx);
> +
> + pfree(nsp_literal);
> + pfree(seq_literal);
>
> We don't need this retail pfree as the current memory context at this
> place will be TopTransactionContext that will anyway be freed after a
> batch of sequences.

Modified

> 2.
> @@ -147,13 +148,18 @@ get_sequences_string(List *seqindexes, StringInfo buf)
> resetStringInfo(buf);
> foreach_int(seqidx, seqindexes)
> {
> + char *qualified_name;
> +
> LogicalRepSequenceInfo *seqinfo =
> (LogicalRepSequenceInfo *) list_nth(seqinfos, seqidx);
>
> if (buf->len > 0)
> appendStringInfoString(buf, ", ");
>
> - appendStringInfo(buf, "\"%s.%s\"", seqinfo->nspname, seqinfo->seqname);
> + qualified_name = quote_qualified_identifier(seqinfo->nspname,
> + seqinfo->seqname);
> + appendStringInfoString(buf, qualified_name);
>
> The function get_sequences_string() is used in WARNING code path and
> normally we don't quote names in messages. For example, see following
> cases:

Agree on this.

> 3. Also, see, if the newly added test can be combined with other existing tests.

Modified

The patch also includes the change for buildfarm failure at [1].
[1] - https://buildfarm.postgresql.org/cgi-bin/show_log.pl?nm=prion&dt=2025-11-05%2010%3A30%3A15

The attached v2 version patch has the changes for the same.

Regards,
Vignesh