Re: Logical Replication of sequences

On Thu, Oct 9, 2025 at 3:08 PM vignesh C <vignesh21@gmail.com> wrote:
>
> >
> > So my suggestion is very different.  Just this:
> > "ALTER SUBSCRIPTION sub REFRESH SEQUENCES"
> >
> > I feel this is entirely consistent, because:
> >
> > PUBLICATION objects have changed. Refresh me the new objects => ALTER
> > SUBSCRIPTION sub REFRESH PUBLICATION;
> >
> > SEQUENCE values have changed. Refresh me the new values => ALTER
> > SUBSCRIPTION sub REFRESH SEQUENCES;
>
> +1 for this syntax. Here is an updated patch having the changes for the same.
>

Few comments:
=============
1.
+ /* From version 19, inclusion of sequences in the target is supported */
+ if (server_version >= 190000)
+ appendStringInfo(&cmd,
+ "UNION ALL\n"
+ "  SELECT DISTINCT s.schemaname, s.sequencename, NULL::int2vector AS
attrs, 'S'::\"char\" AS relkind\n"

Instead of hard coding 'S', can we use RELKIND_SEQUENCE?

2.
  else
  {
  tableRow[2] = NAMEARRAYOID;
- appendStringInfoString(&cmd, "SELECT DISTINCT t.schemaname, t.tablename \n");
+ appendStringInfoString(&cmd, "SELECT DISTINCT t.schemaname, t.tablename\n");

Why the above change?

3.
+ pubisseq = (relinfo->relkind == RELKIND_SEQUENCE);
+ subisseq = (relkind == RELKIND_SEQUENCE);
+
+ /*
+ * Allow RELKIND_RELATION and RELKIND_PARTITIONED_TABLE to be
+ * treated interchangeably, but ensure that sequences
+ * (RELKIND_SEQUENCE) match exactly on both publisher and
+ * subscriber.
+ */
+ if (pubisseq != subisseq)
+ ereport(ERROR,
+ errcode(ERRCODE_OBJECT_NOT_IN_PREREQUISITE_STATE),

We can directly compare relkind here and avoid having two extra
variables. The same code is present in AlterSubscription_refresh. So,
we can make the similar change there as well. BTW, the same scenario
could happen between table and sequences, no? If so, then we should
deal with that as well. It would be better if can make these checks as
part of CheckSubscriptionRelkind().

4.
+ errmsg("relation \"%s.%s\" has relkind \"%c\" on the publisher but
relkind \"%c\" on the subscriber",

I would like this message to be bit short and speak in terms of source
and target, something like: errmsg("relation \"%s.%s\" type mismatch:
source \"%c\", target \"%c\"")

5.
+ *
+ * XXX: Currently, a replication slot is created for all
+ * subscriptions, including those for sequence-only publications.
+ * However, this is unnecessary, as incremental synchronization of
+ * sequences is not supported.

Can we try to avoid creating slot/origin for sequence-only
subscriptions? We don't want to make code complicated due to this, so
try to create a top-up patch so that we can evaluate this change
separately?

6.
@@ -2403,11 +2503,15 @@ check_publications_origin(WalReceiverConn
*wrconn, List *publications,
for (i = 0; i < subrel_count; i++)
{
Oid relid = subrel_local_oids[i];
- char    *schemaname = get_namespace_name(get_rel_namespace(relid));
- char    *tablename = get_rel_name(relid);
- appendStringInfo(&cmd, "AND NOT (N.nspname = '%s' AND C.relname = '%s')\n",
- schemaname, tablename);
+ if (get_rel_relkind(relid) != RELKIND_SEQUENCE)

Why do we have the above check in the 0002 patch? We can add some
comments to clarify the same, if it is required. Also, we should do
this origin check during ALTER SUBSCRIPTION … REFRESH command as we
don't have incremental WAL based filtering of origin for sequences.

-- 
With Regards,
Amit Kapila.