Re: Logical Replication of sequences

From: Nisha Moond <nisha(dot)moond412(at)gmail(dot)com>
To: vignesh C <vignesh21(at)gmail(dot)com>
Cc: Amit Kapila <amit(dot)kapila16(at)gmail(dot)com>, shveta malik <shveta(dot)malik(at)gmail(dot)com>, Peter Smith <smithpb2250(at)gmail(dot)com>, "Hayato Kuroda (Fujitsu)" <kuroda(dot)hayato(at)fujitsu(dot)com>, Masahiko Sawada <sawada(dot)mshk(at)gmail(dot)com>, Shlok Kyal <shlok(dot)kyal(dot)oss(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>, "Zhijie Hou (Fujitsu)" <houzj(dot)fnst(at)fujitsu(dot)com>, "Jonathan S(dot) Katz" <jkatz(at)postgresql(dot)org>
Subject: Re: Logical Replication of sequences
Date: 2025-06-24 09:37:28
Message-ID: CABdArM7h1qQLUb_S7i6MrLPEtHXnX+Y2fPQaSnqhCdHktcQk5Q@mail.gmail.com
Views: Whole Thread | Raw Message | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

On Sun, Jun 22, 2025 at 8:05 AM vignesh C <vignesh21(at)gmail(dot)com> wrote:
>
> Thanks for the comment, the attached v20250622 version patch has the
> changes for the same.
>

Thanks for the patches, please find my review comments for patches 001 and 002:

1) patch-001 :pg_sequence_state()

+ /* open and lock sequence */
+ init_sequence(seq_relid, &elm, &seqrel);

/ open/ Open
~~~

patch-0002:

2)
- /* FOR ALL TABLES requires superuser */
- if (stmt->for_all_tables && !superuser())
- ereport(ERROR,
- (errcode(ERRCODE_INSUFFICIENT_PRIVILEGE),
- errmsg("must be superuser to create FOR ALL TABLES publication")));
+ if (!superuser())

I think we can retain the original comment here with required modification :
/* FOR ALL TABLES and FOR ALL SEQUENCES requires superuser */
~~~

3) ALL TABLES vs ALL SEQUENCES

For the command:
CREATE PUBLICATION FOR ALL TABLES, [...]
- only "SEQUENCES" is allowed after ',', and TABLE or TABLES IN
SCHEMA are not allowed. Which aligns with the fact that "all tables"
is inclusive of "FOR TABLE" and "FOR TABLES IN SCHEMA".
Therefore, adding and dropping of tables from a "ALL TABLES"
publication is also not allowed. e.g.,

```
postgres=# alter publication test add table t1;
ERROR: publication "test" is defined as FOR ALL TABLES
DETAIL: Tables cannot be added to or dropped from FOR ALL TABLES publications.

postgres=# alter publication test add tables in schema public;
ERROR: publication "test" is defined as FOR ALL TABLES
DETAIL: Schemas cannot be added to or dropped from FOR ALL TABLES publications.
```

However, for ALL SEQUENCES, the behavior seems inconsistent. CREATE
PUBLICATION doesn’t allow adding TABLE or TABLES IN SCHEMA along with
ALL SEQUENCES, but ALTER PUBLICATION does.
e.g., for a all sequence publication 'pubs', below succeeds :
postgres=# alter publication pubs add table t1;
ALTER PUBLICATION

Is this expected?
If adding tables is allowed using ALTER PUBLICATION, perhaps it should
also be permitted during CREATE PUBLICATION or disallowed in both
cases. Thoughts?
~~~

4) Consider a publication 'pubs' on all sequences and 'n1' is a sequence, now -

postgres=# alter publication pubs drop table n1;
ERROR: relation "n1" is not part of the publication

This error message can be misleading, as the relation n1 is part of
the publication - it's just a sequence, not a table.
It might be more accurate to add a DETAIL line similar to ADD case:

postgres=# alter publication pubs add table n1;
ERROR: cannot add relation "n1" to publication
DETAIL: This operation is not supported for sequences.
~~~

--
Thanks.
Nisha

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Roman Khapov 2025-06-24 09:40:37 Hooks or another better way to track session objects
Previous Message Tomas Vondra 2025-06-24 09:20:15 Re: pgsql: Introduce pg_shmem_allocations_numa view