Re: Logical Replication of sequences - Mailing list pgsql-hackers

From Amit Kapila
Subject Re: Logical Replication of sequences
Date
Msg-id CAA4eK1Jd+6pWp_KejFc052JQC5jLsGb-2pzOCXt61G9bmiYtPA@mail.gmail.com
Whole thread Raw
In response to Re: Logical Replication of sequences  (Shinya Kato <shinya11.kato@gmail.com>)
Responses Re: Logical Replication of sequences
List pgsql-hackers
On Thu, Nov 6, 2025 at 8:00 AM Shinya Kato <shinya11.kato@gmail.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.



pgsql-hackers by date:

Previous
From: Shubham Khanna
Date:
Subject: Re: Add support for specifying tables in pg_createsubscriber.
Next
From: Peter Geoghegan
Date:
Subject: Re: index prefetching