| From: | shveta malik <shveta(dot)malik(at)gmail(dot)com> |
|---|---|
| To: | Shlok Kyal <shlok(dot)kyal(dot)oss(at)gmail(dot)com> |
| Cc: | vignesh C <vignesh21(at)gmail(dot)com>, PostgreSQL Hackers <pgsql-hackers(at)lists(dot)postgresql(dot)org>, shveta malik <shveta(dot)malik(at)gmail(dot)com> |
| Subject: | Re: Support EXCEPT for ALL SEQUENCES publications |
| Date: | 2026-04-14 09:42:17 |
| Message-ID: | CAJpy0uCAJQvBjD7qNWWGnZP_LDwS8AiUJC7YMict9UcYqH=XeQ@mail.gmail.com |
| Views: | Whole Thread | Raw Message | Download mbox | Resend email |
| Thread: | |
| Lists: | pgsql-hackers |
On Mon, Apr 13, 2026 at 10:48 AM shveta malik <shveta(dot)malik(at)gmail(dot)com> wrote:
>
> On Sun, Apr 12, 2026 at 12:33 AM Shlok Kyal <shlok(dot)kyal(dot)oss(at)gmail(dot)com> wrote:
> >
> > Hi Vignesh and Shveta,
> >
> > Thanks for reviewing the patches.
> > I have addressed the comments and attached the updated patch.
> >
> > This also addressed the comments shared by Shveta in [1].
> > [1]: https://www.postgresql.org/message-id/CAJpy0uDU0PrH=gvFZjpphOX7t=2jH5wTqYry=C22vnuJJ9Q5=g@mail.gmail.com
> >
>
> Please find few comments on 001 and 002:
>
>
> v-001:
> 1)
> - List *except_tables; /* tables specified in the EXCEPT clause */
> + List *except_relations; /* relations specified in the EXCEPT
> + * clause */
> Since we have not changed the comments for anything else in patch001,
> we can keep this comment too same as old and changeit in 002.
>
>
> v-002:
> 2)
> pg_publication_rel's prrelid doc says:
> Reference to table
> We shall change it now to 'Reference to table or sequence'
>
> 3)
> In doc, do we eed to change pg_publication_rel's prqual too? IMO, it
> is not applicable to sequence and thus we can change 'relation' to
> 'table' in explanation..
>
> 4)
> Marks the publication as one that synchronizes changes for all sequences
> - in the database, including sequences created in the future.
> + in the database, including sequences created in the future. Sequences
> + listed in <literal>EXCEPT</literal> clause are excluded from the
> + publication.
>
> I think we should place it the end of second paragraph rather than at
> the end of first. How about something liek this:
>
> Marks the publication as one that synchronizes changes for all
> sequences in the database, including sequences created in the future.
>
> Only persistent sequences are included in the publication. Temporary
> sequences and unlogged sequences are excluded from the publication.
> Sequences listed in EXCEPT clause are also excluded from the
> publication.
>
> 5)
> + In such a case, a table or partition or sequence that is included in one
> + publication but excluded (explicitly or implicitly) by the
> + <literal>EXCEPT</literal> clause of another is considered included for
> + replication.
>
> 'a table or partition or sequence' can be changed to 'a table,
> partition, or sequence'
>
> 6)
> In existign doc, shall we give example of publication creation for
> both tables and sequences, each having its except list? This is
> important to show that EXCEPT to be given with individual ALL OBJ. We
> can cahnge last example of doc file to make this. This one:
> 'Create a publication that publishes all sequences for
> synchronization, and all changes in all tables except users and
> departments:'
>
> 7)
> getPublications:
> + * Get the list of tables/sequences for publications specified in the
> + * EXCEPT clause.
>
> We can have both tables and sequences in single publication. We should change
>
> 'tables/sequences' --> tables and sequences
>
> 8)
> In describePublications(),
>
> We had:
> if (!puballtables)
> else
> * Get tables in the EXCEPT clause for this publication */
>
> now we have added:
> if (puballsequences)
> /* Get sequences in the EXCEPT clause for this publication */
>
> Since now we can hit this function for 'all-seq' pub too, shall we
> change if-block's condition to:
>
> if (!puballtables && !puballsequences)
>
> and else-block to
>
> else if (puballtables)
>
> otherwise all-seq case will unnecessary enter these blocks and will
> exceute the logic
>
> Please review other functions too in pg_dump to see if we need such
> conditions altering.
>
>
> 9)
> +# Check the initial data on subscriber
> +$result = $node_subscriber->safe_psql('postgres',
> + "SELECT last_value, is_called FROM seq1");
> +is($result, '200|t', 'sequences in EXCEPT list is excluded');
> +
> +$result = $node_subscriber->safe_psql('postgres',
> + "SELECT last_value, is_called FROM seq2");
> +is($result, '200|t', 'initial test data replicated for seq2');
>
> Since both are replicated now because of conflicting EXCEPT in
> multi-pub, shall we change
> comment in 'is(..)'?
>
For v-003, one trivial thing:
Shall we change the name of AlterPublicationTables() as well? It now
deals in both tables and sequences.
thanks
Shveta
| From | Date | Subject | |
|---|---|---|---|
| Next Message | Nisha Moond | 2026-04-14 09:45:18 | Re: Support EXCEPT for TABLES IN SCHEMA publications |
| Previous Message | Amit Kapila | 2026-04-14 09:36:50 | Re: Support EXCEPT for TABLES IN SCHEMA publications |