Re: Logical Replication of sequences

From: Amit Kapila <amit(dot)kapila16(at)gmail(dot)com>
To: Shinya Kato <shinya11(dot)kato(at)gmail(dot)com>
Cc: vignesh C <vignesh21(at)gmail(dot)com>, Peter Smith <smithpb2250(at)gmail(dot)com>, Chao Li <li(dot)evan(dot)chao(at)gmail(dot)com>, Masahiko Sawada <sawada(dot)mshk(at)gmail(dot)com>, "Hayato Kuroda (Fujitsu)" <kuroda(dot)hayato(at)fujitsu(dot)com>, "Zhijie Hou (Fujitsu)" <houzj(dot)fnst(at)fujitsu(dot)com>, Dilip Kumar <dilipbalaut(at)gmail(dot)com>, shveta malik <shveta(dot)malik(at)gmail(dot)com>, Shlok Kyal <shlok(dot)kyal(dot)oss(at)gmail(dot)com>, Nisha Moond <nisha(dot)moond412(at)gmail(dot)com>, Peter Eisentraut <peter(at)eisentraut(dot)org>, PostgreSQL Hackers <pgsql-hackers(at)lists(dot)postgresql(dot)org>, Euler Taveira <euler(at)eulerto(dot)com>, Michael Paquier <michael(at)paquier(dot)xyz>, "Jonathan S(dot) Katz" <jkatz(at)postgresql(dot)org>
Subject: Re: Logical Replication of sequences
Date: 2025-11-06 04:40:23
Message-ID: CAA4eK1Jd+6pWp_KejFc052JQC5jLsGb-2pzOCXt61G9bmiYtPA@mail.gmail.com
Views: Whole Thread | Raw Message | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

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 for spotting the issue and providing a fix. Here are few comments:

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.

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:

parserOpenTable:
ereport(ERROR,
    (errcode(ERRCODE_UNDEFINED_TABLE),
    errmsg("relation \"%s.%s\" does not exist",
        relation->schemaname, relation->relname)));

RangeVarGetRelidExtended:
        ereport(elevel,
            (errcode(ERRCODE_LOCK_NOT_AVAILABLE),
             errmsg("could not obtain lock on relation \"%s.%s\"",
                relation->schemaname, relation->relname)));

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

--
With Regards,
Amit Kapila.

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Peter Geoghegan 2025-11-06 04:55:50 Re: index prefetching
Previous Message Shubham Khanna 2025-11-06 04:05:36 Re: Add support for specifying tables in pg_createsubscriber.