Re: Logical Replication of sequences

On Wed, 18 Dec 2024 at 11:40, Peter Smith <smithpb2250(at)gmail(dot)com> wrote:
>
> Hi Vignesh,
>
> Here are some review comments for the patch v20241211-0002.
> ~~~
>
> publish option effect fro SEQUENCE replication?
>
> 2.
> It's not obvious to me if the SEQUENCE replication stuff is affected
> but the setting of pubactions (ie the 'publish' option). I'm thinking
> that probably anything to do with SEQUENCEs may no be considered a DML
> operation, but if that is true it might be better to explicitly say
> so.
>
> Also, we might need to include a test to show even if publish='' that
> the SEQUENCE synchronization is not affected.

Since we just synchronize the data and we don't replicate incremental
changes and insert/update/delete are only for incremental data sync, I
felt we need not mention this explicitly in this case as we have
mentioned that "Incremental sequence changes are not replicated" in
0005 patch. I also felt that there was no need to add a testcase for
this as currently it has nothing to do with this option.

>
> ======
> src/backend/commands/publicationcmds.c
>
> CreatePublication:
>
> 3.
> - /* FOR ALL TABLES requires superuser */
> - if (stmt->for_all_tables && !superuser())
> + /* FOR ALL TABLES or FOR ALL SEQUENCES requires superuser */
> + if ((stmt->for_all_tables || stmt->for_all_sequences) && !superuser())
> ereport(ERROR,
> (errcode(ERRCODE_INSUFFICIENT_PRIVILEGE),
> - errmsg("must be superuser to create FOR ALL TABLES publication")));
> + errmsg("must be superuser to create a %s publication",
> + stmt->for_all_tables ? "FOR ALL TABLES" :
> + "FOR ALL SEQUENCES")));
>
> It seems a quirk that if FOR ALL TABLES and FOR ALL SEQUENCES are
> specified at the same time then you would only get the "FOR ALL
> TABLES" error, but maybe that is OK?

I have modified the error message slightly

> ~~~
>
> AlterPublicationOwner_internal:
>
> 4.
> Ditto quirk as commented above, but maybe it is OK.

I have modified the error message slightly

The rest of the comments are fixed. Also the comments from [1] are fixed.
The attached v202412123 version patch has the changes for the same.

[1] - https://www.postgresql.org/message-id/CAHut%2BPsk1V2eUL_nreWp1trO1iSDqhDtBnfu65PrsoorpuNzKA%40mail.gmail.com

Regards,
Vignesh