On Fri, 18 Jul 2025 at 16:15, Amit Kapila <amit.kapila16@gmail.com> wrote:
>
> On Thu, Jul 17, 2025 at 4:52 PM vignesh C <vignesh21@gmail.com> wrote:
> >
> > The attached v20250717 version patch has the changes for the same.
> >
>
> Few comments on 0001 and 0002:
> 0001
> 1. Instead of introducing a new function, can we think of extending
> the existing function pg_get_sequence_data()?
Yes, this can be extended. Modified.
> 0002
> 2.
> postgres=# Create publication pub2 for all tables, sequences;
> CREATE PUBLICATION
> ...
> postgres=# Create publication pub3 for all tables, all sequences;
> ERROR: syntax error at or near "all"
> LINE 1: Create publication pub3 for all tables, all sequences;
>
> I was expecting first syntax to give ERROR and second to work. I have
> given this comment in my earlier email[1]. I see a follow up response
> by Nisha indicating that she agreed with it [2]. By any chance, did
> you people misunderstood and implemented the reverse of what I asked?
This was a misunderstanding here. Updated accordingly.
> 3.
> postgres=> Create publication pub3 for all tables, sequences;
> ERROR: must be superuser to create a FOR ALL TABLES publication
>
> In the above, the publication is both FOR ALL TABLES and ALL
> SEQUENCES. Won't it be better to give a message like: "must be
> superuser to create a FOR ALL TABLES OR ALL SEQUENCES publication"? I
> think we can give this same message in cases where publication is (a)
> FOR ALL TABLES, (b) FOR ALL SEQUENCES, or (c) FOR ALL TABLES,
> SEQUENCES.
Modified
> Whatever we decide here, we can follow that in other parts of the
> patch (where applicable) as well. For example, one case is as below:
> + if (!superuser_arg(newOwnerId))
> + {
> + if (form->puballtables)
> + ereport(ERROR,
> + errcode(ERRCODE_INSUFFICIENT_PRIVILEGE),
> + errmsg("permission denied to change owner of publication \"%s\"",
> + NameStr(form->pubname)),
> + errhint("The owner of a FOR ALL TABLES publication must be a superuser."));
> + if (form->puballsequences)
> + ereport(ERROR,
> + errcode(ERRCODE_INSUFFICIENT_PRIVILEGE),
> + errmsg("permission denied to change owner of publication \"%s\"",
> + NameStr(form->pubname)),
> + errhint("The owner of a FOR ALL SEQUENCES publication must be a superuser."));
Modified here too.
The attached v20250720 version patch has the changes for the same.
Regards,
Vignesh
